From 973b0eebe5532e9f7dd4a33806bd9bab534b547a Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 14 Oct 2026 13:18:23 +1100 Subject: [PATCH 01/12] feat(sogs): show each account's posted name before a ban sogs-ban now lists every id before the confirmation prompt, and in a dry run, with the display name from its newest post in each room's last 256 messages: "posted as 'X'", "posted, with no display name" or "no recent posts". A room that cannot be read is named under the list. Claude-Session: https://claude.ai/code/session_019cnvrsCXsqLykDyQburD2f --- docs/tools/sogs-ban.md | 14 +++++ src/session_ops/sogs/ban.py | 17 +++++- src/session_ops/sogs/names.py | 104 ++++++++++++++++++++++++++++++++++ tests/sogs/test_names.py | 102 +++++++++++++++++++++++++++++++++ 4 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 src/session_ops/sogs/names.py create mode 100644 tests/sogs/test_names.py diff --git a/docs/tools/sogs-ban.md b/docs/tools/sogs-ban.md index 9c0037f..80959da 100644 --- a/docs/tools/sogs-ban.md +++ b/docs/tools/sogs-ban.md @@ -30,6 +30,20 @@ the deletion has already walked. There is no flag to keep the messages or to narrow the scope: the tool exists for abuse reports, where both are always wanted. +## Who is about to be banned + +Before the prompt, and in a dry run, each id is listed with the display name of its newest +post in our rooms' last 256 messages: + +``` + 05abc...def posted as 'Mallory' + 05123...456 no recent posts +``` + +SOGS stores no profiles, so a name exists only in a message the account posted: an +account that has not posted recently has none to show. A room that cannot be read is +named under the list, since its posters would otherwise read as "no recent posts". + ## Confirming the ban Each step's HTTP status is the confirmation, and the script prints one line per step: diff --git a/src/session_ops/sogs/ban.py b/src/session_ops/sogs/ban.py index 4344a3a..312ec80 100644 --- a/src/session_ops/sogs/ban.py +++ b/src/session_ops/sogs/ban.py @@ -112,6 +112,8 @@ import requests from nacl.signing import SigningKey +from session_ops.sogs import names as sogs_names + try: from session_util import blinding, xed25519 except ImportError as e: # pragma: no cover - environment, not logic @@ -392,6 +394,12 @@ def read_ids(args): return out +def describe_poster(sid, names): + if sid not in names: + return "no recent posts" + return f"posted as {names[sid]!r}" if names[sid] else "posted, with no display name" + + def confirm(assume_yes): """Asks before acting. A closed stdin is a refusal, not a traceback: these run from cron and CI, where the prompt would otherwise fail with an unhandled EOFError.""" @@ -478,10 +486,15 @@ def main(): for step in steps: print(f" {step}") + print() + names, unread = sogs_names.recent_names(client, rooms, ids) + for sid in ids: + print(f" {sid} {describe_poster(sid, names)}") + for token, code in unread: + print(f" (could not read {token}: {code}; its posters are missing above)") + if args.dry_run: print("\nDry run: no ban or deletion was sent.") - for sid in ids: - print(f" {sid}") return 0 if not confirm(args.yes): diff --git a/src/session_ops/sogs/names.py b/src/session_ops/sogs/names.py new file mode 100644 index 0000000..bc9d6cd --- /dev/null +++ b/src/session_ops/sogs/names.py @@ -0,0 +1,104 @@ +""" +The display names accounts last posted under, read from our rooms' recent messages. + +SOGS stores no profiles: a name exists on the server only inside the messages an account +has posted, as the profile its client attaches to each one. An account that has not +posted recently has no name to find. +""" + +from base64 import b64decode + +# Module imports both ways: ban and names import each other at load time. +from session_ops.sogs import ban + +# The most GET /room//messages/recent returns. +RECENT_LIMIT = 256 + +CONTENT_DATA_MESSAGE = 1 +DATA_MESSAGE_PROFILE = 101 +PROFILE_DISPLAY_NAME = 1 + + +def _varint(buf, i): + out = shift = 0 + while True: + b = buf[i] + i += 1 + out |= (b & 0x7F) << shift + shift += 7 + if not b & 0x80: + return out, i + + +def _fields(buf): + """(field number, value) for each field of a protobuf message; bytes for + length-delimited fields, which is all a name lookup descends into.""" + i = 0 + while i < len(buf): + key, i = _varint(buf, i) + number, wire_type = key >> 3, key & 7 + if wire_type == 0: + value, i = _varint(buf, i) + elif wire_type == 1: + value, i = buf[i:i + 8], i + 8 + elif wire_type == 2: + size, i = _varint(buf, i) + value, i = buf[i:i + size], i + size + elif wire_type == 5: + value, i = buf[i:i + 4], i + 4 + else: + raise ValueError(f"unsupported wire type {wire_type}") + yield number, value + + +def _field(buf, number): + return next((v for n, v in _fields(buf) if n == number and isinstance(v, bytes)), None) + + +def display_name(data_b64): + """The display name in a SOGS message's `data`, or None if it carries none or does + not parse.""" + try: + raw = b64decode(data_b64) + except ValueError: + return None + # Clients pad the Content with 0x80 and then zeros. + raw = raw.rstrip(b'\x00') + if raw.endswith(b'\x80'): + raw = raw[:-1] + try: + name = raw + for number in (CONTENT_DATA_MESSAGE, DATA_MESSAGE_PROFILE, PROFILE_DISPLAY_NAME): + name = _field(name, number) + if name is None: + return None + except (IndexError, ValueError): + return None + return name.decode('utf-8', 'replace') or None + + +def recent_names(client, rooms, session_ids): + """Looks each 05 id up in every room's recent messages. + + Returns ({id: name}, [(room token, status)]). An id that posted is in the dict, with + the name from its newest post (None if that post carried none); an id that did not + is absent. The list names the rooms that could not be read, so a missing id is not + mistaken for one that never posted. + """ + owner = {b: sid for sid in session_ids for b in ban.blinded_ids(sid, client.server_pubkey)} + owner.update({sid: sid for sid in session_ids}) + newest, unread = {}, [] + for room in rooms: + token = room['token'] + code, messages = client.request('GET', f'/room/{token}/messages/recent?limit={RECENT_LIMIT}') + if code != 200 or not isinstance(messages, list): + unread.append((token, code)) + continue + for m in messages: + sid = owner.get(m.get('session_id')) + # A deleted message stays in the list with its data nulled. + if not sid or not m.get('data'): + continue + if sid not in newest or m['posted'] > newest[sid][0]: + newest[sid] = (m['posted'], display_name(m['data'])) + return {sid: name for sid, (_, name) in newest.items()}, unread diff --git a/tests/sogs/test_names.py b/tests/sogs/test_names.py new file mode 100644 index 0000000..f1e069f --- /dev/null +++ b/tests/sogs/test_names.py @@ -0,0 +1,102 @@ +""" + uv run python -m unittest tests.sogs.test_names +""" + +import unittest +from base64 import b64encode + +from nacl.signing import SigningKey + +from session_ops.sogs import ban, names +from tests.sogs.test_ban import BLINDED_ABS, SERVER_PUBKEY, SESSION_ID + +OTHER_ID = ban.session_id_of(SigningKey(bytes(range(32)))) + + +def field(number, payload): + return varint(number << 3 | 2) + varint(len(payload)) + payload + + +def varint(n): + out = b'' + while True: + b, n = n & 0x7F, n >> 7 + out += bytes([b | (0x80 if n else 0)]) + if not n: + return out + + +def message_data(name, pad=True): + profile = field(1, name.encode()) + data_message = field(1, b'hello') + field(101, profile) + content = field(1, data_message) + if pad: + content += b'\x80' + b'\x00' * (160 - len(content) - 1) + return b64encode(content).decode() + + +def post(sid, posted, data): + return {'session_id': sid, 'posted': posted, 'data': data} + + +class FakeClient: + def __init__(self, by_room): + self.server_pubkey = bytes.fromhex(SERVER_PUBKEY) + self.by_room = by_room + + def request(self, method, path, body_json=None): + token = path.split('/')[2] + return self.by_room[token] + + +class TestDisplayName(unittest.TestCase): + def test_reads_the_name_from_padded_content(self): + self.assertEqual(names.display_name(message_data('Mallory')), 'Mallory') + + def test_reads_it_without_padding(self): + self.assertEqual(names.display_name(message_data('Mallory', pad=False)), 'Mallory') + + def test_content_without_a_profile_has_no_name(self): + content = field(1, field(1, b'hello')) + self.assertIsNone(names.display_name(b64encode(content).decode())) + + def test_garbage_is_no_name_rather_than_an_error(self): + self.assertIsNone(names.display_name(b64encode(b'\x0a\xff\xff').decode())) + self.assertIsNone(names.display_name('not base64!')) + + +class TestRecentNames(unittest.TestCase): + def test_the_newest_post_names_the_account(self): + client = FakeClient({ + 'a': (200, [post(BLINDED_ABS, 10, message_data('Old')), post(BLINDED_ABS, 30, message_data('New'))]), + 'b': (200, [post(BLINDED_ABS, 20, message_data('Middle'))]), + }) + found, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], [SESSION_ID]) + self.assertEqual(found, {SESSION_ID: 'New'}) + self.assertEqual(unread, []) + + def test_an_account_that_did_not_post_is_absent(self): + client = FakeClient({'a': (200, [post(BLINDED_ABS, 10, message_data('Someone'))])}) + found, _ = names.recent_names(client, [{'token': 'a'}], [OTHER_ID]) + self.assertEqual(found, {}) + + def test_a_deleted_post_is_skipped(self): + client = FakeClient({'a': (200, [post(BLINDED_ABS, 10, None)])}) + found, _ = names.recent_names(client, [{'token': 'a'}], [SESSION_ID]) + self.assertEqual(found, {}) + + def test_an_unreadable_room_is_reported(self): + client = FakeClient({'a': (403, 'nope'), 'b': (200, [])}) + _, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], [SESSION_ID]) + self.assertEqual(unread, [('a', 403)]) + + +class TestDescribePoster(unittest.TestCase): + def test_each_case_reads_differently(self): + self.assertEqual(ban.describe_poster(SESSION_ID, {}), "no recent posts") + self.assertEqual(ban.describe_poster(SESSION_ID, {SESSION_ID: None}), "posted, with no display name") + self.assertEqual(ban.describe_poster(SESSION_ID, {SESSION_ID: 'Mallory'}), "posted as 'Mallory'") + + +if __name__ == '__main__': + unittest.main() From f8d1a8e647a30083e2c4adcea3c1784795ef3120 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 14 Oct 2026 13:27:19 +1100 Subject: [PATCH 02/12] feat(zendesk): link every unflagged ticket on one digest line A ticket the model did not mark worth looking into was only counted in the header's category tally, then recorded as reported, so it never reached Discord as a link at all. #28891, a lost recovery phrase, went that way. They now get one line under the abuse reports, grouped by category, most common first, with the same link budget and a count that covers them all. Claude-Session: https://claude.ai/code/session_019cnvrsCXsqLykDyQburD2f --- src/session_ops/zendesk/triage.py | 46 +++++++++++++++++++++--- tests/zendesk/test_triage.py | 60 +++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/session_ops/zendesk/triage.py b/src/session_ops/zendesk/triage.py index edb79ee..0b8bb01 100644 --- a/src/session_ops/zendesk/triage.py +++ b/src/session_ops/zendesk/triage.py @@ -670,6 +670,9 @@ def analyze(model, effort, compact_tickets): # Character budget for the links on that line, so a day of nothing but abuse reports # can't push it past MAX_MESSAGE_CHARS. Past it the rest are counted, not linked. COLLAPSED_LINKS_CHARS = 900 +# Every other ticket the model did not flag goes on one line under it, linked and +# grouped by category: counted only in the tally, a ticket left the digest unseen. +UNFLAGGED_MARKER = "πŸ’€" # ---- Components V2 --------------------------------------------------------- # @@ -756,6 +759,36 @@ def build_collapsed_line(collapsed, subdomain): f"actionable ({', '.join(links)})") +def build_unflagged_line(unflagged, subdomain): + """The tickets nobody is asked to look at, as one line under the abuse reports: + + πŸ’€ **5** not flagged β€” πŸ”‘ #28891, #28880 Β· ❓ #28870, #28866 Β· πŸ’‘ #28900 + + Grouped by category, most common first, so the emoji says what each run is + without a summary per ticket. Links share COLLAPSED_LINKS_CHARS the same way, and + the count always covers every one of them. + """ + by_category = {} + for f in unflagged: + by_category.setdefault(f.get("category"), []).append(f) + groups, used, unlisted = [], 0, 0 + for category, members in sorted(by_category.items(), key=lambda kv: -len(kv[1])): + links = [] + for f in members: + link = f"[#{f.get('id')}]({ticket_url(subdomain, f.get('id'))})" + used += len(link) + 2 + if used > COLLAPSED_LINKS_CHARS: + unlisted += 1 + continue + links.append(link) + if links: + groups.append(f"{CATEGORY_EMOJI.get(category, 'β€’')} {', '.join(links)}") + if unlisted: + groups.append(f"+{unlisted} more") + return (f"{UNFLAGGED_MARKER} **{len(unflagged)}** not flagged β€” " + + " Β· ".join(groups)) + + def build_header(findings, highlights, stats=None): """The lead lines: what was looked at, the category tally, duplicate clusters. @@ -861,13 +894,14 @@ def build_messages(findings, subdomain, stats=None, updated_ids=None): omitted_ids = {f.get("id") for f in omitted} collapsed = [f for f in findings if is_collapsed(f)] collapsed_ids = {f.get("id") for f in collapsed} + unflagged = [f for f in findings if f.get("id") not in shown_ids | omitted_ids | collapsed_ids] + unflagged_ids = {f.get("id") for f in unflagged} - # The header accounts for every classified ticket except the highlights that - # didn't fit; those are covered by no message and stay eligible next run. The - # collapsed line accounts for the abuse reports, links or not β€” its count covers - # every one of them, so re-reporting them next run would be noise. + # The highlights that didn't fit are covered by no message and stay eligible next + # run. The two summary lines account for every ticket they count, links or not, + # so re-reporting those next run would be noise. header_ids = ({f.get("id") for f in findings} - - shown_ids - omitted_ids - collapsed_ids) + - shown_ids - omitted_ids - collapsed_ids - unflagged_ids) header = build_header(findings, shown + omitted, stats) if omitted: header += (f"\nShowing the top **{len(shown)}** of " @@ -880,6 +914,8 @@ def build_messages(findings, subdomain, stats=None, updated_ids=None): # Last, under the individual tickets: lowest position for the lowest priority. if collapsed: entries.append((build_collapsed_line(collapsed, subdomain), collapsed_ids)) + if unflagged: + entries.append((build_unflagged_line(unflagged, subdomain), unflagged_ids)) messages, coverage = discord.messages_from_entries( header, entries, MAX_ENTRIES_PER_MESSAGE, MAX_COMPONENT_CHARS) diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index bf6010e..6274399 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -558,8 +558,8 @@ class TestBuildMessages(unittest.TestCase): def test_only_tickets_worth_looking_into_get_a_line(self): findings = [finding(1), finding(2, worth_looking_into=False)] text = digest_text(build_messages(findings, "acme")) - self.assertEqual(sum(1 for line in text.splitlines() if "[#" in line), 1) - self.assertIn("[#1]", text) + self.assertIn("| [#1]", text) + self.assertNotIn("| [#2]", text) def test_highlights_are_ordered_by_priority_rank(self): findings = [finding(1, priority_rank=3), finding(2, priority_rank=1)] @@ -714,6 +714,62 @@ def test_nothing_is_clipped_when_it_fits(self): self.assertNotIn("more", line) +class TestUnflaggedLine(unittest.TestCase): + """Every ticket the model did not flag is still linked once, so a lost recovery + phrase the model judged routine can be found from the digest.""" + + def quiet(self, ticket_id, category="account_access"): + return finding(ticket_id, category=category, worth_looking_into=False) + + def test_an_unflagged_ticket_is_linked(self): + text = digest_text(build_messages([finding(1), self.quiet(2)], "acme")) + self.assertIn("[#2](https://acme.zendesk.com/agent/tickets/2)", text) + self.assertIn("**1** not flagged", text) + + def test_it_sits_below_the_abuse_reports(self): + findings = [self.quiet(2), finding(3, category="abuse_report", + severity="not_applicable"), finding(1)] + lines = [l for l in digest_text(build_messages(findings, "acme")).splitlines() + if "[#" in l] + self.assertIn("abuse report", lines[-2]) + self.assertIn("not flagged", lines[-1]) + + def test_tickets_are_grouped_by_category_most_common_first(self): + findings = [self.quiet(1, "question"), self.quiet(2, "account_access"), + self.quiet(3, "account_access")] + line = triage.build_unflagged_line(findings, "acme") + recovery, question = (triage.CATEGORY_EMOJI[c] for c in ("account_access", "question")) + self.assertLess(line.index(recovery), line.index(question)) + self.assertEqual(line.count(recovery), 1) + + def test_abuse_reports_and_highlights_are_not_repeated_on_it(self): + findings = [finding(1), finding(2, category="abuse_report", severity="not_applicable"), + self.quiet(3)] + line = [l for l in digest_text(build_messages(findings, "acme")).splitlines() + if "not flagged" in l][0] + self.assertNotIn("[#1]", line) + self.assertNotIn("[#2]", line) + self.assertIn("**1** not flagged", line) + + def test_highlights_beyond_the_cap_stay_off_it(self): + """Those stay eligible for the next run rather than being marked seen.""" + findings = [finding(i, priority_rank=i) for i in range(triage.MAX_HIGHLIGHTS + 2)] + self.assertNotIn("not flagged", digest_text(build_messages(findings, "acme"))) + + def test_it_covers_the_tickets_it_counts(self): + messages, coverage = triage.build_messages([finding(1), self.quiet(2)], "acme") + self.assertIn(2, set().union(*coverage)) + + def test_a_long_run_of_links_is_clipped_but_fully_counted(self): + line = triage.build_unflagged_line([self.quiet(27000 + i) for i in range(200)], "acme") + self.assertLess(len(line), triage.MAX_MESSAGE_CHARS) + self.assertIn("**200** not flagged", line) + self.assertRegex(line, r"\+\d+ more$") + + def test_no_line_when_every_ticket_is_flagged(self): + self.assertNotIn("not flagged", digest_text(build_messages([finding(1)], "acme"))) + + # ---- Parsing helpers ------------------------------------------------------- From 04fe017916cc0d69a1a5caad849e516b114cb960 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 14 Oct 2026 13:29:53 +1100 Subject: [PATCH 03/12] feat(zendesk): write an English transcript for every non-English ticket The digest translated only the tickets it gave a card, a scope set for the Comment button those cards no longer carry. 2 of the 35 non-English tickets in the queue had a transcript. Every ticket the run classifies is now passed to attach_english. Unchanged tickets are skipped before classification, so each one is translated once per change in its conversation. Claude-Session: https://claude.ai/code/session_019cnvrsCXsqLykDyQburD2f --- deploy/env/zendesk.env.example | 2 +- src/session_ops/zendesk/transcript.py | 15 +++++---------- src/session_ops/zendesk/triage.py | 14 ++++++-------- tests/zendesk/test_triage.py | 17 +++++++++++++++++ 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/deploy/env/zendesk.env.example b/deploy/env/zendesk.env.example index c8f731c..f172120 100644 --- a/deploy/env/zendesk.env.example +++ b/deploy/env/zendesk.env.example @@ -20,7 +20,7 @@ ZENDESK_WEBHOOK_SECRET= # install -o zendesk -g zendesk -m 640 house_answers.json /var/lib/session-ops/zendesk-digest/ #ZENDESK_HOUSE_ANSWERS=/var/lib/session-ops/zendesk-digest/house_answers.json # Optional. A non-customer-visible multi-line text ticket field (its id is in its URL) -# that gets an English rendering of each non-English ticket the digest posts. +# that gets an English rendering of each non-English ticket the digest classifies. #ZENDESK_ENGLISH_FIELD_ID= # The whole `claude:` note path, writing nothing to Zendesk. Set it for the first deploy. #RELAY_DRY_RUN=1 diff --git a/src/session_ops/zendesk/transcript.py b/src/session_ops/zendesk/transcript.py index 33ed9b2..fc75e4f 100644 --- a/src/session_ops/zendesk/transcript.py +++ b/src/session_ops/zendesk/transcript.py @@ -122,16 +122,11 @@ def write_english_field(session, subdomain, ticket_id, field_id, english): def attach_english(session, subdomain, tickets, findings, model, field_id): - """Render every non-English ticket about to be posted into English, on the ticket. + """Render every non-English ticket in the run into English, on the ticket. - Runs before the digest is posted, and that order is the whole design: the Comment - button exists only on a digest card, so a ticket that reaches the dialog has - necessarily been through here first. relay.py can then read the field it needs - without a Claude call of its own β€” which it has no time for, being on the three - seconds Discord allows a dialog that cannot be deferred. - - Scoped to the tickets that actually get a button. Translating the rest would be - paying for every ticket in the window to serve the handful anybody replies to. + One call per non-English ticket the run classified. The digest only classifies + tickets that are new or changed since they were last reported, so a ticket is + translated again only when its conversation has moved on. Never raises: this is enrichment, and a digest that fails to post because a translation failed would be a worse trade than a dialog showing German. @@ -143,7 +138,7 @@ def attach_english(session, subdomain, tickets, findings, model, field_id): return 0 candidates = [f for f in findings if not is_english(f)] if not candidates: - print(f"No non-English tickets among the {len(findings)} being posted; " + print(f"No non-English tickets among the {len(findings)} classified; " f"no English transcripts to write.") return 0 by_id = {t.get("id"): t for t in tickets} diff --git a/src/session_ops/zendesk/triage.py b/src/session_ops/zendesk/triage.py index edb79ee..6eb32f1 100644 --- a/src/session_ops/zendesk/triage.py +++ b/src/session_ops/zendesk/triage.py @@ -1080,16 +1080,14 @@ def main(argv=None): print(f"{len(findings)} tickets classified; {len(shown) + len(omitted)} worth looking into" + (f" ({len(omitted)} beyond the display cap)." if omitted else ".")) - # Before the post, never after: the Comment button only exists on a digest card, - # so writing the rendering first is what guarantees every ticket that can reach - # the dialog already carries one. + # Every classified ticket, not just the highlights: whoever opens one from any + # line of the digest, or straight from the queue, reads it in English. # - # Gated on the post actually happening, not just on --dry-run. --no-discord is - # the flag CI runs with, and a run that posts no card creates no button β€” so a - # rendering written there would be a write to a production ticket for a dialog - # that can never be opened. A rehearsal passes no field, so it writes nothing. + # Gated on the post actually happening, not just on --dry-run: --no-discord is + # the flag CI runs with, and CI must not write to production tickets. A + # rehearsal passes no field, so it writes nothing. if needs_discord: - attach_english(zd, subdomain, classified, shown, MODEL, + attach_english(zd, subdomain, classified, findings, MODEL, None if rehearsing() else get_env(ENGLISH_FIELD_ENV, required=False)) messages, coverage = build_messages(findings, subdomain, stats, updated_ids) diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index bf6010e..7a85bf2 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -1966,6 +1966,23 @@ def test_a_rehearsal_writes_no_transcript(self): triage.main(["--findings", path]) self.assertEqual(seen, [None]) + def test_unflagged_tickets_are_translated_too(self): + """A ticket with no card of its own is still opened from the digest or the + queue, and whoever opens it should read it in English.""" + seen = [] + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "findings.json") + with open(path, "w", encoding="utf-8") as handle: + json.dump([finding(1), finding(2, worth_looking_into=False, language="German")], + handle) + with Patched(triage, attach_english=lambda *a: seen.extend(f["id"] for f in a[3]), + post_to_discord=lambda session, url, messages: len(messages)), \ + mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", + "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y"}), \ + contextlib.redirect_stdout(io.StringIO()): + triage.main(["--findings", path]) + self.assertEqual(sorted(seen), [1, 2]) + def said(self, **kwargs): """attach_english's console output for one call.""" out = io.StringIO() From ec405f7a18a907e5b2cc530314b6f7aa00f6f6da Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 14 Oct 2026 14:06:50 +1100 Subject: [PATCH 04/12] feat(zendesk): post claude: english's note on non-English tickets The digest wrote its English rendering into a ticket field that nothing reads any more. It now posts the private note `claude: english` writes, with the same marker, so whichever gets there first stops the other repeating it until the customer writes again. translate_conversation and english_note are shared by both. The field code, its env setting and its tests are gone. Claude-Session: https://claude.ai/code/session_019cnvrsCXsqLykDyQburD2f --- deploy/env/zendesk.env.example | 3 - docs/jobs/zendesk-digest.md | 2 + src/session_ops/zendesk/note_reply.py | 131 ++++++++--- src/session_ops/zendesk/transcript.py | 129 +---------- src/session_ops/zendesk/triage.py | 11 +- tests/zendesk/test_note_reply.py | 11 + tests/zendesk/test_triage.py | 310 ++++++++++---------------- 7 files changed, 246 insertions(+), 351 deletions(-) diff --git a/deploy/env/zendesk.env.example b/deploy/env/zendesk.env.example index f172120..2d3602b 100644 --- a/deploy/env/zendesk.env.example +++ b/deploy/env/zendesk.env.example @@ -19,8 +19,5 @@ ZENDESK_WEBHOOK_SECRET= # text, so it is not in the repo; copy it by hand: # install -o zendesk -g zendesk -m 640 house_answers.json /var/lib/session-ops/zendesk-digest/ #ZENDESK_HOUSE_ANSWERS=/var/lib/session-ops/zendesk-digest/house_answers.json -# Optional. A non-customer-visible multi-line text ticket field (its id is in its URL) -# that gets an English rendering of each non-English ticket the digest classifies. -#ZENDESK_ENGLISH_FIELD_ID= # The whole `claude:` note path, writing nothing to Zendesk. Set it for the first deploy. #RELAY_DRY_RUN=1 diff --git a/docs/jobs/zendesk-digest.md b/docs/jobs/zendesk-digest.md index ef7e365..b50fa84 100644 --- a/docs/jobs/zendesk-digest.md +++ b/docs/jobs/zendesk-digest.md @@ -10,6 +10,8 @@ Claude reviews the Zendesk tickets awaiting a reply β€” `new` and `open`, no app | Re-run | `systemctl start session-ops@zendesk-digest.service` | | Logs | `journalctl -u session-ops@zendesk-digest -n 50 --no-pager` | +Every non-English ticket it classifies gets the private note `claude: english` writes: the whole conversation, both sides, in English. The two share a marker, so a conversation is translated once per new public comment whichever writes first. `--no-discord`, `--dry-run` and rehearsals write nothing to Zendesk. + ## Categories `CATEGORY_SPECS` in [triage.py](../../src/session_ops/zendesk/triage.py) is the single source of truth β€” the schema enum, the Discord labels and emoji, which categories count as urgent, and the prompt guidance are all derived from it, so adding a category is one edit. diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index 4f69cc2..fb8b616 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -62,6 +62,8 @@ import sys import textwrap +import requests + from session_ops.shared.discord import clip from session_ops.shared.env import get_env from session_ops.shared.text import undash_english @@ -1014,31 +1016,27 @@ def already_english(turns, translated): for turn in turns) -def run_english(session, subdomain, model, ticket, comments, command, dry_run): - """Put the conversation on the ticket in English, as a private note. +def translate_conversation(session, subdomain, model, ticket, comments): + """The conversation on a ticket in English, or why there is none. + + Returns (outcome, detail): ("current", None) when a transcript already covers the + newest public comment, ("empty", None), ("english", None) when the conversation + is already English, ("failed", the reason), or ("ready", (turns, rendered, latest)). Reads both sides, not just the customer's: their second message is usually an answer to a reply, and without the reply it reads as a complaint about nothing. - Python owns the speaker labels and timestamps and the model only translates β€” - the same split transcript.py makes, for the same reason: a model asked to format the - transcript can drop a turn, merge two, or date one it was never given, and each - of those is invisible in the output. + Python owns the speaker labels and timestamps and the model only translates: a + model asked to format the transcript can drop a turn, merge two, or date one it + was never given, and each of those is invisible in the output. """ ticket_id = ticket["id"] latest = next((c.get("id") for c in comments if c.get("public")), None) if latest is not None and api.has_marker(comments, english_marker(latest)): - say(session, subdomain, ticket_id, command["id"], - "The English transcript on this ticket is already up to date β€” nothing " - "has been said since it was written.", dry_run, error=False) - return + return "current", None turns = api.conversation_turns(session, subdomain, ticket) if not turns: - say(session, subdomain, ticket_id, command["id"], - "There are no public comments on this ticket to translate.", dry_run, - error=False) - return - + return "empty", None payload = json.dumps([{"index": t["index"], "speaker": t["who"], "text": t["body"]} for t in turns], ensure_ascii=False) try: @@ -1047,28 +1045,101 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): clip(payload, transcript.TRANSCRIPT_INPUT_CHARS), transcript.ENGLISH_TIMEOUT_SECONDS, f"the English transcript of #{ticket_id}") except SystemExit as exc: - say(session, subdomain, ticket_id, command["id"], - f"The English transcript could not be produced: {exc} To try again, add " - "a new private note reading claude: english.", dry_run) - return + return "failed", str(exc) + # A transcript of English text repeats what is already a few comments above it. if already_english(turns, rendered.get("turns")): - # A transcript of English text repeats what is already a few comments above - # it. Say so rather than posting the same words back. + return "english", None + return "ready", (turns, rendered, latest) + + +def english_note(turns, rendered, latest, command_id=None): + """The private note carrying the transcript. Its english_marker is what stops + either the command or the digest writing the same transcript twice.""" + markers = ([done_marker(command_id)] if command_id is not None else []) + [english_marker(latest)] + return "".join( + [para(f"This conversation in English β€” {len(turns)} turn(s), both sides."), + para("Translated for reading; the customer has not seen this.")] + + transcript_blocks(turns, rendered.get("turns")) + + [para(" ".join(markers))]) + + +def run_english(session, subdomain, model, ticket, comments, command, dry_run): + """Put the conversation on the ticket in English, as a private note.""" + ticket_id = ticket["id"] + outcome, detail = translate_conversation(session, subdomain, model, ticket, comments) + answers = { + "current": "The English transcript on this ticket is already up to date β€” nothing " + "has been said since it was written.", + "empty": "There are no public comments on this ticket to translate.", + "english": "This conversation is already in English, so there is nothing to " + "translate.", + } + if outcome in answers: + say(session, subdomain, ticket_id, command["id"], answers[outcome], dry_run, + error=False) + return + if outcome == "failed": say(session, subdomain, ticket_id, command["id"], - "This conversation is already in English, so there is nothing to " - "translate.", dry_run, error=False) + f"The English transcript could not be produced: {detail} To try again, add " + "a new private note reading claude: english.", dry_run) return + turns, rendered, latest = detail print(f"#{ticket_id}: rendered {len(turns)} turn(s) in English.") if dry_run: print(f"#{ticket_id}: dry run, nothing written.") return - note = "".join( - [para(f"This conversation in English β€” {len(turns)} turn(s), both sides."), - para("Translated for reading; the customer has not seen this.")] - + transcript_blocks(turns, rendered.get("turns")) - + [para(f"{done_marker(command['id'])} {english_marker(latest)}")]) - write_to_ticket(session, subdomain, ticket_id, note, public=False, as_html=True, - drop_tags=[TAG_QUEUED, TAG_ERROR]) + write_to_ticket(session, subdomain, ticket_id, + english_note(turns, rendered, latest, command["id"]), + public=False, as_html=True, drop_tags=[TAG_QUEUED, TAG_ERROR]) + + +def post_english_notes(session, subdomain, tickets, findings, model): + """The digest's `claude: english`, unasked: the same private note on every ticket + in the run the classifier did not call English. + + The digest only classifies tickets that are new or changed, and the note's marker + is shared with the command, so a conversation is translated once per new public + comment whichever of the two gets there first. + + Never raises: a digest that fails to post because one translation failed is a + worse trade than one ticket left untranslated. + """ + if not session: + return 0 + candidates = [f for f in findings if not transcript.is_english(f)] + if not candidates: + print(f"No non-English tickets among the {len(findings)} classified; " + f"no English notes to post.") + return 0 + by_id = {t.get("id"): t for t in tickets} + written = 0 + for finding in candidates: + ticket = by_id.get(finding.get("id")) + if not ticket: + print(f"Note: #{finding.get('id')} was classified {finding.get('language')!r} " + f"but never fetched; no English note.") + continue + comments = api.fetch_comments(session, subdomain, ticket["id"], optional=True) + if comments is None: + continue + outcome, detail = translate_conversation(session, subdomain, model, ticket, comments) + if outcome != "ready": + print(f"#{ticket['id']}: no English note ({outcome}" + + (f": {detail})" if outcome == "failed" else ")")) + continue + turns, rendered, latest = detail + try: + write_to_ticket(session, subdomain, ticket["id"], + english_note(turns, rendered, latest), + public=False, as_html=True) + except (SystemExit, requests.RequestException) as exc: + print(f"Note: could not post the English note on #{ticket['id']} ({exc}).") + continue + written += 1 + # Printed even at zero, so a run that wrote nothing reads differently in the + # journal from one that never reached this step. + print(f"Posted an English note on {written} of {len(candidates)} non-English ticket(s).") + return written def say(session, subdomain, ticket_id, comment_id, text, dry_run, error=True): diff --git a/src/session_ops/zendesk/transcript.py b/src/session_ops/zendesk/transcript.py index fc75e4f..efcc3c4 100644 --- a/src/session_ops/zendesk/transcript.py +++ b/src/session_ops/zendesk/transcript.py @@ -1,26 +1,12 @@ -"""The English transcript the digest writes onto a non-English ticket. -""" -import json - -import requests - -from session_ops.shared.discord import clip -from session_ops.zendesk.api import conversation_turns -from session_ops.zendesk.claude_cli import claude_cli_json +"""The prompt and schema that put a ticket's conversation into English. - -# Optional, and absent until the field exists in Zendesk. Everything below is a -# no-op without it: the digest posts exactly as it did before and relay.py falls -# back to the ticket's own comments, which is what it showed all along. -ENGLISH_FIELD_ENV = "ZENDESK_ENGLISH_FIELD_ID" +Shared by `claude: english` and the digest, which post the same private note. +""" ENGLISH_TIMEOUT_SECONDS = 180 -# The transcript is a whole conversation rather than one description, so both budgets -# are larger than the classifier's. The field holds well past the 1,200 relay.py -# shows, so the field is never why the dialog is missing a sentence. +# A whole conversation rather than one description, so larger than the classifier's. TRANSCRIPT_INPUT_CHARS = 8000 -TRANSCRIPT_CHARS = 12000 TRANSCRIPT_SCHEMA = { @@ -66,7 +52,7 @@ def is_english(finding): """Whether the classifier called this ticket English. - Unknown counts as English: the field is only worth writing when it says + Unknown counts as English: the note is only worth posting when it says something the agent cannot already read, and a blank `language` is far more likely to be a classification that came back thin than a ticket nobody could read. Guessing wrong this way costs a transcript nobody needed; the other way @@ -74,108 +60,3 @@ def is_english(finding): """ language = (finding.get("language") or "").strip().lower() return not language or language.startswith(("english", "en")) - - -def render_transcript(turns, translated): - """Turns plus their translations as the text that goes on the ticket. - - Python owns the timestamps and the speaker labels rather than the model. Asked to - format the transcript itself, a model can drop a turn, merge two, or date one it - was never given β€” and every one of those is invisible in the output. Translating - is the only part that needs a model, so it is the only part it is given. - - A turn the model did not return keeps its original text. Untranslated is a - degraded transcript; missing is a conversation that reads as if it never happened. - """ - english = {} - for item in translated or []: - try: - english[int(item.get("index"))] = (item.get("english") or "").strip() - except (TypeError, ValueError): - continue - blocks = [] - for turn in turns: - header = " ".join(part for part in (turn["when"], f'{turn["who"]}:') if part) - blocks.append(f'{header}\n{english.get(turn["index"]) or turn["body"]}') - return "\n\n".join(blocks) - - -def write_english_field(session, subdomain, ticket_id, field_id, english): - """Put the transcript on the ticket. Returns whether Zendesk took it. - - One field overwritten, not a note appended: a ticket carries one current English - version of the whole conversation rather than a chain of partial ones to read in - order. - """ - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket_id}.json" - payload = {"ticket": {"custom_fields": [{"id": field_id, "value": english}]}} - try: - resp = session.request("PUT", url, attempts=2, json=payload) - except requests.RequestException as exc: - print(f"Note: could not write the English transcript to #{ticket_id} ({exc}).") - return False - if resp.status_code >= 400: - print(f"Note: #{ticket_id} rejected the English transcript " - f"({resp.status_code}).") - return False - return True - - -def attach_english(session, subdomain, tickets, findings, model, field_id): - """Render every non-English ticket in the run into English, on the ticket. - - One call per non-English ticket the run classified. The digest only classifies - tickets that are new or changed since they were last reported, so a ticket is - translated again only when its conversation has moved on. - - Never raises: this is enrichment, and a digest that fails to post because a - translation failed would be a worse trade than a dialog showing German. - """ - if not session: - return 0 - if not field_id: - print(f"No {ENGLISH_FIELD_ENV} set; no English transcripts written.") - return 0 - candidates = [f for f in findings if not is_english(f)] - if not candidates: - print(f"No non-English tickets among the {len(findings)} classified; " - f"no English transcripts to write.") - return 0 - by_id = {t.get("id"): t for t in tickets} - written = 0 - for finding in candidates: - ticket = by_id.get(finding.get("id")) - if not ticket: - # --findings, or a fetch that returned the classification but not the row. - print(f"Note: #{finding.get('id')} was classified {finding.get('language')!r} " - f"but never fetched; no transcript.") - continue - turns = conversation_turns(session, subdomain, ticket) - if not turns: - print(f"Note: #{ticket['id']} has no public comments to render.") - continue - payload = json.dumps( - [{"index": t["index"], "speaker": t["who"], "text": t["body"]} - for t in turns], ensure_ascii=False) - try: - rendered = claude_cli_json( - model, "medium", TRANSCRIPT_SYSTEM_PROMPT, TRANSCRIPT_SCHEMA, - clip(payload, TRANSCRIPT_INPUT_CHARS), ENGLISH_TIMEOUT_SECONDS, - f"the English transcript of #{ticket['id']}") - except SystemExit as exc: - # claude_cli_json exits on a failed call, which is right for the - # classification it was written for and wrong here: one ticket nobody - # can translate must not take the digest down with it. - print(f"Note: could not render #{ticket['id']} in English ({exc}).") - continue - english = clip(render_transcript(turns, rendered.get("turns")), - TRANSCRIPT_CHARS) - if english and write_english_field(session, subdomain, ticket["id"], - field_id, english): - written += 1 - # Printed even at zero. A run that wrote nothing and a run that never reached - # this step read identically in the journal otherwise, which is the one thing - # somebody checking whether the feature is on actually needs to tell apart. - print(f"Wrote an English transcript to {written} of {len(candidates)} " - f"non-English ticket(s).") - return written diff --git a/src/session_ops/zendesk/triage.py b/src/session_ops/zendesk/triage.py index 6eb32f1..0d68536 100644 --- a/src/session_ops/zendesk/triage.py +++ b/src/session_ops/zendesk/triage.py @@ -86,7 +86,7 @@ review_platform, review_stars, ticket_url, zendesk_session) from session_ops.zendesk.claude_cli import MODEL, claude_cli_json -from session_ops.zendesk.transcript import ENGLISH_FIELD_ENV, attach_english +from session_ops.zendesk.note_reply import post_english_notes # Never analyzed. A store review cannot be answered the way a ticket can: it takes # one developer response, replacing any previous one, with no way to ask a follow-up @@ -1084,11 +1084,10 @@ def main(argv=None): # line of the digest, or straight from the queue, reads it in English. # # Gated on the post actually happening, not just on --dry-run: --no-discord is - # the flag CI runs with, and CI must not write to production tickets. A - # rehearsal passes no field, so it writes nothing. - if needs_discord: - attach_english(zd, subdomain, classified, findings, MODEL, - None if rehearsing() else get_env(ENGLISH_FIELD_ENV, required=False)) + # the flag CI runs with, and neither CI nor a rehearsal may write to production + # tickets. + if needs_discord and not rehearsing(): + post_english_notes(zd, subdomain, classified, findings, MODEL) messages, coverage = build_messages(findings, subdomain, stats, updated_ids) if args.dry_run: diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index 896d573..bf50ea4 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -258,6 +258,17 @@ def test_each_turn_gets_its_speaker_line(self): self.assertIn("B", joined) # translated turn used self.assertIn("a", joined) # untranslated turn falls back to original + def test_a_scrambled_index_does_not_shift_every_later_turn(self): + """The model echoes indexes back. Matching by position instead would put one + speaker's words under another's name for the rest of the transcript.""" + turns = [{"index": 0, "who": "Customer", "when": "", "body": "eins"}, + {"index": 1, "who": "Support", "when": "", "body": "zwei"}] + joined = "".join(note_reply.transcript_blocks( + turns, [{"index": 1, "english": "two"}, {"index": 0, "english": "one"}])) + self.assertLess(joined.index("Customer:"), joined.index("one")) + self.assertLess(joined.index("one"), joined.index("Support:")) + self.assertLess(joined.index("Support:"), joined.index("two")) + def test_the_transcript_note_is_never_public(self): turns = [{"index": 0, "who": "Customer", "when": "2026-09-03 10:00 UTC", "body": "Hallo"}] diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index 7a85bf2..79d9c26 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -21,7 +21,7 @@ import requests from session_ops.shared import discord -from session_ops.zendesk import api, claude_cli, transcript, triage +from session_ops.zendesk import api, claude_cli, note_reply, transcript, triage from session_ops.shared.testing import ( FakeResponse, FakeSession, NonJsonResponse, Patched) @@ -1783,8 +1783,8 @@ def test_a_wedged_cli_does_not_hang_the_run(self): class TestEnglishTranscript(unittest.TestCase): - """The digest writes the English the reply dialog reads. It runs before the post - because the Comment button only exists on a card the post creates.""" + """The digest posts `claude: english`'s private note on every non-English ticket + it classifies, so nobody has to ask for one.""" def test_a_ticket_the_classifier_called_english_is_left_alone(self): """Translating English into English would put a machine's wording in front of @@ -1857,45 +1857,9 @@ def test_empty_comments_are_dropped(self): # ---- rendering --------------------------------------------------------- - def test_python_owns_the_timestamps_and_the_speaker_labels(self): - """Asked to format the transcript itself, a model can drop a turn, merge two, - or date one it was never given β€” and each is invisible in the output.""" - turns = [{"index": 0, "who": "Customer", "when": "2026-08-28 00:22 UTC", - "body": "Es geht nicht."}, - {"index": 1, "who": "Support", "when": "2026-08-28 01:31 UTC", - "body": "Have you tried?"}] - got = transcript.render_transcript(turns, [ - {"index": 0, "english": "It does not work."}, - {"index": 1, "english": "Have you tried?"}, - ]) - self.assertEqual(got, - "2026-08-28 00:22 UTC Customer:\nIt does not work.\n\n" - "2026-08-28 01:31 UTC Support:\nHave you tried?") - - def test_a_turn_the_model_skipped_keeps_its_original_text(self): - """Untranslated is a degraded transcript. Missing is a conversation that reads - as if that turn never happened.""" - turns = [{"index": 0, "who": "Customer", "when": "", "body": "Es geht nicht."}, - {"index": 1, "who": "Customer", "when": "", "body": "Immer noch."}] - got = transcript.render_transcript(turns, [{"index": 0, "english": "Broken."}]) - self.assertIn("Broken.", got) - self.assertIn("Immer noch.", got) - - def test_a_scrambled_index_does_not_shift_every_later_turn(self): - """The model echoes indexes back. Matching by position instead would put one - speaker's words under another's name for the rest of the transcript.""" - turns = [{"index": 0, "who": "Customer", "when": "", "body": "eins"}, - {"index": 1, "who": "Support", "when": "", "body": "zwei"}] - got = transcript.render_transcript(turns, [{"index": 1, "english": "two"}, - {"index": 0, "english": "one"}]) - self.assertEqual(got, "Customer:\none\n\nSupport:\ntwo") - - def test_an_unparseable_timestamp_leaves_the_turn_labelled(self): + def test_an_unparseable_timestamp_leaves_the_turn_unstamped(self): self.assertEqual(api.stamp_minutes("not a date"), "") self.assertEqual(api.stamp_minutes(None), "") - got = transcript.render_transcript( - [{"index": 0, "who": "Customer", "when": "", "body": "x"}], []) - self.assertEqual(got, "Customer:\nx") def test_the_stamp_is_minutes_not_seconds(self): """This dates a turn for somebody reading a conversation; seconds are noise in @@ -1905,124 +1869,100 @@ def test_the_stamp_is_minutes_not_seconds(self): # ---- the run ----------------------------------------------------------- - def test_nothing_happens_until_the_field_exists(self): - """The field is not in Zendesk yet. Until its id is configured the digest has - to behave exactly as it did before β€” no comment fetches, no Claude calls, no - writes.""" - calls = [] - with Patched(transcript, conversation_turns=lambda *a: calls.append(a), - claude_cli_json=lambda *a, **k: calls.append(a)): - written = transcript.attach_english(object(), "acme", [{"id": 1}], - [{"id": 1, "language": "German"}], - "claude-sonnet-5", field_id=None) - self.assertEqual(written, 0) - self.assertEqual(calls, []) - - def test_nothing_happens_without_a_zendesk_session(self): - """--findings never builds one: the findings already exist and no ticket was - ever fetched, so there is nothing to read comments from or write back to.""" - self.assertEqual( - transcript.attach_english(None, "acme", [{"id": 1}], - [{"id": 1, "language": "German"}], - "claude-sonnet-5", field_id=42), - 0) - - def test_a_run_that_posts_no_card_writes_nothing(self): - """--no-discord is the flag CI runs with. No post means no Comment button, - so a transcript written on that path is a write to a production ticket for a - dialog nobody can open. Guarded at the call site by `needs_discord`.""" - source = inspect.getsource(triage.main) - call = source.index("attach_english(") - guard = source.rindex("if needs_discord:", 0, call) - self.assertNotIn("\n ", source[guard:call].rstrip()) - - def test_the_transcript_uses_the_same_model_as_the_classification(self): + GERMAN_TURNS = [{"index": 0, "who": "Customer", "when": "2026-08-28 00:22 UTC", + "body": "Es geht nicht."}] + + def run_notes(self, tickets, findings, comments=(), translated=None, session=object()): + """post_english_notes with Zendesk and Claude faked. Returns (count, notes, out).""" + notes, out = [], io.StringIO() + translated = translated or (lambda *a, **k: { + "turns": [{"index": 0, "english": "It does not work."}]}) + with Patched(api, fetch_comments=lambda *a, **k: list(comments), + conversation_turns=lambda *a: self.GERMAN_TURNS), \ + Patched(claude_cli, claude_cli_json=translated), \ + Patched(note_reply, write_to_ticket=lambda *a, **k: notes.append((a, k))), \ + contextlib.redirect_stdout(out): + got = note_reply.post_english_notes(session, "acme", tickets, findings, + "claude-sonnet-5") + return got, notes, out.getvalue() + + def run_main(self, findings, env): + """triage.main over --findings, recording post_english_notes' arguments.""" seen = [] with tempfile.TemporaryDirectory() as directory: path = os.path.join(directory, "findings.json") with open(path, "w", encoding="utf-8") as handle: - json.dump([finding(1)], handle) - with Patched(triage, attach_english=lambda *a: seen.append(a[4]), + json.dump(findings, handle) + with Patched(triage, post_english_notes=lambda *a: seen.append(a), post_to_discord=lambda session, url, messages: len(messages)), \ mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", - "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y"}), \ + "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y", + **env}), \ contextlib.redirect_stdout(io.StringIO()): triage.main(["--findings", path]) - self.assertEqual(seen, [claude_cli.MODEL]) + return seen + + def test_a_run_that_posts_nothing_writes_nothing(self): + """--no-discord is the flag CI runs with, and CI must not write to production + tickets. Guarded at the call site by `needs_discord`.""" + source = inspect.getsource(triage.main) + call = source.index("post_english_notes(") + guard = source.rindex("if needs_discord", 0, call) + self.assertNotIn("\n ", source[guard:call].rstrip()) + + def test_the_transcript_uses_the_same_model_as_the_classification(self): + seen = self.run_main([finding(1)], {}) + self.assertEqual([a[4] for a in seen], [claude_cli.MODEL]) def test_a_rehearsal_writes_no_transcript(self): - seen = [] - with tempfile.TemporaryDirectory() as directory: - path = os.path.join(directory, "findings.json") - with open(path, "w", encoding="utf-8") as handle: - json.dump([finding(1)], handle) - with Patched(triage, attach_english=lambda *a: seen.append(a[5]), - post_to_discord=lambda session, url, messages: len(messages)), \ - mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", - "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y", - "ZENDESK_ENGLISH_FIELD_ID": "42", - "SESSION_OPS_REHEARSAL": "1"}), \ - contextlib.redirect_stdout(io.StringIO()): - triage.main(["--findings", path]) - self.assertEqual(seen, [None]) + self.assertEqual(self.run_main([finding(1)], {"SESSION_OPS_REHEARSAL": "1"}), []) def test_unflagged_tickets_are_translated_too(self): """A ticket with no card of its own is still opened from the digest or the queue, and whoever opens it should read it in English.""" - seen = [] - with tempfile.TemporaryDirectory() as directory: - path = os.path.join(directory, "findings.json") - with open(path, "w", encoding="utf-8") as handle: - json.dump([finding(1), finding(2, worth_looking_into=False, language="German")], - handle) - with Patched(triage, attach_english=lambda *a: seen.extend(f["id"] for f in a[3]), - post_to_discord=lambda session, url, messages: len(messages)), \ - mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", - "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y"}), \ - contextlib.redirect_stdout(io.StringIO()): - triage.main(["--findings", path]) - self.assertEqual(sorted(seen), [1, 2]) - - def said(self, **kwargs): - """attach_english's console output for one call.""" - out = io.StringIO() - defaults = dict(session=object(), subdomain="acme", tickets=[], findings=[], - model="claude-sonnet-5", field_id=42) - defaults.update(kwargs) - with contextlib.redirect_stdout(out): - transcript.attach_english(**defaults) - return out.getvalue() - - def test_a_run_that_wrote_nothing_says_so(self): - """The count printed only when it was non-zero, so a run that wrote nothing - and a run that never reached this step read identically in the journal β€” the - one thing somebody checking whether the feature is on needs to tell apart.""" - with Patched(transcript, conversation_turns=lambda *a: None): - said = self.said(tickets=[{"id": 1}], - findings=[{"id": 1, "language": "German"}]) - self.assertIn("0 of 1", said) - - def test_an_unset_field_id_says_which_variable_is_missing(self): - said = self.said(field_id=None, findings=[{"id": 1, "language": "German"}]) - self.assertIn(transcript.ENGLISH_FIELD_ENV, said) - - def test_an_all_english_digest_says_so_rather_than_nothing(self): - said = self.said(findings=[{"id": 1, "language": "English"}, - {"id": 2, "language": "English"}]) - self.assertIn("No non-English tickets", said) - self.assertIn("2", said) - - def test_a_ticket_that_was_classified_but_never_fetched_is_named(self): - said = self.said(tickets=[], findings=[{"id": 99, "language": "German"}]) - self.assertIn("99", said) - self.assertIn("never fetched", said) + seen = self.run_main([finding(1), finding(2, worth_looking_into=False, + language="German")], {}) + self.assertEqual(sorted(f["id"] for f in seen[0][3]), [1, 2]) + + def test_a_non_english_ticket_gets_a_private_note(self): + got, notes, _ = self.run_notes([{"id": 1}, {"id": 2}], + [{"id": 1, "language": "German"}, + {"id": 2, "language": "English"}]) + self.assertEqual(got, 1) + (args, kwargs), = notes + self.assertEqual(args[2], 1) + self.assertIs(kwargs["public"], False) + self.assertIn("It does not work.", args[3]) + self.assertIn("Customer:", args[3]) + + def test_the_note_is_the_one_claude_english_writes(self): + """Same note, same marker: a transcript either one wrote stops the other + repeating it until somebody writes again.""" + _, notes, _ = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], + comments=[{"id": 5, "public": True, "body": "x"}]) + self.assertIn(note_reply.english_marker(5), notes[0][0][3]) + + def test_a_ticket_already_transcribed_is_not_re_rendered(self): + calls = [] + comments = [{"id": 6, "public": False, "body": note_reply.english_marker(5)}, + {"id": 5, "public": True, "body": "x"}] + got, notes, _ = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], + comments=comments, + translated=lambda *a, **k: calls.append(a)) + self.assertEqual((got, notes, calls), (0, [], [])) + + def test_a_conversation_that_turns_out_english_gets_no_note(self): + got, notes, _ = self.run_notes( + [{"id": 1}], [{"id": 1, "language": "German"}], + translated=lambda *a, **k: {"turns": [{"index": 0, "english": "Es geht nicht."}]}) + self.assertEqual((got, notes), (0, [])) - def test_a_ticket_with_no_public_comments_is_named(self): - with Patched(transcript, conversation_turns=lambda *a: None): - said = self.said(tickets=[{"id": 7}], - findings=[{"id": 7, "language": "German"}]) - self.assertIn("7", said) - self.assertIn("no public comments", said) + def test_nothing_happens_without_a_zendesk_session(self): + """--findings never builds one: the findings already exist and no ticket was + ever fetched, so there is nothing to read comments from or write back to.""" + got, notes, _ = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], + session=None) + self.assertEqual((got, notes), (0, [])) def test_one_ticket_that_cannot_be_rendered_does_not_stop_the_digest(self): """claude_cli_json exits on a failed call, which is right for the @@ -2031,51 +1971,45 @@ def test_one_ticket_that_cannot_be_rendered_does_not_stop_the_digest(self): def explode(*args, **kwargs): raise SystemExit("claude exited 1") - written = [] - with Patched(transcript, - conversation_turns=lambda *a: [ - {"index": 0, "who": "Customer", "when": "", "body": "x"}], - claude_cli_json=explode, - write_english_field=lambda *a: written.append(a) or True): - got = transcript.attach_english( - object(), "acme", [{"id": 1}, {"id": 2}], - [{"id": 1, "language": "German"}, {"id": 2, "language": "German"}], - "claude-sonnet-5", field_id=42) - self.assertEqual(got, 0) - self.assertEqual(written, []) - - def test_the_transcript_is_written_to_the_configured_field(self): - puts = [] - with Patched(transcript, - conversation_turns=lambda *a: [ - {"index": 0, "who": "Customer", "when": "2026-08-28 00:22 UTC", - "body": "Es geht nicht."}], - claude_cli_json=lambda *a, **k: { - "turns": [{"index": 0, "english": "It does not work."}]}, - write_english_field=lambda *a: puts.append(a) or True): - got = transcript.attach_english( - object(), "acme", [{"id": 1}, {"id": 2}], - [{"id": 1, "language": "German"}, {"id": 2, "language": "English"}], - "claude-sonnet-5", field_id=42) - self.assertEqual(got, 1) - # The English ticket was never touched; the German one carries the transcript. - self.assertEqual([(p[2], p[3]) for p in puts], [(1, 42)]) - self.assertIn("Customer:", puts[0][4]) - self.assertIn("It does not work.", puts[0][4]) - - def test_a_ticket_with_no_matching_row_is_skipped(self): - """--findings and a partial fetch both leave findings whose ticket was never - loaded. There is nothing to read comments from, so there is nothing to do.""" - with Patched(transcript, - conversation_turns=lambda *a: [ - {"index": 0, "who": "Customer", "when": "", "body": "x"}], - claude_cli_json=lambda *a, **k: {"turns": []}, - write_english_field=lambda *a: True): - self.assertEqual( - transcript.attach_english(object(), "acme", [], - [{"id": 99, "language": "German"}], - "claude-sonnet-5", field_id=42), - 0) + got, notes, out = self.run_notes([{"id": 1}, {"id": 2}], + [{"id": 1, "language": "German"}, + {"id": 2, "language": "German"}], + translated=explode) + self.assertEqual((got, notes), (0, [])) + self.assertIn("claude exited 1", out) + + def test_a_rejected_note_does_not_stop_the_digest(self): + def reject(*a, **k): + raise SystemExit("Zendesk rejected the note on #1 (422).") + + with Patched(note_reply, write_to_ticket=reject), \ + Patched(api, fetch_comments=lambda *a, **k: [], + conversation_turns=lambda *a: self.GERMAN_TURNS), \ + Patched(claude_cli, claude_cli_json=lambda *a, **k: { + "turns": [{"index": 0, "english": "It does not work."}]}), \ + contextlib.redirect_stdout(io.StringIO()): + self.assertEqual(note_reply.post_english_notes( + object(), "acme", [{"id": 1}], [{"id": 1, "language": "German"}], "m"), 0) + + def test_a_run_that_wrote_nothing_says_so(self): + """A run that wrote nothing and a run that never reached this step must read + differently in the journal.""" + _, _, out = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], + translated=lambda *a, **k: { + "turns": [{"index": 0, "english": "Es geht nicht."}]}) + self.assertIn("0 of 1", out) + + def test_an_all_english_digest_says_so_rather_than_nothing(self): + _, _, out = self.run_notes([], [{"id": 1, "language": "English"}, + {"id": 2, "language": "English"}]) + self.assertIn("No non-English tickets", out) + self.assertIn("2", out) + + def test_a_ticket_that_was_classified_but_never_fetched_is_named(self): + _, notes, out = self.run_notes([], [{"id": 99, "language": "German"}]) + self.assertEqual(notes, []) + self.assertIn("99", out) + self.assertIn("never fetched", out) class TestCustomerSide(unittest.TestCase): From 5077f6e2dfb4258c539450aeb2674ee69706923b Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 14 Oct 2026 14:56:55 +1100 Subject: [PATCH 05/12] fix: address the review of the digest and sogs-ban changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Post the English notes after the digest and its state, so a slow translation cannot hold up or time out the post; skip abuse reports. - One πŸ’€ line per category, each with its own link budget, and record only the tickets actually linked: one shared budget let a busy category hide a rare ticket and still mark it reported. - Guard each ticket's whole note step, so no error on one ticket reaches the digest. - A room that cannot be reached is listed as unread rather than aborting sogs-ban; SogsError moves to its own module, ending the ban/names cycle. - Drop references to the removed field code and document the πŸ’€ lines. Claude-Session: https://claude.ai/code/session_019cnvrsCXsqLykDyQburD2f --- docs/jobs/zendesk-digest.md | 8 +- src/session_ops/sogs/ban.py | 8 +- src/session_ops/sogs/errors.py | 2 + src/session_ops/sogs/names.py | 27 +++--- src/session_ops/zendesk/note_reply.py | 43 +++++---- src/session_ops/zendesk/transcript.py | 4 +- src/session_ops/zendesk/triage.py | 67 ++++++-------- tests/sogs/test_names.py | 32 +++++-- tests/zendesk/test_api.py | 2 +- tests/zendesk/test_note_reply.py | 4 +- tests/zendesk/test_triage.py | 123 ++++++++++++++++++++------ 11 files changed, 203 insertions(+), 117 deletions(-) create mode 100644 src/session_ops/sogs/errors.py diff --git a/docs/jobs/zendesk-digest.md b/docs/jobs/zendesk-digest.md index b50fa84..5027b10 100644 --- a/docs/jobs/zendesk-digest.md +++ b/docs/jobs/zendesk-digest.md @@ -45,13 +45,13 @@ The star-floor machinery (`partition_reviews`, `--review-star-floor`, `--include ## What else is out of scope - **`pending` tickets.** Somebody already replied and the ball is with the customer; the "Pending to Solved" automation resolves them after 72h. The query is `status`, and `updated_at` moves on any change β€” a tag edit, the hourly automation, and every private note the `claude:` commands write. So the run drops anything whose `requester_updated_at` falls outside the window. Measured on a real 72h window: 79 fetched, 23 the requester had actually touched. +- **Tickets only we touched.** The window is on `updated>`, and `updated_at` moves on any change β€” a tag edit, the hourly automation, and every private note the `claude:` commands and the digest's own English notes write. So the run drops anything whose `requester_updated_at` falls outside the window. Measured on a real 72h window: 79 fetched, 23 the requester had actually touched. ## Content-free tickets Twitter DM tickets arrive with `description` identical to `subject` β€” both just `"Conversation with "` β€” which is 15% of non-review tickets and unclassifiable as fetched. For those only, `hydrate_descriptions` fetches a page of up to 10 comments and joins every body that differs from the subject into the description; later replies often carry the actual detail. Hydration is an enrichment, so an HTTP error or an unreachable endpoint leaves the ticket as-is rather than failing the run (`--no-hydrate` to skip it entirely). -The script (`zendesk-triage`, [triage.py](../../src/session_ops/zendesk/triage.py)) fetches the tickets in a rolling time window, classifies the whole batch in one schema-enforced request through the Claude Code CLI, and posts a Discord digest: a short header, then one line per ticket worth looking into. +The script (`zendesk-triage`, [triage.py](../../src/session_ops/zendesk/triage.py)) fetches the tickets in a rolling time window, classifies the whole batch in one schema-enforced request through the Claude Code CLI, and posts a Discord digest: a short header, one line per ticket worth looking into, then the abuse reports and the unflagged tickets. Each line leads with a severity marker, a category emoji and a platform icon, links the ticket id, and carries the model's one-line summary plus its root-cause guess: @@ -66,6 +66,8 @@ Likely duplicates: **push-notifications-not-delivered** Γ—5 (#27637, #27610, #27 🟠 | ⭐ | 🍎 | #27610 Β· Messages not delivered for days; nothing shows even after opening | Likely cause: Push notification delivery / message retrieval failure 🟠 | πŸ› | πŸ€– | πŸ”„ #27605 Β· Message and call notifications only appear when the app is opened | Likely cause: Push notification service failure on Android πŸ”‡ **2** abuse reports β€” reported Session IDs, nothing actionable (#27640, #27641) +πŸ’€ πŸ”‘ **2** not flagged β€” #27650, #27652 +πŸ’€ ❓ **1** not flagged β€” #27648 ``` | Column | Values | @@ -74,7 +76,7 @@ Likely duplicates: **push-notifications-not-delivered** Γ—5 (#27637, #27610, #27 | Category | The emoji from `CATEGORY_SPECS`, so it matches the tally line | | Platform | πŸ€– Android Β· 🍎 iOS Β· πŸ–₯️ desktop (all three) Β· 🌐 multiple Β· ❔ unknown | -The header accounts for the batch in full, so nothing is dropped silently. The backlog line is scoped exactly like the analysis β€” `status/messages/recent returns. RECENT_LIMIT = 256 @@ -77,25 +76,29 @@ def display_name(data_b64): return name.decode('utf-8', 'replace') or None -def recent_names(client, rooms, session_ids): - """Looks each 05 id up in every room's recent messages. +def recent_names(client, rooms, owners): + """Looks accounts up in every room's recent messages. - Returns ({id: name}, [(room token, status)]). An id that posted is in the dict, with - the name from its newest post (None if that post carried none); an id that did not - is absent. The list names the rooms that could not be read, so a missing id is not - mistaken for one that never posted. + `owners` maps each id form an account can post under (05 and both blinded 15 + forms) to the 05 id to report it as. Returns ({id: name}, [(room token, why)]). + An id that posted is in the dict, with the name from its newest post (None if + that post carried none); an id that did not is absent. The list names the rooms + that could not be read, so a missing id is not mistaken for one that never + posted. Never raises: the lookup is informational and must not stop a ban. """ - owner = {b: sid for sid in session_ids for b in ban.blinded_ids(sid, client.server_pubkey)} - owner.update({sid: sid for sid in session_ids}) newest, unread = {}, [] for room in rooms: token = room['token'] - code, messages = client.request('GET', f'/room/{token}/messages/recent?limit={RECENT_LIMIT}') + try: + code, messages = client.request('GET', f'/room/{token}/messages/recent?limit={RECENT_LIMIT}') + except SogsError as e: + unread.append((token, str(e))) + continue if code != 200 or not isinstance(messages, list): unread.append((token, code)) continue for m in messages: - sid = owner.get(m.get('session_id')) + sid = owners.get(m.get('session_id')) # A deleted message stays in the list with its data nulled. if not sid or not m.get('data'): continue diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index fb8b616..5f8511f 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -293,11 +293,9 @@ def quoted_para(text): def transcript_blocks(turns, translated): """One turn at a time, as readable paragraphs. - Deliberately not transcript.render_transcript's output re-split on blank lines: a - turn whose own text contains a blank line gets torn into several pieces that - way, which is what made the first version render as a row of disconnected code - boxes. The speaker line leads each turn and the body follows as prose β€” nothing - extracts this text, so it wants readability, not byte fidelity. + One speaker line per turn; a turn's own blank lines split its paragraphs, never + the turn. The body is prose, not a code box: nothing extracts this text, so it + wants readability, not byte fidelity. """ english = {} for item in translated or []: @@ -1093,6 +1091,22 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): public=False, as_html=True, drop_tags=[TAG_QUEUED, TAG_ERROR]) +def post_english_note(session, subdomain, model, ticket): + """One ticket's note, if it needs one. Returns 1 if a note was written, else 0.""" + comments = api.fetch_comments(session, subdomain, ticket["id"], optional=True) + if comments is None: + return 0 + outcome, detail = translate_conversation(session, subdomain, model, ticket, comments) + if outcome != "ready": + print(f"#{ticket['id']}: no English note ({outcome}" + + (f": {detail})" if outcome == "failed" else ")")) + return 0 + turns, rendered, latest = detail + write_to_ticket(session, subdomain, ticket["id"], english_note(turns, rendered, latest), + public=False, as_html=True) + return 1 + + def post_english_notes(session, subdomain, tickets, findings, model): """The digest's `claude: english`, unasked: the same private note on every ticket in the run the classifier did not call English. @@ -1119,23 +1133,14 @@ def post_english_notes(session, subdomain, tickets, findings, model): print(f"Note: #{finding.get('id')} was classified {finding.get('language')!r} " f"but never fetched; no English note.") continue - comments = api.fetch_comments(session, subdomain, ticket["id"], optional=True) - if comments is None: - continue - outcome, detail = translate_conversation(session, subdomain, model, ticket, comments) - if outcome != "ready": - print(f"#{ticket['id']}: no English note ({outcome}" - + (f": {detail})" if outcome == "failed" else ")")) - continue - turns, rendered, latest = detail try: - write_to_ticket(session, subdomain, ticket["id"], - english_note(turns, rendered, latest), - public=False, as_html=True) + written += post_english_note(session, subdomain, model, ticket) except (SystemExit, requests.RequestException) as exc: print(f"Note: could not post the English note on #{ticket['id']} ({exc}).") - continue - written += 1 + except Exception as exc: # noqa: BLE001 - one ticket must not take the digest down + # The type only: the message of an unexpected error may carry ticket text. + print(f"Note: the English note on #{ticket['id']} failed " + f"({type(exc).__name__}).") # Printed even at zero, so a run that wrote nothing reads differently in the # journal from one that never reached this step. print(f"Posted an English note on {written} of {len(candidates)} non-English ticket(s).") diff --git a/src/session_ops/zendesk/transcript.py b/src/session_ops/zendesk/transcript.py index efcc3c4..ca1da39 100644 --- a/src/session_ops/zendesk/transcript.py +++ b/src/session_ops/zendesk/transcript.py @@ -55,8 +55,8 @@ def is_english(finding): Unknown counts as English: the note is only worth posting when it says something the agent cannot already read, and a blank `language` is far more likely to be a classification that came back thin than a ticket nobody could - read. Guessing wrong this way costs a transcript nobody needed; the other way - puts a machine translation over the top of words everyone could already read. + read. Guessing wrong this way costs a missing transcript; the other way puts a + machine translation over the top of words everyone could already read. """ language = (finding.get("language") or "").strip().lower() return not language or language.startswith(("english", "en")) diff --git a/src/session_ops/zendesk/triage.py b/src/session_ops/zendesk/triage.py index b11e431..68c6117 100644 --- a/src/session_ops/zendesk/triage.py +++ b/src/session_ops/zendesk/triage.py @@ -506,7 +506,7 @@ def hydrate_descriptions(session, subdomain, tickets): return hydrated -# ---- English transcript, for the reply dialog ------------------------------- +# ---- The classification request --------------------------------------------- def compact_ticket(ticket): @@ -670,8 +670,7 @@ def analyze(model, effort, compact_tickets): # Character budget for the links on that line, so a day of nothing but abuse reports # can't push it past MAX_MESSAGE_CHARS. Past it the rest are counted, not linked. COLLAPSED_LINKS_CHARS = 900 -# Every other ticket the model did not flag goes on one line under it, linked and -# grouped by category: counted only in the tally, a ticket left the digest unseen. +# Every other classified ticket, linked once, one line per category under it. UNFLAGGED_MARKER = "πŸ’€" # ---- Components V2 --------------------------------------------------------- @@ -759,34 +758,35 @@ def build_collapsed_line(collapsed, subdomain): f"actionable ({', '.join(links)})") -def build_unflagged_line(unflagged, subdomain): - """The tickets nobody is asked to look at, as one line under the abuse reports: +def build_unflagged_lines(unflagged, subdomain): + """The tickets nobody is asked to look at: one (line, linked ids) per category, + most common first, under the abuse reports. - πŸ’€ **5** not flagged β€” πŸ”‘ #28891, #28880 Β· ❓ #28870, #28866 Β· πŸ’‘ #28900 + πŸ’€ πŸ”‘ **2** not flagged β€” #28891, #28880 - Grouped by category, most common first, so the emoji says what each run is - without a summary per ticket. Links share COLLAPSED_LINKS_CHARS the same way, and - the count always covers every one of them. + Each category has its own COLLAPSED_LINKS_CHARS, so a busy category cannot push + a rarer one off the digest. Only linked tickets are returned as covered: one past + the budget stays eligible for the next run rather than being recorded unseen. """ by_category = {} for f in unflagged: by_category.setdefault(f.get("category"), []).append(f) - groups, used, unlisted = [], 0, 0 + out = [] for category, members in sorted(by_category.items(), key=lambda kv: -len(kv[1])): - links = [] + links, linked, used = [], set(), 0 for f in members: link = f"[#{f.get('id')}]({ticket_url(subdomain, f.get('id'))})" used += len(link) + 2 if used > COLLAPSED_LINKS_CHARS: - unlisted += 1 - continue + break links.append(link) - if links: - groups.append(f"{CATEGORY_EMOJI.get(category, 'β€’')} {', '.join(links)}") - if unlisted: - groups.append(f"+{unlisted} more") - return (f"{UNFLAGGED_MARKER} **{len(unflagged)}** not flagged β€” " - + " Β· ".join(groups)) + linked.add(f.get("id")) + unlisted = len(members) - len(links) + if unlisted: + links.append(f"+{unlisted} more") + out.append((f"{UNFLAGGED_MARKER} {CATEGORY_EMOJI.get(category, 'β€’')} " + f"**{len(members)}** not flagged β€” {', '.join(links)}", linked)) + return out def build_header(findings, highlights, stats=None): @@ -895,13 +895,9 @@ def build_messages(findings, subdomain, stats=None, updated_ids=None): collapsed = [f for f in findings if is_collapsed(f)] collapsed_ids = {f.get("id") for f in collapsed} unflagged = [f for f in findings if f.get("id") not in shown_ids | omitted_ids | collapsed_ids] - unflagged_ids = {f.get("id") for f in unflagged} # The highlights that didn't fit are covered by no message and stay eligible next - # run. The two summary lines account for every ticket they count, links or not, - # so re-reporting those next run would be noise. - header_ids = ({f.get("id") for f in findings} - - shown_ids - omitted_ids - collapsed_ids - unflagged_ids) + # run. The abuse-report line accounts for every report it counts, links or not. header = build_header(findings, shown + omitted, stats) if omitted: header += (f"\nShowing the top **{len(shown)}** of " @@ -914,13 +910,10 @@ def build_messages(findings, subdomain, stats=None, updated_ids=None): # Last, under the individual tickets: lowest position for the lowest priority. if collapsed: entries.append((build_collapsed_line(collapsed, subdomain), collapsed_ids)) - if unflagged: - entries.append((build_unflagged_line(unflagged, subdomain), unflagged_ids)) + entries += build_unflagged_lines(unflagged, subdomain) - messages, coverage = discord.messages_from_entries( + return discord.messages_from_entries( header, entries, MAX_ENTRIES_PER_MESSAGE, MAX_COMPONENT_CHARS) - coverage[0] |= header_ids - return messages, coverage def main(argv=None): @@ -1116,15 +1109,6 @@ def main(argv=None): print(f"{len(findings)} tickets classified; {len(shown) + len(omitted)} worth looking into" + (f" ({len(omitted)} beyond the display cap)." if omitted else ".")) - # Every classified ticket, not just the highlights: whoever opens one from any - # line of the digest, or straight from the queue, reads it in English. - # - # Gated on the post actually happening, not just on --dry-run: --no-discord is - # the flag CI runs with, and neither CI nor a rehearsal may write to production - # tickets. - if needs_discord and not rehearsing(): - post_english_notes(zd, subdomain, classified, findings, MODEL) - messages, coverage = build_messages(findings, subdomain, stats, updated_ids) if args.dry_run: print(json.dumps(messages, indent=2, ensure_ascii=False)) @@ -1159,6 +1143,13 @@ def main(argv=None): print(f"Recorded {len(recorded)} tickets; state now tracks {kept} " f"({pruned} pruned beyond {args.state_retention_days} days).") + # After the post and the state, so a slow translation can never hold up or lose + # the digest. Abuse reports are left out: nobody acts on them. Only on a run that + # posts: neither CI (--no-discord) nor a rehearsal may write to production tickets. + if not rehearsing(): + post_english_notes(zd, subdomain, classified, + [f for f in findings if not is_collapsed(f)], MODEL) + if posted < len(messages): sys.exit(f"Aborted after {posted}/{len(messages)} messages; " f"undelivered tickets stay eligible for the next run.") diff --git a/tests/sogs/test_names.py b/tests/sogs/test_names.py index f1e069f..a776d5b 100644 --- a/tests/sogs/test_names.py +++ b/tests/sogs/test_names.py @@ -8,6 +8,7 @@ from nacl.signing import SigningKey from session_ops.sogs import ban, names +from session_ops.sogs.errors import SogsError from tests.sogs.test_ban import BLINDED_ABS, SERVER_PUBKEY, SESSION_ID OTHER_ID = ban.session_id_of(SigningKey(bytes(range(32)))) @@ -39,14 +40,20 @@ def post(sid, posted, data): return {'session_id': sid, 'posted': posted, 'data': data} +def owners(*sids): + pub = bytes.fromhex(SERVER_PUBKEY) + return {form: sid for sid in sids for form in (sid, *ban.blinded_ids(sid, pub))} + + class FakeClient: def __init__(self, by_room): - self.server_pubkey = bytes.fromhex(SERVER_PUBKEY) self.by_room = by_room def request(self, method, path, body_json=None): - token = path.split('/')[2] - return self.by_room[token] + answer = self.by_room[path.split('/')[2]] + if isinstance(answer, Exception): + raise answer + return answer class TestDisplayName(unittest.TestCase): @@ -71,26 +78,37 @@ def test_the_newest_post_names_the_account(self): 'a': (200, [post(BLINDED_ABS, 10, message_data('Old')), post(BLINDED_ABS, 30, message_data('New'))]), 'b': (200, [post(BLINDED_ABS, 20, message_data('Middle'))]), }) - found, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], [SESSION_ID]) + found, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], owners(SESSION_ID)) self.assertEqual(found, {SESSION_ID: 'New'}) self.assertEqual(unread, []) def test_an_account_that_did_not_post_is_absent(self): client = FakeClient({'a': (200, [post(BLINDED_ABS, 10, message_data('Someone'))])}) - found, _ = names.recent_names(client, [{'token': 'a'}], [OTHER_ID]) + found, _ = names.recent_names(client, [{'token': 'a'}], owners(OTHER_ID)) self.assertEqual(found, {}) def test_a_deleted_post_is_skipped(self): client = FakeClient({'a': (200, [post(BLINDED_ABS, 10, None)])}) - found, _ = names.recent_names(client, [{'token': 'a'}], [SESSION_ID]) + found, _ = names.recent_names(client, [{'token': 'a'}], owners(SESSION_ID)) self.assertEqual(found, {}) def test_an_unreadable_room_is_reported(self): client = FakeClient({'a': (403, 'nope'), 'b': (200, [])}) - _, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], [SESSION_ID]) + _, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], owners(SESSION_ID)) self.assertEqual(unread, [('a', 403)]) +class TestUnreachableRoom(unittest.TestCase): + def test_a_room_that_times_out_is_reported_not_raised(self): + """The lookup is informational; a timeout on one room must not stop the ban.""" + client = FakeClient({'a': SogsError('GET /room/a failed: timed out'), + 'b': (200, [post(BLINDED_ABS, 10, message_data('Mallory'))])}) + found, unread = names.recent_names(client, [{'token': 'a'}, {'token': 'b'}], + owners(SESSION_ID)) + self.assertEqual(found, {SESSION_ID: 'Mallory'}) + self.assertEqual([t for t, _ in unread], ['a']) + + class TestDescribePoster(unittest.TestCase): def test_each_case_reads_differently(self): self.assertEqual(ban.describe_poster(SESSION_ID, {}), "no recent posts") diff --git a/tests/zendesk/test_api.py b/tests/zendesk/test_api.py index 11b0d8f..debe004 100644 --- a/tests/zendesk/test_api.py +++ b/tests/zendesk/test_api.py @@ -15,7 +15,7 @@ class TestFetchUser(unittest.TestCase): def test_an_unreachable_zendesk_is_an_unknown_author_not_an_error(self): - """attach_english promises never to raise; an unknown author counts as a customer.""" + """post_english_notes promises never to raise; an unknown author counts as a customer.""" self.assertEqual(api.fetch_user(FakeSession([requests.ConnectionError("down")]), "acme", 5), {}) diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index bf50ea4..54fec33 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -224,8 +224,8 @@ def test_whitespace_and_case_do_not_count_as_a_translation(self): turns, [{"index": 0, "english": "my app crashes"}])) def test_a_turn_with_blank_lines_stays_one_turn(self): - """The first version split render_transcript's output on blank lines, which - tore a single multi-paragraph message into a row of disconnected boxes.""" + """A multi-paragraph message is one turn with several paragraphs, never + several turns.""" turns = [{"index": 0, "who": "Customer", "when": "2026-09-03 10:00 UTC", "body": "Hallo,\n\nzweiter Absatz.\n\ndritter Absatz."}] blocks = note_reply.transcript_blocks(turns, []) diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index 8a0fe46..2fb36f0 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -9,7 +9,6 @@ tests drive fetch_tickets with a stub session instead. """ import contextlib -import inspect import io import json import os @@ -721,6 +720,10 @@ class TestUnflaggedLine(unittest.TestCase): def quiet(self, ticket_id, category="account_access"): return finding(ticket_id, category=category, worth_looking_into=False) + def lines(self, findings): + return [l for l in digest_text(build_messages(findings, "acme")).splitlines() + if "not flagged" in l] + def test_an_unflagged_ticket_is_linked(self): text = digest_text(build_messages([finding(1), self.quiet(2)], "acme")) self.assertIn("[#2](https://acme.zendesk.com/agent/tickets/2)", text) @@ -734,40 +737,44 @@ def test_it_sits_below_the_abuse_reports(self): self.assertIn("abuse report", lines[-2]) self.assertIn("not flagged", lines[-1]) - def test_tickets_are_grouped_by_category_most_common_first(self): - findings = [self.quiet(1, "question"), self.quiet(2, "account_access"), - self.quiet(3, "account_access")] - line = triage.build_unflagged_line(findings, "acme") - recovery, question = (triage.CATEGORY_EMOJI[c] for c in ("account_access", "question")) - self.assertLess(line.index(recovery), line.index(question)) - self.assertEqual(line.count(recovery), 1) + def test_one_line_per_category_most_common_first(self): + findings = [self.quiet(1, "question"), self.quiet(2), self.quiet(3)] + lines = self.lines(findings) + self.assertEqual(len(lines), 2) + self.assertIn(triage.CATEGORY_EMOJI["account_access"] + " **2**", lines[0]) + self.assertIn(triage.CATEGORY_EMOJI["question"] + " **1**", lines[1]) + + def test_a_busy_category_cannot_hide_a_rare_one(self): + """One shared budget let fifteen questions push the lone recovery ticket into + "+N more", which was then recorded as reported without ever being shown.""" + findings = [self.quiet(28000 + i, "question") for i in range(15)] + [self.quiet(28891)] + messages, coverage = triage.build_messages(findings, "acme") + self.assertIn("[#28891]", digest_text(messages)) + self.assertIn(28891, set().union(*coverage)) + + def test_a_ticket_past_the_budget_is_not_recorded(self): + findings = [self.quiet(27000 + i) for i in range(200)] + messages, coverage = triage.build_messages(findings, "acme") + covered, text = set().union(*coverage), digest_text(messages) + for f in findings: + self.assertEqual(f["id"] in covered, f"[#{f['id']}]" in text, f["id"]) + self.assertIn("**200** not flagged", text) + self.assertRegex(text, r"\+\d+ more") def test_abuse_reports_and_highlights_are_not_repeated_on_it(self): findings = [finding(1), finding(2, category="abuse_report", severity="not_applicable"), self.quiet(3)] - line = [l for l in digest_text(build_messages(findings, "acme")).splitlines() - if "not flagged" in l][0] + line, = self.lines(findings) self.assertNotIn("[#1]", line) self.assertNotIn("[#2]", line) - self.assertIn("**1** not flagged", line) def test_highlights_beyond_the_cap_stay_off_it(self): """Those stay eligible for the next run rather than being marked seen.""" findings = [finding(i, priority_rank=i) for i in range(triage.MAX_HIGHLIGHTS + 2)] - self.assertNotIn("not flagged", digest_text(build_messages(findings, "acme"))) - - def test_it_covers_the_tickets_it_counts(self): - messages, coverage = triage.build_messages([finding(1), self.quiet(2)], "acme") - self.assertIn(2, set().union(*coverage)) - - def test_a_long_run_of_links_is_clipped_but_fully_counted(self): - line = triage.build_unflagged_line([self.quiet(27000 + i) for i in range(200)], "acme") - self.assertLess(len(line), triage.MAX_MESSAGE_CHARS) - self.assertIn("**200** not flagged", line) - self.assertRegex(line, r"\+\d+ more$") + self.assertEqual(self.lines(findings), []) def test_no_line_when_every_ticket_is_flagged(self): - self.assertNotIn("not flagged", digest_text(build_messages([finding(1)], "acme"))) + self.assertEqual(self.lines([finding(1)]), []) # ---- Parsing helpers ------------------------------------------------------- @@ -1960,11 +1967,39 @@ def run_main(self, findings, env): def test_a_run_that_posts_nothing_writes_nothing(self): """--no-discord is the flag CI runs with, and CI must not write to production - tickets. Guarded at the call site by `needs_discord`.""" - source = inspect.getsource(triage.main) - call = source.index("post_english_notes(") - guard = source.rindex("if needs_discord", 0, call) - self.assertNotIn("\n ", source[guard:call].rstrip()) + tickets: the notes come after the post, past every early return.""" + for flag in ("--no-discord", "--dry-run"): + with self.subTest(flag=flag): + seen = [] + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "findings.json") + with open(path, "w", encoding="utf-8") as handle: + json.dump([finding(1, language="German")], handle) + with Patched(triage, post_english_notes=lambda *a: seen.append(a)), \ + mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme"}), \ + contextlib.redirect_stdout(io.StringIO()): + triage.main(["--findings", path, flag]) + self.assertEqual(seen, []) + + def test_the_notes_come_after_the_post(self): + """A slow translation must never hold up, or time out, the digest.""" + order = [] + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "findings.json") + with open(path, "w", encoding="utf-8") as handle: + json.dump([finding(1, language="German")], handle) + with Patched(triage, post_english_notes=lambda *a: order.append("notes"), + post_to_discord=lambda s, u, m: order.append("post") or len(m)), \ + mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", + "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y"}), \ + contextlib.redirect_stdout(io.StringIO()): + triage.main(["--findings", path]) + self.assertEqual(order, ["post", "notes"]) + + def test_abuse_reports_are_not_translated(self): + seen = self.run_main([finding(1, language="German"), + finding(2, category="abuse_report", language="German")], {}) + self.assertEqual([f["id"] for f in seen[0][3]], [1]) def test_the_transcript_uses_the_same_model_as_the_classification(self): seen = self.run_main([finding(1)], {}) @@ -2047,6 +2082,38 @@ def reject(*a, **k): self.assertEqual(note_reply.post_english_notes( object(), "acme", [{"id": 1}], [{"id": 1, "language": "German"}], "m"), 0) + def test_comments_that_cannot_be_fetched_skip_the_ticket(self): + calls = [] + with Patched(api, fetch_comments=lambda *a, **k: None), \ + Patched(claude_cli, claude_cli_json=lambda *a, **k: calls.append(a)), \ + contextlib.redirect_stdout(io.StringIO()): + got = note_reply.post_english_notes(object(), "acme", [{"id": 1}], + [{"id": 1, "language": "German"}], "m") + self.assertEqual((got, calls), (0, [])) + + def test_no_failure_on_one_ticket_reaches_the_digest(self): + """A network error on the write, and anything unforeseen, are both logged and + skipped; the next ticket is still translated.""" + for error in (requests.ConnectionError("down"), KeyError("turns")): + with self.subTest(error=type(error).__name__): + writes = [] + + def write(*a, **k): + writes.append(a[2]) + if a[2] == 1: + raise error + + with Patched(note_reply, write_to_ticket=write), \ + Patched(api, fetch_comments=lambda *a, **k: [], + conversation_turns=lambda *a: self.GERMAN_TURNS), \ + Patched(claude_cli, claude_cli_json=lambda *a, **k: { + "turns": [{"index": 0, "english": "It does not work."}]}), \ + contextlib.redirect_stdout(io.StringIO()): + got = note_reply.post_english_notes( + object(), "acme", [{"id": 1}, {"id": 2}], + [{"id": 1, "language": "German"}, {"id": 2, "language": "German"}], "m") + self.assertEqual((got, writes), (1, [1, 2])) + def test_a_run_that_wrote_nothing_says_so(self): """A run that wrote nothing and a run that never reached this step must read differently in the journal.""" From afd4827ef46e976f5bd7f82c686ff3421159cc03 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 15 Oct 2026 12:04:47 +1100 Subject: [PATCH 06/12] feat(zendesk): translate when the customer writes, not in the digest The digest translated only weekday mornings, only tickets in its window, and every translation stretched a job that has a timeout and an alert. A second Zendesk trigger, on the customer writing, now calls the relay with "event": "customer", which runs `note_reply --english-if-needed`: the `claude: english` note when the conversation needs one, never a command, never on an app-store review, journal only otherwise, and covered by RELAY_DRY_RUN like the rest of the relay. An unknown event is a 400 rather than falling through to the commands. The digest is read-only again. post_english_notes and is_english go; recent_names no longer raises on a malformed room. Claude-Session: https://claude.ai/code/session_019cnvrsCXsqLykDyQburD2f --- docs/jobs/zendesk-digest.md | 4 +- docs/jobs/zendesk-relay.md | 19 ++- src/session_ops/sogs/names.py | 19 ++- src/session_ops/zendesk/note_reply.py | 80 ++++----- src/session_ops/zendesk/relay.py | 27 ++- src/session_ops/zendesk/transcript.py | 13 -- src/session_ops/zendesk/triage.py | 10 +- tests/sogs/test_names.py | 14 ++ tests/zendesk/test_api.py | 2 +- tests/zendesk/test_note_reply.py | 85 ++++++++++ tests/zendesk/test_relay.py | 39 ++++- tests/zendesk/test_triage.py | 229 +------------------------- 12 files changed, 220 insertions(+), 321 deletions(-) diff --git a/docs/jobs/zendesk-digest.md b/docs/jobs/zendesk-digest.md index 5027b10..ee6cd3e 100644 --- a/docs/jobs/zendesk-digest.md +++ b/docs/jobs/zendesk-digest.md @@ -10,8 +10,6 @@ Claude reviews the Zendesk tickets awaiting a reply β€” `new` and `open`, no app | Re-run | `systemctl start session-ops@zendesk-digest.service` | | Logs | `journalctl -u session-ops@zendesk-digest -n 50 --no-pager` | -Every non-English ticket it classifies gets the private note `claude: english` writes: the whole conversation, both sides, in English. The two share a marker, so a conversation is translated once per new public comment whichever writes first. `--no-discord`, `--dry-run` and rehearsals write nothing to Zendesk. - ## Categories `CATEGORY_SPECS` in [triage.py](../../src/session_ops/zendesk/triage.py) is the single source of truth β€” the schema enum, the Discord labels and emoji, which categories count as urgent, and the prompt guidance are all derived from it, so adding a category is one edit. @@ -45,7 +43,7 @@ The star-floor machinery (`partition_reviews`, `--review-star-floor`, `--include ## What else is out of scope - **`pending` tickets.** Somebody already replied and the ball is with the customer; the "Pending to Solved" automation resolves them after 72h. The query is `status`, and `updated_at` moves on any change β€” a tag edit, the hourly automation, and every private note the `claude:` commands and the digest's own English notes write. So the run drops anything whose `requester_updated_at` falls outside the window. Measured on a real 72h window: 79 fetched, 23 the requester had actually touched. +- **Tickets only we touched.** The window is on `updated>`, and `updated_at` moves on any change β€” a tag edit, the hourly automation, and every private note the relay writes, the automatic English transcripts included. So the run drops anything whose `requester_updated_at` falls outside the window. Measured on a real 72h window: 79 fetched, 23 the requester had actually touched. ## Content-free tickets diff --git a/docs/jobs/zendesk-relay.md b/docs/jobs/zendesk-relay.md index c2dee79..be1ee5a 100644 --- a/docs/jobs/zendesk-relay.md +++ b/docs/jobs/zendesk-relay.md @@ -46,6 +46,14 @@ there are several. | `claude: explain` | Post what support usually replied to this kind of ticket, what was actually done about it, and the caveats | no | | `claude: solve [reason]` | Solve without writing to the customer, for tickets that need no reply. The note records who decided and why | no comment, but **solving fires the CSAT automation** | +The same transcript is also posted unasked, when the customer writes on a ticket that +is not in English (see the second trigger below). It shares `claude: english`'s marker, +so whichever gets there first stops the other repeating it until there is a new public +comment. It answers nobody: an English conversation, a failed translation or an +app-store review leaves only a line in the journal, and nothing is tagged. It costs one +Claude call per customer message, English ones included: the language is judged from +the translation, because a character test calls unaccented German English. + ## Why a draft is always reviewed The reply flow this replaced sent an English ticket immediately, because the agent had @@ -128,6 +136,14 @@ A trigger, and a webhook it calls: - **Trigger** β€” conditions: *Ticket is Updated*, *Comment is Private*, *Comment text contains `claude:`*, and *Current user is not* the automation user. Actions: notify the webhook, and add the tag `claude-queued`. +- **Second trigger, the customer wrote** β€” conditions: *Comment is Public*, *Current + user is (end-user)*, and *Channel is not* the app-store review integration. Action: + notify the same webhook with `{"ticket_id": "{{ticket.id}}", "event": "customer"}`, + and no tag. It runs `note_reply --english-if-needed`, which never acts on a command. + App-store reviews are refused in the code as well, whatever these conditions say. + +Any other `event` is refused with a 400, so a misconfigured trigger cannot fall +through to the command path. ## Required Secrets @@ -147,4 +163,5 @@ uv run zendesk-note-reply --ticket 27603 --dry-run ``` A ticket with no command note prints `no command note to act on` and stops, so this -is safe to point at anything. +is safe to point at anything. `--english-if-needed --dry-run` shows what the customer +trigger would do. diff --git a/src/session_ops/sogs/names.py b/src/session_ops/sogs/names.py index 2091d71..0ca17cd 100644 --- a/src/session_ops/sogs/names.py +++ b/src/session_ops/sogs/names.py @@ -59,7 +59,7 @@ def display_name(data_b64): not parse.""" try: raw = b64decode(data_b64) - except ValueError: + except (TypeError, ValueError): return None # Clients pad the Content with 0x80 and then zeros. raw = raw.rstrip(b'\x00') @@ -97,11 +97,14 @@ def recent_names(client, rooms, owners): if code != 200 or not isinstance(messages, list): unread.append((token, code)) continue - for m in messages: - sid = owners.get(m.get('session_id')) - # A deleted message stays in the list with its data nulled. - if not sid or not m.get('data'): - continue - if sid not in newest or m['posted'] > newest[sid][0]: - newest[sid] = (m['posted'], display_name(m['data'])) + try: + for m in messages: + sid = owners.get(m.get('session_id')) + # A deleted message stays in the list with its data nulled. + if not sid or not m.get('data'): + continue + if sid not in newest or m['posted'] > newest[sid][0]: + newest[sid] = (m['posted'], display_name(m['data'])) + except (AttributeError, KeyError, TypeError) as e: + unread.append((token, f"unexpected message shape ({type(e).__name__})")) return {sid: name for sid, (_, name) in newest.items()}, unread diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index 5f8511f..35f68f0 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -62,8 +62,6 @@ import sys import textwrap -import requests - from session_ops.shared.discord import clip from session_ops.shared.env import get_env from session_ops.shared.text import undash_english @@ -1091,60 +1089,33 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): public=False, as_html=True, drop_tags=[TAG_QUEUED, TAG_ERROR]) -def post_english_note(session, subdomain, model, ticket): - """One ticket's note, if it needs one. Returns 1 if a note was written, else 0.""" - comments = api.fetch_comments(session, subdomain, ticket["id"], optional=True) - if comments is None: - return 0 +def run_auto_english(session, subdomain, model, ticket, comments, dry_run): + """`claude: english`, unasked, after the customer writes: the same private note, + posted only when the conversation is not English and no transcript covers it yet. + + The note's marker is shared with the command, so whichever writes first stops the + other repeating it until there is a new public comment. Nobody asked, so there is + no one to answer: every outcome other than a note goes to the journal only, and + nothing is tagged. + """ + ticket_id = ticket["id"] + # App-store reviews take one developer response, not a conversation, and they + # are most of the account: refused here whatever the trigger's conditions say. + if api.is_store_review(ticket): + print(f"#{ticket_id}: an app-store review; no English note.") + return outcome, detail = translate_conversation(session, subdomain, model, ticket, comments) if outcome != "ready": - print(f"#{ticket['id']}: no English note ({outcome}" + print(f"#{ticket_id}: no English note ({outcome}" + (f": {detail})" if outcome == "failed" else ")")) - return 0 + return turns, rendered, latest = detail - write_to_ticket(session, subdomain, ticket["id"], english_note(turns, rendered, latest), + print(f"#{ticket_id}: rendered {len(turns)} turn(s) in English.") + if dry_run: + print(f"#{ticket_id}: dry run, nothing written.") + return + write_to_ticket(session, subdomain, ticket_id, english_note(turns, rendered, latest), public=False, as_html=True) - return 1 - - -def post_english_notes(session, subdomain, tickets, findings, model): - """The digest's `claude: english`, unasked: the same private note on every ticket - in the run the classifier did not call English. - - The digest only classifies tickets that are new or changed, and the note's marker - is shared with the command, so a conversation is translated once per new public - comment whichever of the two gets there first. - - Never raises: a digest that fails to post because one translation failed is a - worse trade than one ticket left untranslated. - """ - if not session: - return 0 - candidates = [f for f in findings if not transcript.is_english(f)] - if not candidates: - print(f"No non-English tickets among the {len(findings)} classified; " - f"no English notes to post.") - return 0 - by_id = {t.get("id"): t for t in tickets} - written = 0 - for finding in candidates: - ticket = by_id.get(finding.get("id")) - if not ticket: - print(f"Note: #{finding.get('id')} was classified {finding.get('language')!r} " - f"but never fetched; no English note.") - continue - try: - written += post_english_note(session, subdomain, model, ticket) - except (SystemExit, requests.RequestException) as exc: - print(f"Note: could not post the English note on #{ticket['id']} ({exc}).") - except Exception as exc: # noqa: BLE001 - one ticket must not take the digest down - # The type only: the message of an unexpected error may carry ticket text. - print(f"Note: the English note on #{ticket['id']} failed " - f"({type(exc).__name__}).") - # Printed even at zero, so a run that wrote nothing reads differently in the - # journal from one that never reached this step. - print(f"Posted an English note on {written} of {len(candidates)} non-English ticket(s).") - return written def say(session, subdomain, ticket_id, comment_id, text, dry_run, error=True): @@ -1208,6 +1179,9 @@ def main(argv=None): parser.add_argument("--ticket", type=int, required=True) parser.add_argument("--dry-run", action="store_true", help="do everything except write to Zendesk") + parser.add_argument("--english-if-needed", action="store_true", + help="ignore commands; post the English transcript if the " + "conversation needs one (the customer just wrote)") args = parser.parse_args(argv) model = claude_cli.MODEL @@ -1224,6 +1198,10 @@ def main(argv=None): return comments = api.fetch_comments(session, subdomain, args.ticket) + if args.english_if_needed: + run_auto_english(session, subdomain, model, ticket, comments, args.dry_run) + return + command = latest_command(comments, api_user, session, subdomain) if not command: print(f"#{args.ticket}: no command note to act on.") diff --git a/src/session_ops/zendesk/relay.py b/src/session_ops/zendesk/relay.py index d85ab12..0b92c3f 100644 --- a/src/session_ops/zendesk/relay.py +++ b/src/session_ops/zendesk/relay.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 """ -Webhook endpoint for acting on `claude:` private notes in Zendesk. +Webhook endpoint for acting on `claude:` private notes in Zendesk, and for putting a +conversation into English when the customer writes. Serves POST /zendesk/notes β€” vendor-namespaced so the same host and certificate can carry other integrations later. Run it behind a TLS terminator; see @@ -105,7 +106,12 @@ def dry_run_requested(): return bool(value) and value.split()[0] not in ("0", "false", "no", "off") -def run_note_reply(ticket_id): +# The `event` a trigger may send, and the note_reply.py mode it runs. Absent is the +# `claude:` note trigger; "customer" is the trigger on the customer writing. +EVENT_ARGS = {None: [], "customer": ["--english-if-needed"]} + + +def run_note_reply(ticket_id, extra_args=()): """Run note_reply.py over one ticket. Never raises. A subprocess because its error paths are sys.exit @@ -115,7 +121,7 @@ def run_note_reply(ticket_id): ticket, not at an open dialog β€” so the outcome goes to the journal and, where note_reply.py got far enough to write one, to the ticket itself. """ - command = [sys.executable, "-m", NOTE_MODULE, "--ticket", str(ticket_id)] + command = [sys.executable, "-m", NOTE_MODULE, "--ticket", str(ticket_id), *extra_args] if dry_run_requested(): command.append("--dry-run") try: @@ -170,10 +176,13 @@ def healthz(): @app.post("/zendesk/notes") async def zendesk_notes(request: Request, background: BackgroundTasks): - """A private note on a ticket asked Claude to do something. + """A private note on a ticket asked Claude to do something, or, with + `"event": "customer"`, the customer wrote and the ticket may need an English + transcript. - The Zendesk trigger that calls this is the first gate β€” it fires only on private - comments, and must exclude the API user so a draft cannot trigger another draft. + The Zendesk trigger that calls this is the first gate β€” the note trigger fires + only on private comments, and both must exclude the API user so a note Claude + wrote cannot trigger another run. This is the second: without ZENDESK_WEBHOOK_SECRET set, nothing is accepted, so an unconfigured relay refuses rather than trusting anything that reaches the URL. Which Zendesk user may command it is note_reply.py's decision, on the note. @@ -199,7 +208,11 @@ async def zendesk_notes(request: Request, background: BackgroundTasks): if isinstance(body, dict) else None) if not ticket_id: return Response("no ticket id", status_code=400) - background.add_task(run_in_threadpool, run_note_reply, ticket_id) + event = body.get("event") + # An unhashable value, a list say, would raise in the lookup and answer 500. + if not (event is None or isinstance(event, str)) or event not in EVENT_ARGS: + return Response("unknown event", status_code=400) + background.add_task(run_in_threadpool, run_note_reply, ticket_id, EVENT_ARGS[event]) return {"ok": True} diff --git a/src/session_ops/zendesk/transcript.py b/src/session_ops/zendesk/transcript.py index ca1da39..688ac92 100644 --- a/src/session_ops/zendesk/transcript.py +++ b/src/session_ops/zendesk/transcript.py @@ -47,16 +47,3 @@ "paraphrase it and do not 'improve' it.\n\n" "Leave Session IDs, version numbers, URLs and error strings exactly as written." ) - - -def is_english(finding): - """Whether the classifier called this ticket English. - - Unknown counts as English: the note is only worth posting when it says - something the agent cannot already read, and a blank `language` is far more - likely to be a classification that came back thin than a ticket nobody could - read. Guessing wrong this way costs a missing transcript; the other way puts a - machine translation over the top of words everyone could already read. - """ - language = (finding.get("language") or "").strip().lower() - return not language or language.startswith(("english", "en")) diff --git a/src/session_ops/zendesk/triage.py b/src/session_ops/zendesk/triage.py index 68c6117..a73016c 100644 --- a/src/session_ops/zendesk/triage.py +++ b/src/session_ops/zendesk/triage.py @@ -78,7 +78,7 @@ from session_ops.shared import discord, http, state as dedup from session_ops.shared.discord import clip, post_to_discord -from session_ops.shared.env import get_env, rehearsing +from session_ops.shared.env import get_env from session_ops.shared.text import squash from session_ops.zendesk.api import (REVIEW_CHANNEL, SEARCH_RESULT_LIMIT, fetch_comments, fetch_tickets, @@ -86,7 +86,6 @@ review_platform, review_stars, ticket_url, zendesk_session) from session_ops.zendesk.claude_cli import MODEL, claude_cli_json -from session_ops.zendesk.note_reply import post_english_notes # Never analyzed. A store review cannot be answered the way a ticket can: it takes # one developer response, replacing any previous one, with no way to ask a follow-up @@ -1143,13 +1142,6 @@ def main(argv=None): print(f"Recorded {len(recorded)} tickets; state now tracks {kept} " f"({pruned} pruned beyond {args.state_retention_days} days).") - # After the post and the state, so a slow translation can never hold up or lose - # the digest. Abuse reports are left out: nobody acts on them. Only on a run that - # posts: neither CI (--no-discord) nor a rehearsal may write to production tickets. - if not rehearsing(): - post_english_notes(zd, subdomain, classified, - [f for f in findings if not is_collapsed(f)], MODEL) - if posted < len(messages): sys.exit(f"Aborted after {posted}/{len(messages)} messages; " f"undelivered tickets stay eligible for the next run.") diff --git a/tests/sogs/test_names.py b/tests/sogs/test_names.py index a776d5b..96438ed 100644 --- a/tests/sogs/test_names.py +++ b/tests/sogs/test_names.py @@ -109,6 +109,20 @@ def test_a_room_that_times_out_is_reported_not_raised(self): self.assertEqual([t for t, _ in unread], ['a']) +class TestMalformedRoom(unittest.TestCase): + def test_a_room_with_malformed_messages_is_reported_not_raised(self): + for messages in ([post(BLINDED_ABS, None, message_data('x')), post(BLINDED_ABS, 5, message_data('y'))], + [{'session_id': BLINDED_ABS, 'data': message_data('x')}], + ['not a message']): + with self.subTest(messages=messages): + client = FakeClient({'a': (200, messages)}) + _, unread = names.recent_names(client, [{'token': 'a'}], owners(SESSION_ID)) + self.assertEqual([t for t, _ in unread], ['a']) + + def test_non_text_data_is_no_name(self): + self.assertIsNone(names.display_name(12345)) + + class TestDescribePoster(unittest.TestCase): def test_each_case_reads_differently(self): self.assertEqual(ban.describe_poster(SESSION_ID, {}), "no recent posts") diff --git a/tests/zendesk/test_api.py b/tests/zendesk/test_api.py index debe004..9dbfaa6 100644 --- a/tests/zendesk/test_api.py +++ b/tests/zendesk/test_api.py @@ -15,7 +15,7 @@ class TestFetchUser(unittest.TestCase): def test_an_unreachable_zendesk_is_an_unknown_author_not_an_error(self): - """post_english_notes promises never to raise; an unknown author counts as a customer.""" + """An unreachable Zendesk is an unknown author, which counts as a customer.""" self.assertEqual(api.fetch_user(FakeSession([requests.ConnectionError("down")]), "acme", 5), {}) diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index 54fec33..cfbd64d 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -1049,6 +1049,91 @@ def test_the_brief_is_bounded(self): self.assertLessEqual(len(brief), note_reply.BRIEF_CHARS) +class AutoEnglish(unittest.TestCase): + """`--english-if-needed`, run when the customer writes. Nobody asked, so it + answers nobody: a note when one is needed, the journal otherwise.""" + + GERMAN_TURNS = [{"index": 0, "who": "Customer", "when": "t", "body": "Es geht nicht."}] + + def run_auto(self, ticket=None, comments=(), translated=None, dry_run=False): + session = fake_session(FakeResponse({"ticket": {}})) + calls = [] + + def translate(*a, **k): + calls.append(a) + return translated if translated is not None else { + "turns": [{"index": 0, "english": "It does not work."}]} + + with Patched(api, conversation_turns=lambda *a: self.GERMAN_TURNS), \ + Patched(claude_cli, claude_cli_json=translate), \ + contextlib.redirect_stdout(io.StringIO()) as out: + note_reply.run_auto_english(session, "sub", "model", ticket or {"id": 7}, + list(comments), dry_run) + return session.calls, calls, out.getvalue() + + def test_a_foreign_conversation_gets_the_private_note(self): + writes, _, _ = self.run_auto(comments=[comment("Hallo", author=42, public=True, cid=3)]) + note = writes[0][2]["json"]["ticket"]["comment"] + self.assertIs(note["public"], False) + self.assertIn("It does not work.", note["html_body"]) + self.assertIn(note_reply.english_marker(3), note["html_body"]) + + def test_it_tags_nothing_and_claims_no_command(self): + writes, _, _ = self.run_auto() + self.assertEqual([c for c in writes if "/tags.json" in c[1]], []) + self.assertNotIn("claude:done", writes[0][2]["json"]["ticket"]["comment"]["html_body"]) + + def test_a_covered_conversation_costs_nothing(self): + prior = comment(f"transcript {note_reply.english_marker(3)}", author=API_USER, cid=4) + writes, calls, _ = self.run_auto( + comments=[prior, comment("Hallo", author=42, public=True, cid=3)]) + self.assertEqual((writes, calls), ([], [])) + + def test_an_english_conversation_writes_nothing(self): + writes, _, out = self.run_auto( + translated={"turns": [{"index": 0, "english": "Es geht nicht."}]}) + self.assertEqual(writes, []) + self.assertIn("english", out) + + def test_a_failed_translation_writes_nothing_and_flags_no_error(self): + def fail(*a, **k): + raise SystemExit("claude exited 1") + + session = fake_session() + with Patched(api, conversation_turns=lambda *a: self.GERMAN_TURNS), \ + Patched(claude_cli, claude_cli_json=fail), \ + contextlib.redirect_stdout(io.StringIO()) as out: + note_reply.run_auto_english(session, "sub", "model", {"id": 7}, [], False) + self.assertEqual(session.calls, []) + self.assertIn("claude exited 1", out.getvalue()) + + def test_an_app_store_review_is_never_translated(self): + writes, calls, _ = self.run_auto(ticket={"id": 7, "via": {"channel": "any_channel"}}) + self.assertEqual((writes, calls), ([], [])) + + def test_a_dry_run_writes_nothing(self): + writes, calls, _ = self.run_auto(dry_run=True) + self.assertEqual(writes, []) + self.assertEqual(len(calls), 1) + + def test_the_flag_never_runs_a_pending_command(self): + """The customer trigger fires on their reply, and a `claude: reply` waiting + on the ticket must not be sent because of it.""" + ran = [] + env = {"ZENDESK_SUBDOMAIN": "sub", "ZENDESK_EMAIL": "e", "ZENDESK_API_TOKEN": "t"} + command = {"id": 9, "author": AGENT, "action": "reply", "brief": ""} + with mock.patch.dict(os.environ, env), \ + Patched(api, zendesk_session=lambda *a: object(), + fetch_ticket=lambda *a: {"id": 7, "status": "open"}, + fetch_comments=lambda *a, **k: []), \ + Patched(note_reply, api_user_id=lambda *a: API_USER, + latest_command=lambda *a: command, + run_reply=lambda *a: ran.append("reply"), + run_auto_english=lambda *a: ran.append("english")): + note_reply.main(["--ticket", "7", "--english-if-needed"]) + self.assertEqual(ran, ["english"]) + + class Model(unittest.TestCase): def model_for(self, flag): seen = [] diff --git a/tests/zendesk/test_relay.py b/tests/zendesk/test_relay.py index c925350..f4c18fd 100644 --- a/tests/zendesk/test_relay.py +++ b/tests/zendesk/test_relay.py @@ -80,7 +80,7 @@ def setUp(self): def run_with(self, **env): with Env(ZENDESK_WEBHOOK_SECRET=ZENDESK_SECRET, **env), \ - Patched(relay, run_note_reply=self.ran.append): + Patched(relay, run_note_reply=lambda tid, extra=(): self.ran.append(tid)): return zendesk_post(**self.kwargs) def test_a_signed_webhook_queues_the_ticket(self): @@ -167,6 +167,43 @@ def test_a_non_json_body_is_refused_not_crashed(self): +class TestEvents(unittest.TestCase): + """The `claude:` note trigger sends no event; the customer trigger sends + "customer" and must never reach the command path.""" + + def post(self, body): + ran = [] + with Env(ZENDESK_WEBHOOK_SECRET=ZENDESK_SECRET), \ + Patched(relay, run_note_reply=lambda tid, extra=(): ran.append((tid, list(extra)))): + return zendesk_post(body), ran + + def test_no_event_runs_the_commands(self): + response, ran = self.post({"ticket_id": "27603"}) + self.assertEqual((response.status_code, ran), (200, [("27603", [])])) + + def test_a_customer_event_only_translates(self): + response, ran = self.post({"ticket_id": "27603", "event": "customer"}) + self.assertEqual((response.status_code, ran), + (200, [("27603", ["--english-if-needed"])])) + + def test_an_unknown_event_is_refused(self): + """An unrecognised trigger must not fall through to the command path.""" + for event in ("draft", "Customer", 1, ["customer"]): + with self.subTest(event=event): + response, ran = self.post({"ticket_id": "27603", "event": event}) + self.assertEqual((response.status_code, ran), (400, [])) + + def test_the_mode_reaches_the_subprocess(self): + seen = [] + + class Done: + returncode, stdout, stderr = 0, "", "" + + with Env(), Patched(relay.subprocess, run=lambda cmd, **k: seen.append(cmd) or Done()): + relay.run_note_reply("27603", ["--english-if-needed"]) + self.assertEqual(seen[0][-3:], ["--ticket", "27603", "--english-if-needed"]) + + class TestSignatureTimestamps(unittest.TestCase): """The age check is only meaningful if the timestamp is read in the zone it was written in.""" diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index 2fb36f0..a90fdfa 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -8,19 +8,16 @@ Everything here is offline β€” no Zendesk, Claude, or Discord calls. The fetch tests drive fetch_tickets with a stub session instead. """ -import contextlib -import io import json import os import tempfile import unittest -from unittest import mock from datetime import datetime, timedelta, timezone import requests from session_ops.shared import discord -from session_ops.zendesk import api, claude_cli, note_reply, transcript, triage +from session_ops.zendesk import api, claude_cli, triage from session_ops.shared.testing import ( FakeResponse, FakeSession, NonJsonResponse, Patched) @@ -1846,25 +1843,7 @@ def test_a_wedged_cli_does_not_hang_the_run(self): class TestEnglishTranscript(unittest.TestCase): - """The digest posts `claude: english`'s private note on every non-English ticket - it classifies, so nobody has to ask for one.""" - - def test_a_ticket_the_classifier_called_english_is_left_alone(self): - """Translating English into English would put a machine's wording in front of - the agent in place of the words everyone could already read.""" - for language in ("English", "english", "en", "EN"): - self.assertTrue(transcript.is_english({"language": language}), language) - - def test_an_unknown_language_counts_as_english(self): - """A blank `language` is far more likely to be a thin classification than a - ticket nobody could read. Guessing this way wastes nothing; the other way - overwrites words everyone could read with a translation of them.""" - for language in ("", None, " "): - self.assertTrue(transcript.is_english({"language": language}), repr(language)) - - def test_a_non_english_ticket_is_translated(self): - for language in ("German", "Spanish", "Japanese"): - self.assertFalse(transcript.is_english({"language": language}), language) + """What the English transcript is built from: the language call and the turns.""" # ---- turns ------------------------------------------------------------- @@ -1930,210 +1909,6 @@ def test_the_stamp_is_minutes_not_seconds(self): self.assertEqual(api.stamp_minutes("2026-08-28T01:31:09Z"), "2026-08-28 01:31 UTC") - # ---- the run ----------------------------------------------------------- - - GERMAN_TURNS = [{"index": 0, "who": "Customer", "when": "2026-08-28 00:22 UTC", - "body": "Es geht nicht."}] - - def run_notes(self, tickets, findings, comments=(), translated=None, session=object()): - """post_english_notes with Zendesk and Claude faked. Returns (count, notes, out).""" - notes, out = [], io.StringIO() - translated = translated or (lambda *a, **k: { - "turns": [{"index": 0, "english": "It does not work."}]}) - with Patched(api, fetch_comments=lambda *a, **k: list(comments), - conversation_turns=lambda *a: self.GERMAN_TURNS), \ - Patched(claude_cli, claude_cli_json=translated), \ - Patched(note_reply, write_to_ticket=lambda *a, **k: notes.append((a, k))), \ - contextlib.redirect_stdout(out): - got = note_reply.post_english_notes(session, "acme", tickets, findings, - "claude-sonnet-5") - return got, notes, out.getvalue() - - def run_main(self, findings, env): - """triage.main over --findings, recording post_english_notes' arguments.""" - seen = [] - with tempfile.TemporaryDirectory() as directory: - path = os.path.join(directory, "findings.json") - with open(path, "w", encoding="utf-8") as handle: - json.dump(findings, handle) - with Patched(triage, post_english_notes=lambda *a: seen.append(a), - post_to_discord=lambda session, url, messages: len(messages)), \ - mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", - "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y", - **env}), \ - contextlib.redirect_stdout(io.StringIO()): - triage.main(["--findings", path]) - return seen - - def test_a_run_that_posts_nothing_writes_nothing(self): - """--no-discord is the flag CI runs with, and CI must not write to production - tickets: the notes come after the post, past every early return.""" - for flag in ("--no-discord", "--dry-run"): - with self.subTest(flag=flag): - seen = [] - with tempfile.TemporaryDirectory() as directory: - path = os.path.join(directory, "findings.json") - with open(path, "w", encoding="utf-8") as handle: - json.dump([finding(1, language="German")], handle) - with Patched(triage, post_english_notes=lambda *a: seen.append(a)), \ - mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme"}), \ - contextlib.redirect_stdout(io.StringIO()): - triage.main(["--findings", path, flag]) - self.assertEqual(seen, []) - - def test_the_notes_come_after_the_post(self): - """A slow translation must never hold up, or time out, the digest.""" - order = [] - with tempfile.TemporaryDirectory() as directory: - path = os.path.join(directory, "findings.json") - with open(path, "w", encoding="utf-8") as handle: - json.dump([finding(1, language="German")], handle) - with Patched(triage, post_english_notes=lambda *a: order.append("notes"), - post_to_discord=lambda s, u, m: order.append("post") or len(m)), \ - mock.patch.dict(os.environ, {"ZENDESK_SUBDOMAIN": "acme", - "ZENDESK_DISCORD_WEBHOOK_URL": "https://x/y"}), \ - contextlib.redirect_stdout(io.StringIO()): - triage.main(["--findings", path]) - self.assertEqual(order, ["post", "notes"]) - - def test_abuse_reports_are_not_translated(self): - seen = self.run_main([finding(1, language="German"), - finding(2, category="abuse_report", language="German")], {}) - self.assertEqual([f["id"] for f in seen[0][3]], [1]) - - def test_the_transcript_uses_the_same_model_as_the_classification(self): - seen = self.run_main([finding(1)], {}) - self.assertEqual([a[4] for a in seen], [claude_cli.MODEL]) - - def test_a_rehearsal_writes_no_transcript(self): - self.assertEqual(self.run_main([finding(1)], {"SESSION_OPS_REHEARSAL": "1"}), []) - - def test_unflagged_tickets_are_translated_too(self): - """A ticket with no card of its own is still opened from the digest or the - queue, and whoever opens it should read it in English.""" - seen = self.run_main([finding(1), finding(2, worth_looking_into=False, - language="German")], {}) - self.assertEqual(sorted(f["id"] for f in seen[0][3]), [1, 2]) - - def test_a_non_english_ticket_gets_a_private_note(self): - got, notes, _ = self.run_notes([{"id": 1}, {"id": 2}], - [{"id": 1, "language": "German"}, - {"id": 2, "language": "English"}]) - self.assertEqual(got, 1) - (args, kwargs), = notes - self.assertEqual(args[2], 1) - self.assertIs(kwargs["public"], False) - self.assertIn("It does not work.", args[3]) - self.assertIn("Customer:", args[3]) - - def test_the_note_is_the_one_claude_english_writes(self): - """Same note, same marker: a transcript either one wrote stops the other - repeating it until somebody writes again.""" - _, notes, _ = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], - comments=[{"id": 5, "public": True, "body": "x"}]) - self.assertIn(note_reply.english_marker(5), notes[0][0][3]) - - def test_a_ticket_already_transcribed_is_not_re_rendered(self): - calls = [] - comments = [{"id": 6, "public": False, "body": note_reply.english_marker(5)}, - {"id": 5, "public": True, "body": "x"}] - got, notes, _ = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], - comments=comments, - translated=lambda *a, **k: calls.append(a)) - self.assertEqual((got, notes, calls), (0, [], [])) - - def test_a_conversation_that_turns_out_english_gets_no_note(self): - got, notes, _ = self.run_notes( - [{"id": 1}], [{"id": 1, "language": "German"}], - translated=lambda *a, **k: {"turns": [{"index": 0, "english": "Es geht nicht."}]}) - self.assertEqual((got, notes), (0, [])) - - def test_nothing_happens_without_a_zendesk_session(self): - """--findings never builds one: the findings already exist and no ticket was - ever fetched, so there is nothing to read comments from or write back to.""" - got, notes, _ = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], - session=None) - self.assertEqual((got, notes), (0, [])) - - def test_one_ticket_that_cannot_be_rendered_does_not_stop_the_digest(self): - """claude_cli_json exits on a failed call, which is right for the - classification it was written for. Here it would take down a digest that is - otherwise ready to post over one ticket nobody can translate.""" - def explode(*args, **kwargs): - raise SystemExit("claude exited 1") - - got, notes, out = self.run_notes([{"id": 1}, {"id": 2}], - [{"id": 1, "language": "German"}, - {"id": 2, "language": "German"}], - translated=explode) - self.assertEqual((got, notes), (0, [])) - self.assertIn("claude exited 1", out) - - def test_a_rejected_note_does_not_stop_the_digest(self): - def reject(*a, **k): - raise SystemExit("Zendesk rejected the note on #1 (422).") - - with Patched(note_reply, write_to_ticket=reject), \ - Patched(api, fetch_comments=lambda *a, **k: [], - conversation_turns=lambda *a: self.GERMAN_TURNS), \ - Patched(claude_cli, claude_cli_json=lambda *a, **k: { - "turns": [{"index": 0, "english": "It does not work."}]}), \ - contextlib.redirect_stdout(io.StringIO()): - self.assertEqual(note_reply.post_english_notes( - object(), "acme", [{"id": 1}], [{"id": 1, "language": "German"}], "m"), 0) - - def test_comments_that_cannot_be_fetched_skip_the_ticket(self): - calls = [] - with Patched(api, fetch_comments=lambda *a, **k: None), \ - Patched(claude_cli, claude_cli_json=lambda *a, **k: calls.append(a)), \ - contextlib.redirect_stdout(io.StringIO()): - got = note_reply.post_english_notes(object(), "acme", [{"id": 1}], - [{"id": 1, "language": "German"}], "m") - self.assertEqual((got, calls), (0, [])) - - def test_no_failure_on_one_ticket_reaches_the_digest(self): - """A network error on the write, and anything unforeseen, are both logged and - skipped; the next ticket is still translated.""" - for error in (requests.ConnectionError("down"), KeyError("turns")): - with self.subTest(error=type(error).__name__): - writes = [] - - def write(*a, **k): - writes.append(a[2]) - if a[2] == 1: - raise error - - with Patched(note_reply, write_to_ticket=write), \ - Patched(api, fetch_comments=lambda *a, **k: [], - conversation_turns=lambda *a: self.GERMAN_TURNS), \ - Patched(claude_cli, claude_cli_json=lambda *a, **k: { - "turns": [{"index": 0, "english": "It does not work."}]}), \ - contextlib.redirect_stdout(io.StringIO()): - got = note_reply.post_english_notes( - object(), "acme", [{"id": 1}, {"id": 2}], - [{"id": 1, "language": "German"}, {"id": 2, "language": "German"}], "m") - self.assertEqual((got, writes), (1, [1, 2])) - - def test_a_run_that_wrote_nothing_says_so(self): - """A run that wrote nothing and a run that never reached this step must read - differently in the journal.""" - _, _, out = self.run_notes([{"id": 1}], [{"id": 1, "language": "German"}], - translated=lambda *a, **k: { - "turns": [{"index": 0, "english": "Es geht nicht."}]}) - self.assertIn("0 of 1", out) - - def test_an_all_english_digest_says_so_rather_than_nothing(self): - _, _, out = self.run_notes([], [{"id": 1, "language": "English"}, - {"id": 2, "language": "English"}]) - self.assertIn("No non-English tickets", out) - self.assertIn("2", out) - - def test_a_ticket_that_was_classified_but_never_fetched_is_named(self): - _, notes, out = self.run_notes([], [{"id": 99, "language": "German"}]) - self.assertEqual(notes, []) - self.assertIn("99", out) - self.assertIn("never fetched", out) - class TestCustomerSide(unittest.TestCase): """Which comments are the customer's. Decided by requester_id alone this was From 5346cd60f889d45ef8260fc70a381806aa29b2f8 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 15 Oct 2026 15:00:52 +1100 Subject: [PATCH 07/12] fix(zendesk): translate the newest turns whole, one run per ticket A long conversation was cut to 8000 characters from its end, dropping the customer's newest message mid-JSON, and the turns cut off counted as already English: a long English ticket got a transcript on every message, and a long foreign one a note whose marker covered a turn it never translated. Whole turns now go from the oldest end, the newest is clipped only when it alone is too long, and the note says how many were left out. The relay runs one note_reply at a time per ticket and waits 60 seconds after a customer message, restarted by each further one, so a burst of messages costs one translation and two runs cannot both post it. note_reply also re-reads the ticket for the marker just before writing. --- docs/jobs/zendesk-relay.md | 13 +++- src/session_ops/zendesk/note_reply.py | 68 ++++++++++++++++--- src/session_ops/zendesk/relay.py | 54 ++++++++++++++- src/session_ops/zendesk/transcript.py | 2 +- tests/zendesk/test_note_reply.py | 70 +++++++++++++++++-- tests/zendesk/test_relay.py | 96 ++++++++++++++++++++++++++- 6 files changed, 282 insertions(+), 21 deletions(-) diff --git a/docs/jobs/zendesk-relay.md b/docs/jobs/zendesk-relay.md index be1ee5a..bbe88da 100644 --- a/docs/jobs/zendesk-relay.md +++ b/docs/jobs/zendesk-relay.md @@ -51,8 +51,17 @@ is not in English (see the second trigger below). It shares `claude: english`'s so whichever gets there first stops the other repeating it until there is a new public comment. It answers nobody: an English conversation, a failed translation or an app-store review leaves only a line in the journal, and nothing is tagged. It costs one -Claude call per customer message, English ones included: the language is judged from -the translation, because a character test calls unaccented German English. +Claude call per burst of customer messages, English ones included: the language is +judged from the translation, because a character test calls unaccented German English. + +The relay waits 60 seconds after a customer message, restarted by each further one on +that ticket, before translating, and never runs two jobs on one ticket at once: a +message arriving mid-run gets one more run after it. A relay restart drops a wait in +progress; that ticket is translated on the customer's next message, or by +`claude: english`. + +A conversation too long to send whole loses its oldest turns, never its newest, and +the note says how many were left out. ## Why a draft is always reviewed diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index 35f68f0..215417f 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -1012,12 +1012,42 @@ def already_english(turns, translated): for turn in turns) +def transcript_payload(turns): + return json.dumps([{"index": t["index"], "speaker": t["who"], "text": t["body"]} + for t in turns], ensure_ascii=False) + + +def fit_turns(turns, limit): + """The newest turns whose payload fits `limit`, and how many older ones were left out. + + Whole turns go from the oldest end, since the newest is what the customer just sent. + A newest turn too long on its own is clipped, never dropped. + """ + kept = list(turns) + while len(kept) > 1 and len(transcript_payload(kept)) > limit: + kept.pop(0) + omitted = len(turns) - len(kept) + if len(transcript_payload(kept)) <= limit: + return kept, omitted + turn = kept[0] + room = limit - len(transcript_payload([dict(turn, body="")])) + while True: + clipped = dict(turn, body=clip(turn["body"], max(room, 1))) + # JSON escaping makes the payload longer than the text it carries. + excess = len(transcript_payload([clipped])) - limit + if excess <= 0 or room <= 1: + return [clipped], omitted + room -= excess + + def translate_conversation(session, subdomain, model, ticket, comments): """The conversation on a ticket in English, or why there is none. Returns (outcome, detail): ("current", None) when a transcript already covers the newest public comment, ("empty", None), ("english", None) when the conversation - is already English, ("failed", the reason), or ("ready", (turns, rendered, latest)). + is already English, ("failed", the reason), or ("ready", (turns, rendered, latest, + omitted)): `turns` are the ones sent, newest last, and `omitted` the older ones that + did not fit. Reads both sides, not just the customer's: their second message is usually an answer to a reply, and without the reply it reads as a complaint about nothing. @@ -1033,32 +1063,40 @@ def translate_conversation(session, subdomain, model, ticket, comments): turns = api.conversation_turns(session, subdomain, ticket) if not turns: return "empty", None - payload = json.dumps([{"index": t["index"], "speaker": t["who"], "text": t["body"]} - for t in turns], ensure_ascii=False) + turns, omitted = fit_turns(turns, transcript.TRANSCRIPT_INPUT_CHARS) try: rendered = claude_cli.claude_cli_json( model, "medium", transcript.TRANSCRIPT_SYSTEM_PROMPT, transcript.TRANSCRIPT_SCHEMA, - clip(payload, transcript.TRANSCRIPT_INPUT_CHARS), + transcript_payload(turns), transcript.ENGLISH_TIMEOUT_SECONDS, f"the English transcript of #{ticket_id}") except SystemExit as exc: return "failed", str(exc) # A transcript of English text repeats what is already a few comments above it. if already_english(turns, rendered.get("turns")): return "english", None - return "ready", (turns, rendered, latest) + return "ready", (turns, rendered, latest, omitted) -def english_note(turns, rendered, latest, command_id=None): +def english_note(turns, rendered, latest, omitted=0, command_id=None): """The private note carrying the transcript. Its english_marker is what stops - either the command or the digest writing the same transcript twice.""" + either the command or the customer trigger writing the same transcript twice.""" markers = ([done_marker(command_id)] if command_id is not None else []) + [english_marker(latest)] + left_out = ([para(f"The {omitted} earliest turn(s) are left out: the conversation is " + "too long to translate whole.")] if omitted else []) return "".join( [para(f"This conversation in English β€” {len(turns)} turn(s), both sides."), para("Translated for reading; the customer has not seen this.")] + + left_out + transcript_blocks(turns, rendered.get("turns")) + [para(" ".join(markers))]) +def transcript_landed(session, subdomain, ticket_id, latest): + """Whether another run posted this transcript while the model was translating.""" + comments = api.fetch_comments(session, subdomain, ticket_id, optional=True) + return bool(comments) and api.has_marker(comments, english_marker(latest)) + + def run_english(session, subdomain, model, ticket, comments, command, dry_run): """Put the conversation on the ticket in English, as a private note.""" ticket_id = ticket["id"] @@ -1079,13 +1117,17 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): f"The English transcript could not be produced: {detail} To try again, add " "a new private note reading claude: english.", dry_run) return - turns, rendered, latest = detail + turns, rendered, latest, omitted = detail print(f"#{ticket_id}: rendered {len(turns)} turn(s) in English.") if dry_run: print(f"#{ticket_id}: dry run, nothing written.") return + if transcript_landed(session, subdomain, ticket_id, latest): + say(session, subdomain, ticket_id, command["id"], answers["current"], dry_run, + error=False) + return write_to_ticket(session, subdomain, ticket_id, - english_note(turns, rendered, latest, command["id"]), + english_note(turns, rendered, latest, omitted, command["id"]), public=False, as_html=True, drop_tags=[TAG_QUEUED, TAG_ERROR]) @@ -1109,12 +1151,16 @@ def run_auto_english(session, subdomain, model, ticket, comments, dry_run): print(f"#{ticket_id}: no English note ({outcome}" + (f": {detail})" if outcome == "failed" else ")")) return - turns, rendered, latest = detail + turns, rendered, latest, omitted = detail print(f"#{ticket_id}: rendered {len(turns)} turn(s) in English.") if dry_run: print(f"#{ticket_id}: dry run, nothing written.") return - write_to_ticket(session, subdomain, ticket_id, english_note(turns, rendered, latest), + if transcript_landed(session, subdomain, ticket_id, latest): + print(f"#{ticket_id}: another run posted this transcript first; nothing written.") + return + write_to_ticket(session, subdomain, ticket_id, + english_note(turns, rendered, latest, omitted), public=False, as_html=True) diff --git a/src/session_ops/zendesk/relay.py b/src/session_ops/zendesk/relay.py index 0b92c3f..adb783b 100644 --- a/src/session_ops/zendesk/relay.py +++ b/src/session_ops/zendesk/relay.py @@ -19,7 +19,12 @@ Nothing is stored. A ticket is read when its webhook fires and forgotten; the draft awaiting review lives in a private note on the ticket, so a restart mid-review loses -nothing. +nothing. A restart does drop a customer event still waiting out its debounce: that +ticket gets its transcript on the customer's next message, or from `claude: english`. + +One ticket's runs never overlap, so two runs cannot both find no transcript and both +post one. A customer event waits ENGLISH_DEBOUNCE_SECONDS, restarted by each further +event on that ticket, so a burst of messages costs one translation. This used to serve a second route, POST /discord/interactions, behind a Comment button on each digest card. Replies are written on the ticket now β€” the digest is read-only, @@ -42,10 +47,12 @@ Usage: uvicorn session_ops.zendesk.relay:app --host 127.0.0.1 --port 8080 """ +import asyncio import base64 import datetime import hashlib import hmac +import itertools import json import os import subprocess @@ -68,6 +75,8 @@ # Zendesk gets its 200 immediately and the outcome is written to the ticket whenever # it lands. NOTE_TIMEOUT_SECONDS = 420 +# Customers on messaging channels send several short messages in a row. +ENGLISH_DEBOUNCE_SECONDS = 60 # Zendesk signs a webhook over the timestamp followed by the raw body. ZENDESK_SIGNATURE_HEADER = "x-zendesk-webhook-signature" ZENDESK_TIMESTAMP_HEADER = "x-zendesk-webhook-signature-timestamp" @@ -137,6 +146,44 @@ def run_note_reply(ticket_id, extra_args=()): f"{(done.stderr or '').strip()[:300]}", flush=True) +async def run_in_thread(ticket_id, extra_args): + await run_in_threadpool(run_note_reply, ticket_id, extra_args) + + +class TicketRuns: + """note_reply.py runs, one at a time per ticket, with customer events debounced.""" + + def __init__(self, run, debounce_seconds, sleep=asyncio.sleep): + self.run, self.debounce_seconds, self.sleep = run, debounce_seconds, sleep + self.locks = {} + self.latest_event = {} + self.events = itertools.count() + + async def now(self, ticket_id, extra_args): + lock, users = self.locks.get(ticket_id, (asyncio.Lock(), 0)) + self.locks[ticket_id] = (lock, users + 1) + try: + async with lock: + await self.run(ticket_id, extra_args) + finally: + lock, users = self.locks[ticket_id] + if users == 1: + del self.locks[ticket_id] + else: + self.locks[ticket_id] = (lock, users - 1) + + async def customer_wrote(self, ticket_id): + event = self.latest_event[ticket_id] = next(self.events) + await self.sleep(self.debounce_seconds) + if self.latest_event.get(ticket_id) != event: + return + del self.latest_event[ticket_id] + await self.now(ticket_id, EVENT_ARGS["customer"]) + + +runs = TicketRuns(run_in_thread, ENGLISH_DEBOUNCE_SECONDS) + + def zendesk_signature_ok(raw, signature, timestamp, secret): """Whether Zendesk signed this body, recently. @@ -212,7 +259,10 @@ async def zendesk_notes(request: Request, background: BackgroundTasks): # An unhashable value, a list say, would raise in the lookup and answer 500. if not (event is None or isinstance(event, str)) or event not in EVENT_ARGS: return Response("unknown event", status_code=400) - background.add_task(run_in_threadpool, run_note_reply, ticket_id, EVENT_ARGS[event]) + if event == "customer": + background.add_task(runs.customer_wrote, ticket_id) + else: + background.add_task(runs.now, ticket_id, EVENT_ARGS[event]) return {"ok": True} diff --git a/src/session_ops/zendesk/transcript.py b/src/session_ops/zendesk/transcript.py index 688ac92..6de9d54 100644 --- a/src/session_ops/zendesk/transcript.py +++ b/src/session_ops/zendesk/transcript.py @@ -1,6 +1,6 @@ """The prompt and schema that put a ticket's conversation into English. -Shared by `claude: english` and the digest, which post the same private note. +Shared by `claude: english` and the customer trigger, which post the same private note. """ ENGLISH_TIMEOUT_SECONDS = 180 diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index cfbd64d..4522a29 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -17,6 +17,7 @@ """ import contextlib import html +import json import io import os import re @@ -124,6 +125,10 @@ def test_a_generated_draft_note_is_not_a_command(self): note_reply.comment_text(posted_note(note)))) +def writes(session): + return [call for call in session.calls if call[0] != "GET"] + + class EnglishTranscript(unittest.TestCase): def test_english_is_a_command(self): self.assertEqual(note_reply.parse_command("claude: english"), ("english", "")) @@ -280,12 +285,60 @@ def test_the_transcript_note_is_never_public(self): [dict(comment("Hallo", author=42, cid=3), public=True)], {"id": 9, "author": AGENT, "action": "english", "brief": ""}, dry_run=False) - ticket = session.calls[0][2]["json"]["ticket"] + ticket = writes(session)[0][2]["json"]["ticket"] self.assertIs(ticket["comment"]["public"], False) self.assertIn("Hello", ticket["comment"]["html_body"]) self.assertIn(note_reply.english_marker(3), ticket["comment"]["html_body"]) +class FitTurns(unittest.TestCase): + def turns(self, *bodies): + return [{"index": i, "who": "Customer", "when": "", "body": b} + for i, b in enumerate(bodies)] + + def test_a_short_conversation_is_sent_whole(self): + turns = self.turns("eins", "zwei") + self.assertEqual(note_reply.fit_turns(turns, 8000), (turns, 0)) + + def test_the_oldest_turns_go_first_and_whole(self): + turns = self.turns("a" * 3000, "b" * 3000, "c" * 3000) + kept, omitted = note_reply.fit_turns(turns, 8000) + self.assertEqual(([t["index"] for t in kept], omitted), ([1, 2], 1)) + self.assertEqual(kept, turns[1:]) + json.loads(note_reply.transcript_payload(kept)) + + def test_a_newest_turn_too_long_alone_is_clipped_to_fit(self): + turns = self.turns("old", 'neu "zitat"\n' * 2000) + kept, omitted = note_reply.fit_turns(turns, 8000) + self.assertEqual((len(kept), kept[0]["index"], omitted), (1, 1, 1)) + payload = note_reply.transcript_payload(kept) + self.assertLessEqual(len(payload), 8000) + self.assertTrue(json.loads(payload)[0]["text"].endswith("…")) + + def test_only_the_turns_that_fit_are_sent_and_judged(self): + """A turn the model never saw cannot be judged already English.""" + turns = self.turns("a" * 7990, "Es geht nicht.") + sent = [] + + def translate(model, effort, system, schema, payload, *a): + sent.append(json.loads(payload)) + return {"turns": [{"index": 1, "english": "It does not work."}]} + + with Patched(api, conversation_turns=lambda *a: turns), \ + Patched(claude_cli, claude_cli_json=translate): + outcome, detail = note_reply.translate_conversation( + None, "sub", "model", {"id": 7}, [comment("x", public=True, cid=3)]) + self.assertEqual(outcome, "ready") + self.assertEqual([t["index"] for t in sent[0]], [1]) + self.assertEqual(detail[3], 1) + + def test_the_note_says_older_turns_were_left_out(self): + turns = self.turns("Es geht nicht.") + note = note_reply.english_note(turns, {"turns": []}, 3, omitted=2) + self.assertIn("2 earliest turn(s) are left out", note) + self.assertNotIn("left out", note_reply.english_note(turns, {"turns": []}, 3)) + + BOOK = { "groups": [{"key": "attachments", "title": "Attachments fail", "platform_sensitive": True}], "cells": { @@ -1055,8 +1108,9 @@ class AutoEnglish(unittest.TestCase): GERMAN_TURNS = [{"index": 0, "who": "Customer", "when": "t", "body": "Es geht nicht."}] - def run_auto(self, ticket=None, comments=(), translated=None, dry_run=False): - session = fake_session(FakeResponse({"ticket": {}})) + def run_auto(self, ticket=None, comments=(), translated=None, dry_run=False, + landed=()): + session = fake_session(FakeResponse({"comments": list(landed)})) calls = [] def translate(*a, **k): @@ -1069,7 +1123,7 @@ def translate(*a, **k): contextlib.redirect_stdout(io.StringIO()) as out: note_reply.run_auto_english(session, "sub", "model", ticket or {"id": 7}, list(comments), dry_run) - return session.calls, calls, out.getvalue() + return writes(session), calls, out.getvalue() def test_a_foreign_conversation_gets_the_private_note(self): writes, _, _ = self.run_auto(comments=[comment("Hallo", author=42, public=True, cid=3)]) @@ -1107,6 +1161,14 @@ def fail(*a, **k): self.assertEqual(session.calls, []) self.assertIn("claude exited 1", out.getvalue()) + def test_a_transcript_posted_meanwhile_is_not_posted_again(self): + """The model call takes minutes; a `claude: english` run can land in them.""" + landed = comment(f"transcript {note_reply.english_marker(3)}", author=API_USER, cid=4) + writes, calls, out = self.run_auto( + comments=[comment("Hallo", author=42, public=True, cid=3)], landed=[landed]) + self.assertEqual((writes, len(calls)), ([], 1)) + self.assertIn("another run", out) + def test_an_app_store_review_is_never_translated(self): writes, calls, _ = self.run_auto(ticket={"id": 7, "via": {"channel": "any_channel"}}) self.assertEqual((writes, calls), ([], [])) diff --git a/tests/zendesk/test_relay.py b/tests/zendesk/test_relay.py index f4c18fd..2ac10e8 100644 --- a/tests/zendesk/test_relay.py +++ b/tests/zendesk/test_relay.py @@ -12,6 +12,7 @@ request is refused, and that an unconfigured relay refuses everything rather than trusting whatever reaches the URL. """ +import asyncio import base64 import datetime import hashlib @@ -174,7 +175,8 @@ class TestEvents(unittest.TestCase): def post(self, body): ran = [] with Env(ZENDESK_WEBHOOK_SECRET=ZENDESK_SECRET), \ - Patched(relay, run_note_reply=lambda tid, extra=(): ran.append((tid, list(extra)))): + Patched(relay, run_note_reply=lambda tid, extra=(): ran.append((tid, list(extra))), + runs=relay.TicketRuns(relay.run_in_thread, 0)): return zendesk_post(body), ran def test_no_event_runs_the_commands(self): @@ -204,6 +206,98 @@ class Done: self.assertEqual(seen[0][-3:], ["--ticket", "27603", "--english-if-needed"]) +class Clock: + """asyncio.sleep's stand-in: sleepers wake only when the test advances time.""" + + def __init__(self): + self.now, self.sleepers = 0, [] + + async def sleep(self, seconds): + wake = asyncio.get_running_loop().create_future() + self.sleepers.append((self.now + seconds, wake)) + await wake + + async def advance(self, seconds): + self.now += seconds + for due, wake in list(self.sleepers): + if due <= self.now: + self.sleepers.remove((due, wake)) + wake.set_result(None) + await settle() + + +async def settle(): + for _ in range(20): + await asyncio.sleep(0) + + +class TestTicketRuns(unittest.IsolatedAsyncioTestCase): + def setUp(self): + self.clock, self.ran, self.running, self.overlapped = Clock(), [], set(), False + self.gates = {} + self.runs = relay.TicketRuns(self.fake_run, 60, sleep=self.clock.sleep) + + async def fake_run(self, ticket_id, extra_args): + self.overlapped |= ticket_id in self.running + self.running.add(ticket_id) + self.ran.append((ticket_id, list(extra_args))) + gate = self.gates.get(len(self.ran)) + if gate: + await gate.wait() + self.running.discard(ticket_id) + + async def customer(self, ticket_id="1"): + task = asyncio.create_task(self.runs.customer_wrote(ticket_id)) + await settle() + return task + + async def test_a_burst_of_messages_translates_once_after_the_last(self): + await self.customer() + await self.clock.advance(30) + await self.customer() + await self.clock.advance(59) + self.assertEqual(self.ran, []) + await self.clock.advance(1) + self.assertEqual(self.ran, [("1", ["--english-if-needed"])]) + + async def test_a_message_during_a_run_translates_again_after_it(self): + self.gates[1] = gate = asyncio.Event() + await self.customer() + await self.clock.advance(60) + await self.customer() + await self.clock.advance(60) + self.assertEqual(len(self.ran), 1) + gate.set() + await settle() + self.assertEqual(len(self.ran), 2) + self.assertFalse(self.overlapped) + + async def test_a_command_waits_for_the_tickets_translation(self): + self.gates[1] = gate = asyncio.Event() + await self.customer() + await self.clock.advance(60) + command = asyncio.create_task(self.runs.now("1", [])) + await settle() + self.assertEqual(len(self.ran), 1) + gate.set() + await command + self.assertEqual(self.ran[1], ("1", [])) + self.assertFalse(self.overlapped) + + async def test_other_tickets_are_not_held_up(self): + self.gates[1] = asyncio.Event() + asyncio.create_task(self.runs.now("1", [])) + await settle() + await self.runs.now("2", []) + self.assertEqual([ticket for ticket, _ in self.ran], ["1", "2"]) + + async def test_nothing_is_kept_once_a_ticket_is_done(self): + await self.customer() + await self.clock.advance(60) + await self.runs.now("1", []) + self.assertEqual((self.runs.locks, self.runs.latest_event), ({}, {})) + + class TestSignatureTimestamps(unittest.TestCase): """The age check is only meaningful if the timestamp is read in the zone it was written in.""" From 9bf0f05e3c3d6bc90c503a43b34832fd6afd41d7 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 15 Oct 2026 15:11:37 +1100 Subject: [PATCH 08/12] feat(zendesk): list new non-English tickets in the digest instead of translating them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay no longer translates when the customer writes: no customer event, no debounce, no --english-if-needed. The digest instead leads its entries with a 🌐 line linking every ticket reported for the first time that is not in English, with its language, for an agent to answer `claude: english` on. `claude: english` keeps the newest turns whole, drops the oldest when the conversation is too long and says how many, and checks for the transcript again just before writing it. --- docs/jobs/zendesk-digest.md | 7 +- docs/jobs/zendesk-relay.md | 30 +----- src/session_ops/zendesk/note_reply.py | 49 +--------- src/session_ops/zendesk/relay.py | 79 ++------------- src/session_ops/zendesk/transcript.py | 5 +- src/session_ops/zendesk/triage.py | 39 +++++++- tests/zendesk/test_note_reply.py | 117 ++++------------------ tests/zendesk/test_relay.py | 133 +------------------------- tests/zendesk/test_triage.py | 52 ++++++++++ 9 files changed, 132 insertions(+), 379 deletions(-) diff --git a/docs/jobs/zendesk-digest.md b/docs/jobs/zendesk-digest.md index ee6cd3e..acfec0a 100644 --- a/docs/jobs/zendesk-digest.md +++ b/docs/jobs/zendesk-digest.md @@ -43,13 +43,13 @@ The star-floor machinery (`partition_reviews`, `--review-star-floor`, `--include ## What else is out of scope - **`pending` tickets.** Somebody already replied and the ball is with the customer; the "Pending to Solved" automation resolves them after 72h. The query is `status`, and `updated_at` moves on any change β€” a tag edit, the hourly automation, and every private note the relay writes, the automatic English transcripts included. So the run drops anything whose `requester_updated_at` falls outside the window. Measured on a real 72h window: 79 fetched, 23 the requester had actually touched. +- **Tickets only we touched.** The window is on `updated>`, and `updated_at` moves on any change β€” a tag edit, the hourly automation, and every private note the `claude:` commands write. So the run drops anything whose `requester_updated_at` falls outside the window. Measured on a real 72h window: 79 fetched, 23 the requester had actually touched. ## Content-free tickets Twitter DM tickets arrive with `description` identical to `subject` β€” both just `"Conversation with "` β€” which is 15% of non-review tickets and unclassifiable as fetched. For those only, `hydrate_descriptions` fetches a page of up to 10 comments and joins every body that differs from the subject into the description; later replies often carry the actual detail. Hydration is an enrichment, so an HTTP error or an unreachable endpoint leaves the ticket as-is rather than failing the run (`--no-hydrate` to skip it entirely). -The script (`zendesk-triage`, [triage.py](../../src/session_ops/zendesk/triage.py)) fetches the tickets in a rolling time window, classifies the whole batch in one schema-enforced request through the Claude Code CLI, and posts a Discord digest: a short header, one line per ticket worth looking into, then the abuse reports and the unflagged tickets. +The script (`zendesk-triage`, [triage.py](../../src/session_ops/zendesk/triage.py)) fetches the tickets in a rolling time window, classifies the whole batch in one schema-enforced request through the Claude Code CLI, and posts a Discord digest: a short header, the new tickets not in English, one line per ticket worth looking into, then the abuse reports and the unflagged tickets. Each line leads with a severity marker, a category emoji and a platform icon, links the ticket id, and carries the model's one-line summary plus its root-cause guess: @@ -59,6 +59,7 @@ Backlog: **428** awaiting a reply, excluding app-store reviews (**5,252** more a **9** worth looking into. ⭐ **6** Β· πŸ› **3** Β· ❓ **2** Β· πŸ”‡ **2** Β· πŸ”‘ **1** Β· βš–οΈ **1** Β· πŸ”’ **1** Likely duplicates: **push-notifications-not-delivered** Γ—5 (#27637, #27610, #27606, #27605) +🌐 **2** new, not in English β€” #27610 German, #27648 Russian 🚨 | βš–οΈ | ❔ | #27632 Β· Police summons demanding user details for a Session ID 🚨 | πŸ”’ | πŸ€– | #27603 Β· Exported component lets another app obtain internal SharedPreferences | Likely cause: Improperly exported provider allowing external apps to trigger file sharing 🟠 | ⭐ | 🍎 | #27610 Β· Messages not delivered for days; nothing shows even after opening | Likely cause: Push notification delivery / message retrieval failure @@ -76,6 +77,8 @@ Likely duplicates: **push-notifications-not-delivered** Γ—5 (#27637, #27610, #27 The header accounts for the batch in full, so nothing is dropped silently. The backlog line is scoped exactly like the analysis β€” `status` | Compose the reply from the brief, in the requester's language. A second `draft` amends the one already there rather than starting over | no | | `claude: reply [n]` | Publish the chosen option verbatim, status -> `pending` | **yes** | -| `claude: english` | Post the conversation, both sides, in English. Says so and writes nothing when the ticket is already English | no | +| `claude: english` | Post the conversation, both sides, in English. Says so and writes nothing when the ticket is already English. Too long to send whole, it drops the oldest turns and says how many | no | | `claude: explain` | Post what support usually replied to this kind of ticket, what was actually done about it, and the caveats | no | | `claude: solve [reason]` | Solve without writing to the customer, for tickets that need no reply. The note records who decided and why | no comment, but **solving fires the CSAT automation** | -The same transcript is also posted unasked, when the customer writes on a ticket that -is not in English (see the second trigger below). It shares `claude: english`'s marker, -so whichever gets there first stops the other repeating it until there is a new public -comment. It answers nobody: an English conversation, a failed translation or an -app-store review leaves only a line in the journal, and nothing is tagged. It costs one -Claude call per burst of customer messages, English ones included: the language is -judged from the translation, because a character test calls unaccented German English. - -The relay waits 60 seconds after a customer message, restarted by each further one on -that ticket, before translating, and never runs two jobs on one ticket at once: a -message arriving mid-run gets one more run after it. A relay restart drops a wait in -progress; that ticket is translated on the customer's next message, or by -`claude: english`. - -A conversation too long to send whole loses its oldest turns, never its newest, and -the note says how many were left out. - ## Why a draft is always reviewed The reply flow this replaced sent an English ticket immediately, because the agent had @@ -145,14 +128,6 @@ A trigger, and a webhook it calls: - **Trigger** β€” conditions: *Ticket is Updated*, *Comment is Private*, *Comment text contains `claude:`*, and *Current user is not* the automation user. Actions: notify the webhook, and add the tag `claude-queued`. -- **Second trigger, the customer wrote** β€” conditions: *Comment is Public*, *Current - user is (end-user)*, and *Channel is not* the app-store review integration. Action: - notify the same webhook with `{"ticket_id": "{{ticket.id}}", "event": "customer"}`, - and no tag. It runs `note_reply --english-if-needed`, which never acts on a command. - App-store reviews are refused in the code as well, whatever these conditions say. - -Any other `event` is refused with a 400, so a misconfigured trigger cannot fall -through to the command path. ## Required Secrets @@ -172,5 +147,4 @@ uv run zendesk-note-reply --ticket 27603 --dry-run ``` A ticket with no command note prints `no command note to act on` and stops, so this -is safe to point at anything. `--english-if-needed --dry-run` shows what the customer -trigger would do. +is safe to point at anything. diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index 215417f..d9bb279 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -1077,10 +1077,9 @@ def translate_conversation(session, subdomain, model, ticket, comments): return "ready", (turns, rendered, latest, omitted) -def english_note(turns, rendered, latest, omitted=0, command_id=None): - """The private note carrying the transcript. Its english_marker is what stops - either the command or the customer trigger writing the same transcript twice.""" - markers = ([done_marker(command_id)] if command_id is not None else []) + [english_marker(latest)] +def english_note(turns, rendered, latest, omitted, command_id): + """The private note carrying the transcript. Its english_marker is what stops a + second `claude: english` writing the same transcript before anything new is said.""" left_out = ([para(f"The {omitted} earliest turn(s) are left out: the conversation is " "too long to translate whole.")] if omitted else []) return "".join( @@ -1088,7 +1087,7 @@ def english_note(turns, rendered, latest, omitted=0, command_id=None): para("Translated for reading; the customer has not seen this.")] + left_out + transcript_blocks(turns, rendered.get("turns")) - + [para(" ".join(markers))]) + + [para(f"{done_marker(command_id)} {english_marker(latest)}")]) def transcript_landed(session, subdomain, ticket_id, latest): @@ -1131,39 +1130,6 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): public=False, as_html=True, drop_tags=[TAG_QUEUED, TAG_ERROR]) -def run_auto_english(session, subdomain, model, ticket, comments, dry_run): - """`claude: english`, unasked, after the customer writes: the same private note, - posted only when the conversation is not English and no transcript covers it yet. - - The note's marker is shared with the command, so whichever writes first stops the - other repeating it until there is a new public comment. Nobody asked, so there is - no one to answer: every outcome other than a note goes to the journal only, and - nothing is tagged. - """ - ticket_id = ticket["id"] - # App-store reviews take one developer response, not a conversation, and they - # are most of the account: refused here whatever the trigger's conditions say. - if api.is_store_review(ticket): - print(f"#{ticket_id}: an app-store review; no English note.") - return - outcome, detail = translate_conversation(session, subdomain, model, ticket, comments) - if outcome != "ready": - print(f"#{ticket_id}: no English note ({outcome}" - + (f": {detail})" if outcome == "failed" else ")")) - return - turns, rendered, latest, omitted = detail - print(f"#{ticket_id}: rendered {len(turns)} turn(s) in English.") - if dry_run: - print(f"#{ticket_id}: dry run, nothing written.") - return - if transcript_landed(session, subdomain, ticket_id, latest): - print(f"#{ticket_id}: another run posted this transcript first; nothing written.") - return - write_to_ticket(session, subdomain, ticket_id, - english_note(turns, rendered, latest, omitted), - public=False, as_html=True) - - def say(session, subdomain, ticket_id, comment_id, text, dry_run, error=True): """Answer the agent on the ticket, and record that this command was handled. @@ -1225,9 +1191,6 @@ def main(argv=None): parser.add_argument("--ticket", type=int, required=True) parser.add_argument("--dry-run", action="store_true", help="do everything except write to Zendesk") - parser.add_argument("--english-if-needed", action="store_true", - help="ignore commands; post the English transcript if the " - "conversation needs one (the customer just wrote)") args = parser.parse_args(argv) model = claude_cli.MODEL @@ -1244,10 +1207,6 @@ def main(argv=None): return comments = api.fetch_comments(session, subdomain, args.ticket) - if args.english_if_needed: - run_auto_english(session, subdomain, model, ticket, comments, args.dry_run) - return - command = latest_command(comments, api_user, session, subdomain) if not command: print(f"#{args.ticket}: no command note to act on.") diff --git a/src/session_ops/zendesk/relay.py b/src/session_ops/zendesk/relay.py index adb783b..d85ab12 100644 --- a/src/session_ops/zendesk/relay.py +++ b/src/session_ops/zendesk/relay.py @@ -1,7 +1,6 @@ #!/usr/bin/env python3 """ -Webhook endpoint for acting on `claude:` private notes in Zendesk, and for putting a -conversation into English when the customer writes. +Webhook endpoint for acting on `claude:` private notes in Zendesk. Serves POST /zendesk/notes β€” vendor-namespaced so the same host and certificate can carry other integrations later. Run it behind a TLS terminator; see @@ -19,12 +18,7 @@ Nothing is stored. A ticket is read when its webhook fires and forgotten; the draft awaiting review lives in a private note on the ticket, so a restart mid-review loses -nothing. A restart does drop a customer event still waiting out its debounce: that -ticket gets its transcript on the customer's next message, or from `claude: english`. - -One ticket's runs never overlap, so two runs cannot both find no transcript and both -post one. A customer event waits ENGLISH_DEBOUNCE_SECONDS, restarted by each further -event on that ticket, so a burst of messages costs one translation. +nothing. This used to serve a second route, POST /discord/interactions, behind a Comment button on each digest card. Replies are written on the ticket now β€” the digest is read-only, @@ -47,12 +41,10 @@ Usage: uvicorn session_ops.zendesk.relay:app --host 127.0.0.1 --port 8080 """ -import asyncio import base64 import datetime import hashlib import hmac -import itertools import json import os import subprocess @@ -75,8 +67,6 @@ # Zendesk gets its 200 immediately and the outcome is written to the ticket whenever # it lands. NOTE_TIMEOUT_SECONDS = 420 -# Customers on messaging channels send several short messages in a row. -ENGLISH_DEBOUNCE_SECONDS = 60 # Zendesk signs a webhook over the timestamp followed by the raw body. ZENDESK_SIGNATURE_HEADER = "x-zendesk-webhook-signature" ZENDESK_TIMESTAMP_HEADER = "x-zendesk-webhook-signature-timestamp" @@ -115,12 +105,7 @@ def dry_run_requested(): return bool(value) and value.split()[0] not in ("0", "false", "no", "off") -# The `event` a trigger may send, and the note_reply.py mode it runs. Absent is the -# `claude:` note trigger; "customer" is the trigger on the customer writing. -EVENT_ARGS = {None: [], "customer": ["--english-if-needed"]} - - -def run_note_reply(ticket_id, extra_args=()): +def run_note_reply(ticket_id): """Run note_reply.py over one ticket. Never raises. A subprocess because its error paths are sys.exit @@ -130,7 +115,7 @@ def run_note_reply(ticket_id, extra_args=()): ticket, not at an open dialog β€” so the outcome goes to the journal and, where note_reply.py got far enough to write one, to the ticket itself. """ - command = [sys.executable, "-m", NOTE_MODULE, "--ticket", str(ticket_id), *extra_args] + command = [sys.executable, "-m", NOTE_MODULE, "--ticket", str(ticket_id)] if dry_run_requested(): command.append("--dry-run") try: @@ -146,44 +131,6 @@ def run_note_reply(ticket_id, extra_args=()): f"{(done.stderr or '').strip()[:300]}", flush=True) -async def run_in_thread(ticket_id, extra_args): - await run_in_threadpool(run_note_reply, ticket_id, extra_args) - - -class TicketRuns: - """note_reply.py runs, one at a time per ticket, with customer events debounced.""" - - def __init__(self, run, debounce_seconds, sleep=asyncio.sleep): - self.run, self.debounce_seconds, self.sleep = run, debounce_seconds, sleep - self.locks = {} - self.latest_event = {} - self.events = itertools.count() - - async def now(self, ticket_id, extra_args): - lock, users = self.locks.get(ticket_id, (asyncio.Lock(), 0)) - self.locks[ticket_id] = (lock, users + 1) - try: - async with lock: - await self.run(ticket_id, extra_args) - finally: - lock, users = self.locks[ticket_id] - if users == 1: - del self.locks[ticket_id] - else: - self.locks[ticket_id] = (lock, users - 1) - - async def customer_wrote(self, ticket_id): - event = self.latest_event[ticket_id] = next(self.events) - await self.sleep(self.debounce_seconds) - if self.latest_event.get(ticket_id) != event: - return - del self.latest_event[ticket_id] - await self.now(ticket_id, EVENT_ARGS["customer"]) - - -runs = TicketRuns(run_in_thread, ENGLISH_DEBOUNCE_SECONDS) - - def zendesk_signature_ok(raw, signature, timestamp, secret): """Whether Zendesk signed this body, recently. @@ -223,13 +170,10 @@ def healthz(): @app.post("/zendesk/notes") async def zendesk_notes(request: Request, background: BackgroundTasks): - """A private note on a ticket asked Claude to do something, or, with - `"event": "customer"`, the customer wrote and the ticket may need an English - transcript. + """A private note on a ticket asked Claude to do something. - The Zendesk trigger that calls this is the first gate β€” the note trigger fires - only on private comments, and both must exclude the API user so a note Claude - wrote cannot trigger another run. + The Zendesk trigger that calls this is the first gate β€” it fires only on private + comments, and must exclude the API user so a draft cannot trigger another draft. This is the second: without ZENDESK_WEBHOOK_SECRET set, nothing is accepted, so an unconfigured relay refuses rather than trusting anything that reaches the URL. Which Zendesk user may command it is note_reply.py's decision, on the note. @@ -255,14 +199,7 @@ async def zendesk_notes(request: Request, background: BackgroundTasks): if isinstance(body, dict) else None) if not ticket_id: return Response("no ticket id", status_code=400) - event = body.get("event") - # An unhashable value, a list say, would raise in the lookup and answer 500. - if not (event is None or isinstance(event, str)) or event not in EVENT_ARGS: - return Response("unknown event", status_code=400) - if event == "customer": - background.add_task(runs.customer_wrote, ticket_id) - else: - background.add_task(runs.now, ticket_id, EVENT_ARGS[event]) + background.add_task(run_in_threadpool, run_note_reply, ticket_id) return {"ok": True} diff --git a/src/session_ops/zendesk/transcript.py b/src/session_ops/zendesk/transcript.py index 6de9d54..9750ee8 100644 --- a/src/session_ops/zendesk/transcript.py +++ b/src/session_ops/zendesk/transcript.py @@ -1,7 +1,4 @@ -"""The prompt and schema that put a ticket's conversation into English. - -Shared by `claude: english` and the customer trigger, which post the same private note. -""" +"""The prompt and schema `claude: english` puts a ticket's conversation into English with.""" ENGLISH_TIMEOUT_SECONDS = 180 diff --git a/src/session_ops/zendesk/triage.py b/src/session_ops/zendesk/triage.py index a73016c..34fa729 100644 --- a/src/session_ops/zendesk/triage.py +++ b/src/session_ops/zendesk/triage.py @@ -671,6 +671,7 @@ def analyze(model, effort, compact_tickets): COLLAPSED_LINKS_CHARS = 900 # Every other classified ticket, linked once, one line per category under it. UNFLAGGED_MARKER = "πŸ’€" +NON_ENGLISH_MARKER = "🌐" # ---- Components V2 --------------------------------------------------------- # @@ -788,6 +789,39 @@ def build_unflagged_lines(unflagged, subdomain): return out +def is_english(finding): + """Unknown counts as English: a thin classification is likelier than a ticket + nobody here can read.""" + language = (finding.get("language") or "").strip().lower() + return language in ("", "unknown") or language.startswith("english") + + +def build_non_english_line(findings, subdomain, updated_ids): + """The tickets reported for the first time that are not in English, for an agent to + answer `claude: english` on. None when there are none. + + 🌐 **2** new, not in English β€” #28891 German, #28880 Russian + + Covers no ticket: each is accounted for by its own line elsewhere in the digest. + """ + members = [f for f in findings if f.get("id") not in updated_ids and not is_english(f)] + if not members: + return None + links, used = [], 0 + for f in members: + link = (f"[#{f.get('id')}]({ticket_url(subdomain, f.get('id'))}) " + f"{clip(f.get('language'), 20)}") + used += len(link) + 2 + if used > COLLAPSED_LINKS_CHARS: + break + links.append(link) + unlisted = len(members) - len(links) + if unlisted: + links.append(f"+{unlisted} more") + return (f"{NON_ENGLISH_MARKER} **{len(members)}** new, not in English β€” " + f"{', '.join(links)}", set()) + + def build_header(findings, highlights, stats=None): """The lead lines: what was looked at, the category tally, duplicate clusters. @@ -901,7 +935,10 @@ def build_messages(findings, subdomain, stats=None, updated_ids=None): if omitted: header += (f"\nShowing the top **{len(shown)}** of " f"**{len(shown) + len(omitted)}** worth looking into.") - entries = [ + non_english = build_non_english_line(findings, subdomain, updated_ids) + # First, under the header: it is the run's to-do list for `claude: english`. + entries = [non_english] if non_english else [] + entries += [ (build_ticket_line(f, subdomain, is_update=f.get("id") in updated_ids), {f.get("id")}) for f in shown diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index 4522a29..e018934 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -208,6 +208,25 @@ def fail(*a, **k): if m == "PUT" and "/tags.json" in u] self.assertEqual(added, [[note_reply.TAG_ERROR]]) + def test_a_transcript_posted_meanwhile_is_not_posted_again(self): + """The model call takes minutes; a second `claude: english` can land in them.""" + turns = [{"index": 0, "who": "Customer", "when": "t", "body": "Hallo"}] + landed = comment(f"transcript {note_reply.english_marker(3)}", author=API_USER, cid=4) + session = fake_session(FakeResponse({"comments": [landed]})) + with Patched(api, conversation_turns=lambda *a: turns), \ + Patched(claude_cli, claude_cli_json=lambda *a, **k: { + "turns": [{"index": 0, "english": "Hello"}]}), \ + contextlib.redirect_stdout(io.StringIO()): + note_reply.run_english(session, "sub", "model", {"id": 7}, + [comment("claude: english", cid=9), + comment("Hallo", author=42, public=True, cid=3)], + {"id": 9, "author": AGENT, "action": "english", + "brief": ""}, dry_run=False) + bodies = [kw["json"]["ticket"]["comment"]["html_body"] for m, u, kw in writes(session) + if "comment" in (kw.get("json") or {}).get("ticket", {})] + self.assertEqual(len(bodies), 1) + self.assertIn("already up to date", bodies[0]) + def test_unaccented_german_is_still_translated(self): """The reason this is checked after the call, not guessed before it: "Hallo, ich habe ein Problem" is pure ASCII, and a character test would have @@ -334,9 +353,9 @@ def translate(model, effort, system, schema, payload, *a): def test_the_note_says_older_turns_were_left_out(self): turns = self.turns("Es geht nicht.") - note = note_reply.english_note(turns, {"turns": []}, 3, omitted=2) + note = note_reply.english_note(turns, {"turns": []}, 3, 2, 9) self.assertIn("2 earliest turn(s) are left out", note) - self.assertNotIn("left out", note_reply.english_note(turns, {"turns": []}, 3)) + self.assertNotIn("left out", note_reply.english_note(turns, {"turns": []}, 3, 0, 9)) BOOK = { @@ -1102,100 +1121,6 @@ def test_the_brief_is_bounded(self): self.assertLessEqual(len(brief), note_reply.BRIEF_CHARS) -class AutoEnglish(unittest.TestCase): - """`--english-if-needed`, run when the customer writes. Nobody asked, so it - answers nobody: a note when one is needed, the journal otherwise.""" - - GERMAN_TURNS = [{"index": 0, "who": "Customer", "when": "t", "body": "Es geht nicht."}] - - def run_auto(self, ticket=None, comments=(), translated=None, dry_run=False, - landed=()): - session = fake_session(FakeResponse({"comments": list(landed)})) - calls = [] - - def translate(*a, **k): - calls.append(a) - return translated if translated is not None else { - "turns": [{"index": 0, "english": "It does not work."}]} - - with Patched(api, conversation_turns=lambda *a: self.GERMAN_TURNS), \ - Patched(claude_cli, claude_cli_json=translate), \ - contextlib.redirect_stdout(io.StringIO()) as out: - note_reply.run_auto_english(session, "sub", "model", ticket or {"id": 7}, - list(comments), dry_run) - return writes(session), calls, out.getvalue() - - def test_a_foreign_conversation_gets_the_private_note(self): - writes, _, _ = self.run_auto(comments=[comment("Hallo", author=42, public=True, cid=3)]) - note = writes[0][2]["json"]["ticket"]["comment"] - self.assertIs(note["public"], False) - self.assertIn("It does not work.", note["html_body"]) - self.assertIn(note_reply.english_marker(3), note["html_body"]) - - def test_it_tags_nothing_and_claims_no_command(self): - writes, _, _ = self.run_auto() - self.assertEqual([c for c in writes if "/tags.json" in c[1]], []) - self.assertNotIn("claude:done", writes[0][2]["json"]["ticket"]["comment"]["html_body"]) - - def test_a_covered_conversation_costs_nothing(self): - prior = comment(f"transcript {note_reply.english_marker(3)}", author=API_USER, cid=4) - writes, calls, _ = self.run_auto( - comments=[prior, comment("Hallo", author=42, public=True, cid=3)]) - self.assertEqual((writes, calls), ([], [])) - - def test_an_english_conversation_writes_nothing(self): - writes, _, out = self.run_auto( - translated={"turns": [{"index": 0, "english": "Es geht nicht."}]}) - self.assertEqual(writes, []) - self.assertIn("english", out) - - def test_a_failed_translation_writes_nothing_and_flags_no_error(self): - def fail(*a, **k): - raise SystemExit("claude exited 1") - - session = fake_session() - with Patched(api, conversation_turns=lambda *a: self.GERMAN_TURNS), \ - Patched(claude_cli, claude_cli_json=fail), \ - contextlib.redirect_stdout(io.StringIO()) as out: - note_reply.run_auto_english(session, "sub", "model", {"id": 7}, [], False) - self.assertEqual(session.calls, []) - self.assertIn("claude exited 1", out.getvalue()) - - def test_a_transcript_posted_meanwhile_is_not_posted_again(self): - """The model call takes minutes; a `claude: english` run can land in them.""" - landed = comment(f"transcript {note_reply.english_marker(3)}", author=API_USER, cid=4) - writes, calls, out = self.run_auto( - comments=[comment("Hallo", author=42, public=True, cid=3)], landed=[landed]) - self.assertEqual((writes, len(calls)), ([], 1)) - self.assertIn("another run", out) - - def test_an_app_store_review_is_never_translated(self): - writes, calls, _ = self.run_auto(ticket={"id": 7, "via": {"channel": "any_channel"}}) - self.assertEqual((writes, calls), ([], [])) - - def test_a_dry_run_writes_nothing(self): - writes, calls, _ = self.run_auto(dry_run=True) - self.assertEqual(writes, []) - self.assertEqual(len(calls), 1) - - def test_the_flag_never_runs_a_pending_command(self): - """The customer trigger fires on their reply, and a `claude: reply` waiting - on the ticket must not be sent because of it.""" - ran = [] - env = {"ZENDESK_SUBDOMAIN": "sub", "ZENDESK_EMAIL": "e", "ZENDESK_API_TOKEN": "t"} - command = {"id": 9, "author": AGENT, "action": "reply", "brief": ""} - with mock.patch.dict(os.environ, env), \ - Patched(api, zendesk_session=lambda *a: object(), - fetch_ticket=lambda *a: {"id": 7, "status": "open"}, - fetch_comments=lambda *a, **k: []), \ - Patched(note_reply, api_user_id=lambda *a: API_USER, - latest_command=lambda *a: command, - run_reply=lambda *a: ran.append("reply"), - run_auto_english=lambda *a: ran.append("english")): - note_reply.main(["--ticket", "7", "--english-if-needed"]) - self.assertEqual(ran, ["english"]) - - class Model(unittest.TestCase): def model_for(self, flag): seen = [] diff --git a/tests/zendesk/test_relay.py b/tests/zendesk/test_relay.py index 2ac10e8..c925350 100644 --- a/tests/zendesk/test_relay.py +++ b/tests/zendesk/test_relay.py @@ -12,7 +12,6 @@ request is refused, and that an unconfigured relay refuses everything rather than trusting whatever reaches the URL. """ -import asyncio import base64 import datetime import hashlib @@ -81,7 +80,7 @@ def setUp(self): def run_with(self, **env): with Env(ZENDESK_WEBHOOK_SECRET=ZENDESK_SECRET, **env), \ - Patched(relay, run_note_reply=lambda tid, extra=(): self.ran.append(tid)): + Patched(relay, run_note_reply=self.ran.append): return zendesk_post(**self.kwargs) def test_a_signed_webhook_queues_the_ticket(self): @@ -168,136 +167,6 @@ def test_a_non_json_body_is_refused_not_crashed(self): -class TestEvents(unittest.TestCase): - """The `claude:` note trigger sends no event; the customer trigger sends - "customer" and must never reach the command path.""" - - def post(self, body): - ran = [] - with Env(ZENDESK_WEBHOOK_SECRET=ZENDESK_SECRET), \ - Patched(relay, run_note_reply=lambda tid, extra=(): ran.append((tid, list(extra))), - runs=relay.TicketRuns(relay.run_in_thread, 0)): - return zendesk_post(body), ran - - def test_no_event_runs_the_commands(self): - response, ran = self.post({"ticket_id": "27603"}) - self.assertEqual((response.status_code, ran), (200, [("27603", [])])) - - def test_a_customer_event_only_translates(self): - response, ran = self.post({"ticket_id": "27603", "event": "customer"}) - self.assertEqual((response.status_code, ran), - (200, [("27603", ["--english-if-needed"])])) - - def test_an_unknown_event_is_refused(self): - """An unrecognised trigger must not fall through to the command path.""" - for event in ("draft", "Customer", 1, ["customer"]): - with self.subTest(event=event): - response, ran = self.post({"ticket_id": "27603", "event": event}) - self.assertEqual((response.status_code, ran), (400, [])) - - def test_the_mode_reaches_the_subprocess(self): - seen = [] - - class Done: - returncode, stdout, stderr = 0, "", "" - - with Env(), Patched(relay.subprocess, run=lambda cmd, **k: seen.append(cmd) or Done()): - relay.run_note_reply("27603", ["--english-if-needed"]) - self.assertEqual(seen[0][-3:], ["--ticket", "27603", "--english-if-needed"]) - - -class Clock: - """asyncio.sleep's stand-in: sleepers wake only when the test advances time.""" - - def __init__(self): - self.now, self.sleepers = 0, [] - - async def sleep(self, seconds): - wake = asyncio.get_running_loop().create_future() - self.sleepers.append((self.now + seconds, wake)) - await wake - - async def advance(self, seconds): - self.now += seconds - for due, wake in list(self.sleepers): - if due <= self.now: - self.sleepers.remove((due, wake)) - wake.set_result(None) - await settle() - - -async def settle(): - for _ in range(20): - await asyncio.sleep(0) - - -class TestTicketRuns(unittest.IsolatedAsyncioTestCase): - def setUp(self): - self.clock, self.ran, self.running, self.overlapped = Clock(), [], set(), False - self.gates = {} - self.runs = relay.TicketRuns(self.fake_run, 60, sleep=self.clock.sleep) - - async def fake_run(self, ticket_id, extra_args): - self.overlapped |= ticket_id in self.running - self.running.add(ticket_id) - self.ran.append((ticket_id, list(extra_args))) - gate = self.gates.get(len(self.ran)) - if gate: - await gate.wait() - self.running.discard(ticket_id) - - async def customer(self, ticket_id="1"): - task = asyncio.create_task(self.runs.customer_wrote(ticket_id)) - await settle() - return task - - async def test_a_burst_of_messages_translates_once_after_the_last(self): - await self.customer() - await self.clock.advance(30) - await self.customer() - await self.clock.advance(59) - self.assertEqual(self.ran, []) - await self.clock.advance(1) - self.assertEqual(self.ran, [("1", ["--english-if-needed"])]) - - async def test_a_message_during_a_run_translates_again_after_it(self): - self.gates[1] = gate = asyncio.Event() - await self.customer() - await self.clock.advance(60) - await self.customer() - await self.clock.advance(60) - self.assertEqual(len(self.ran), 1) - gate.set() - await settle() - self.assertEqual(len(self.ran), 2) - self.assertFalse(self.overlapped) - - async def test_a_command_waits_for_the_tickets_translation(self): - self.gates[1] = gate = asyncio.Event() - await self.customer() - await self.clock.advance(60) - command = asyncio.create_task(self.runs.now("1", [])) - await settle() - self.assertEqual(len(self.ran), 1) - gate.set() - await command - self.assertEqual(self.ran[1], ("1", [])) - self.assertFalse(self.overlapped) - - async def test_other_tickets_are_not_held_up(self): - self.gates[1] = asyncio.Event() - asyncio.create_task(self.runs.now("1", [])) - await settle() - await self.runs.now("2", []) - self.assertEqual([ticket for ticket, _ in self.ran], ["1", "2"]) - - async def test_nothing_is_kept_once_a_ticket_is_done(self): - await self.customer() - await self.clock.advance(60) - await self.runs.now("1", []) - self.assertEqual((self.runs.locks, self.runs.latest_event), ({}, {})) - - class TestSignatureTimestamps(unittest.TestCase): """The age check is only meaningful if the timestamp is read in the zone it was written in.""" diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index a90fdfa..7c953a4 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -774,6 +774,58 @@ def test_no_line_when_every_ticket_is_flagged(self): self.assertEqual(self.lines([finding(1)]), []) +class TestNonEnglishLine(unittest.TestCase): + """The new tickets not in English, linked on one line for `claude: english`.""" + + def line(self, findings, updated_ids=None): + lines = [l for l in digest_text(build_messages(findings, "acme", {}, + updated_ids=updated_ids)).splitlines() + if "not in English" in l] + return lines[0] if lines else None + + def test_it_lists_each_new_non_english_ticket_with_its_language(self): + line = self.line([finding(1), finding(2, language="German"), + finding(3, language="Russian", worth_looking_into=False)]) + self.assertIn("**2** new, not in English", line) + self.assertIn("[#2](https://acme.zendesk.com/agent/tickets/2) German", line) + self.assertIn("[#3](https://acme.zendesk.com/agent/tickets/3) Russian", line) + self.assertNotIn("[#1]", line) + + def test_it_leads_the_entries(self): + findings = [finding(1), finding(2, language="German")] + lines = [l for l in digest_text(build_messages(findings, "acme")).splitlines() + if "[#" in l] + self.assertIn("not in English", lines[0]) + + def test_a_ticket_reported_before_is_not_listed(self): + self.assertIsNone(self.line([finding(2, language="German")], updated_ids={2})) + + def test_unknown_or_english_variants_are_not_listed(self): + for language in ("", "unknown", "ENGLISH", "English (US)"): + with self.subTest(language=language): + self.assertIsNone(self.line([finding(1, language=language)])) + + def test_it_covers_no_ticket(self): + """Each is recorded by its own line, so one past the highlight cap stays + eligible for the next run.""" + findings = [finding(i, priority_rank=i, language="German") + for i in range(triage.MAX_HIGHLIGHTS + 2)] + messages, coverage = triage.build_messages(findings, "acme") + covered = set().union(*coverage) + self.assertNotIn(triage.MAX_HIGHLIGHTS + 1, covered) + self.assertIn(f"**{triage.MAX_HIGHLIGHTS + 2}** new, not in English", + digest_text(messages)) + + def test_links_stop_at_the_budget_and_the_rest_are_counted(self): + findings = [finding(27000 + i, language="German", worth_looking_into=False) + for i in range(200)] + text = digest_text(build_messages(findings, "acme")) + self.assertIn("**200** new, not in English", text) + line = next(l for l in text.splitlines() if "not in English" in l) + self.assertLess(len(line), triage.COLLAPSED_LINKS_CHARS + 100) + self.assertRegex(line, r"\+\d+ more") + + # ---- Parsing helpers ------------------------------------------------------- From 7bd2301ca29a14a56e7021ac643f0106362595b5 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 15 Oct 2026 16:27:44 +1100 Subject: [PATCH 09/12] fix(zendesk): link the oldest unflagged tickets first, and read en-US as English --- docs/jobs/zendesk-digest.md | 2 +- src/session_ops/zendesk/triage.py | 7 +++++-- tests/zendesk/test_triage.py | 9 ++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/jobs/zendesk-digest.md b/docs/jobs/zendesk-digest.md index acfec0a..7bfbcff 100644 --- a/docs/jobs/zendesk-digest.md +++ b/docs/jobs/zendesk-digest.md @@ -75,7 +75,7 @@ Likely duplicates: **push-notifications-not-delivered** Γ—5 (#27637, #27610, #27 | Category | The emoji from `CATEGORY_SPECS`, so it matches the tally line | | Platform | πŸ€– Android Β· 🍎 iOS Β· πŸ–₯️ desktop (all three) Β· 🌐 multiple Β· ❔ unknown | -The header accounts for the batch in full, so nothing is dropped silently. The backlog line is scoped exactly like the analysis β€” `status COLLAPSED_LINKS_CHARS: @@ -793,7 +795,8 @@ def is_english(finding): """Unknown counts as English: a thin classification is likelier than a ticket nobody here can read.""" language = (finding.get("language") or "").strip().lower() - return language in ("", "unknown") or language.startswith("english") + return (language in ("", "unknown", "en") or language.startswith("english") + or re.match(r"en[-_]", language) is not None) def build_non_english_line(findings, subdomain, updated_ids): diff --git a/tests/zendesk/test_triage.py b/tests/zendesk/test_triage.py index 7c953a4..14e265e 100644 --- a/tests/zendesk/test_triage.py +++ b/tests/zendesk/test_triage.py @@ -758,6 +758,13 @@ def test_a_ticket_past_the_budget_is_not_recorded(self): self.assertIn("**200** not flagged", text) self.assertRegex(text, r"\+\d+ more") + def test_the_oldest_are_linked_when_a_category_overflows(self): + findings = [self.quiet(27000 + i) for i in range(200)] + messages, coverage = triage.build_messages(list(reversed(findings)), "acme") + covered = set().union(*coverage) + self.assertIn(27000, covered) + self.assertNotIn(27199, covered) + def test_abuse_reports_and_highlights_are_not_repeated_on_it(self): findings = [finding(1), finding(2, category="abuse_report", severity="not_applicable"), self.quiet(3)] @@ -801,7 +808,7 @@ def test_a_ticket_reported_before_is_not_listed(self): self.assertIsNone(self.line([finding(2, language="German")], updated_ids={2})) def test_unknown_or_english_variants_are_not_listed(self): - for language in ("", "unknown", "ENGLISH", "English (US)"): + for language in ("", "unknown", "ENGLISH", "English (US)", "en", "EN-us", "en_GB"): with self.subTest(language=language): self.assertIsNone(self.line([finding(1, language=language)])) From 5ee5637a4fc73957e39f840e2a16e4e1657155f5 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 16 Oct 2026 09:33:42 +1100 Subject: [PATCH 10/12] feat(zendesk): answer only the newest message, and confirm before sending a stale draft Once support has replied on a ticket, `claude: draft` gives Claude the conversation so far and the customer's newest message separately, and asks it to answer only the newest one without repeating earlier replies. A draft with a public comment after it is no longer amended, since it was already sent or overtaken. `claude: reply` no longer sends a draft the customer has written after. It says so and waits for `claude: reply [n] anyway`. --- docs/jobs/zendesk-relay.md | 4 +- src/session_ops/zendesk/note_reply.py | 104 ++++++++++++++++++++---- tests/zendesk/test_note_reply.py | 113 +++++++++++++++++++++++++- 3 files changed, 203 insertions(+), 18 deletions(-) diff --git a/docs/jobs/zendesk-relay.md b/docs/jobs/zendesk-relay.md index 93f272e..7ca5515 100644 --- a/docs/jobs/zendesk-relay.md +++ b/docs/jobs/zendesk-relay.md @@ -40,8 +40,8 @@ there are several. | Command | What it does | Touches the customer | | --- | --- | --- | -| `claude: draft - ` | Compose the reply from the brief, in the requester's language. A second `draft` amends the one already there rather than starting over | no | -| `claude: reply [n]` | Publish the chosen option verbatim, status -> `pending` | **yes** | +| `claude: draft - ` | Compose the reply from the brief, in the requester's language. A second `draft` amends the one already there rather than starting over, unless a public comment came after it. Once support has replied, the draft answers only the customer's newest message | no | +| `claude: reply [n] [anyway]` | Publish the chosen option verbatim, status -> `pending`. If the customer wrote after the draft, it sends nothing until asked again with `anyway` | **yes** | | `claude: english` | Post the conversation, both sides, in English. Says so and writes nothing when the ticket is already English. Too long to send whole, it drops the oldest turns and says how many | no | | `claude: explain` | Post what support usually replied to this kind of ticket, what was actually done about it, and the caveats | no | | `claude: solve [reason]` | Solve without writing to the customer, for tickets that need no reply. The note records who decided and why | no comment, but **solving fires the CSAT automation** | diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index d9bb279..8721d77 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -26,7 +26,8 @@ draft Compose the reply from the brief, translate it into the requester's language, and post it as a private note with a back-translation. reply Send the most recent draft, verbatim. It never re-composes: what was - reviewed is what goes out, or the review means nothing. + reviewed is what goes out, or the review means nothing. If the customer + wrote after that draft, it asks for `claude: reply anyway` first. english Post the whole conversation, both sides, in English as a private note. Reads only; the customer never sees it. solve Solve the ticket, writing nothing to the customer. For the ones that @@ -73,6 +74,9 @@ # What Claude is given of the customer's own words, to answer in the right language # and at the right level of detail. CUSTOMER_SAMPLE_CHARS = 2000 +# What a follow-up draft is given of the conversation before the customer's newest +# message: enough to know what support already said, newest end kept. +EARLIER_CHARS = 4000 # A brief is a sentence or two. Anything much longer is a reply somebody has already # written, which does not need composing. BRIEF_CHARS = 2000 @@ -628,14 +632,25 @@ def render_precedent(cell, title, platform): Where the precedent and the brief disagree, THE BRIEF WINS AND THE PRECEDENT IS DROPPED. The agent read this ticket; the precedent did not. + + FOLLOW-UPS. When you are given THE CONVERSATION SO FAR, support has already + replied on this ticket. Answer only THE CUSTOMER'S NEWEST MESSAGE, as the next + message in a thread already under way. The customer has read every earlier + support reply: do not repeat its explanations, steps or apologies, and do not + acknowledge the original problem again. Refer back to an earlier point in a few + words, and only where the newest message needs it. A precedent describes a first + answer, so on a follow-up take from it only what the newest message asks about. """ ).strip() -def build_compose_prompt(sample, brief, previous=None, precedent=None): +def build_compose_prompt(sample, brief, previous=None, precedent=None, thread=None): """The customer's words, the brief, and β€” where they exist β€” the draft being amended and the precedent for this kind of ticket. + `thread` is (earlier, newest) from follow_up. It replaces the sample, which pools + every message the customer ever sent and so invites a reply to all of them. + Order matters twice over. Precedent comes first because it is background; the brief comes last because it is the instruction, and what is nearest the end is what governs. @@ -643,7 +658,14 @@ def build_compose_prompt(sample, brief, previous=None, precedent=None): parts = [] if precedent: parts += [precedent, ""] - parts += ["THE CUSTOMER'S OWN WORDS FROM THE TICKET:", sample] + if thread: + earlier, newest = thread + parts += ["THE CONVERSATION SO FAR, ALREADY ANSWERED:", earlier, "", + "THE CUSTOMER'S NEWEST MESSAGE, WHICH THIS REPLY ANSWERS:", + newest or "(none: the customer has not written since support's " + "last reply)"] + else: + parts += ["THE CUSTOMER'S OWN WORDS FROM THE TICKET:", sample] if previous: parts += ["", "A PREVIOUS DRAFT, WHICH THE AGENT IS AMENDING:", previous, "", "THE AGENT'S BRIEF, AS A CHANGE TO THAT DRAFT:", brief] @@ -678,10 +700,10 @@ def validate_composition(result): return result -def compose(model, sample, brief, previous=None, precedent=None): +def compose(model, sample, brief, previous=None, precedent=None, thread=None): return validate_composition(claude_cli.claude_cli_json( model, "medium", COMPOSE_SYSTEM, COMPOSE_SCHEMA, - build_compose_prompt(sample, brief, previous, precedent), + build_compose_prompt(sample, brief, previous, precedent, thread), COMPOSE_TIMEOUT_SECONDS, "the reply draft")) @@ -731,8 +753,9 @@ def build_draft_note(result, brief, comment_id, amended=False, return "".join(out) -def find_draft(comments, api_user): - """The options carried by the newest draft note, as {number: text}. {} if none. +def newest_draft(comments, api_user): + """(options, the comments newer than the draft note) for the newest draft note. + ({}, comments) if there is none. Options are {number: text}. Newest wins: an agent who did not like the first draft writes a new brief, and `reply` must offer what they last looked at. Restricted to notes this tool wrote, @@ -741,15 +764,41 @@ def find_draft(comments, api_user): api.fetch_comments returns newest first, so this walks the list as it comes. """ - for comment in comments: + for position, comment in enumerate(comments): if comment.get("public") or comment.get("author_id") != api_user: continue found = {int(number): body.strip() for number, body in OPTION_BLOCK.findall(comment_text(comment)) if body.strip()} if found: - return found - return {} + return found, comments[:position] + return {}, comments + + +def find_draft(comments, api_user): + """The options carried by the newest draft note, as {number: text}. {} if none.""" + return newest_draft(comments, api_user)[0] + + +def follow_up(session, subdomain, ticket, comments): + """(earlier, newest) once support has replied publicly on the ticket, else None. + + `earlier` is the conversation up to support's last reply, both sides, labelled. + `newest` is what the customer wrote after it, or "" when they have not written. + """ + public = [c for c in reversed(comments) if c.get("public")] # oldest first + authors = api.customer_authors(session, subdomain, ticket, public) + ours = [i for i, c in enumerate(public) if c.get("author_id") not in authors] + if not ours: + return None + body = lambda c: (c.get("body") or "").strip() + earlier = "\n\n".join( + f"{'Customer' if c.get('author_id') in authors else 'Support'}: {body(c)}" + for c in public[:ours[-1] + 1] if body(c)) + if len(earlier) > EARLIER_CHARS: + earlier = "…" + earlier[-EARLIER_CHARS:] + newest = "\n\n".join(body(c) for c in public[ours[-1] + 1:] if body(c)) + return earlier, clip(newest, CUSTOMER_SAMPLE_CHARS) def choose_option(options, asked): @@ -773,6 +822,9 @@ def choose_option(options, asked): OPTION_NUMBER = re.compile(r"^\s*#?(\d+)\b") +# `claude: reply anyway`, or `claude: reply 2 anyway`: send a draft the customer has +# written after. +CONFIRM = re.compile(r"\banyway\b", re.IGNORECASE) def asked_option(brief): @@ -808,6 +860,8 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr A second `draft` on a ticket that already has one is an amendment, not a fresh start: an agent writing "also mention X" wants the draft they just read plus X, and regenerating from the new brief alone would throw away the wording they kept. + Only while no public comment is newer than that draft: after one, the draft was + sent or overtaken, and amending it would send its paragraphs a second time. """ ticket_id = ticket["id"] comment_id, brief = command["id"], command["brief"] @@ -816,8 +870,11 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr "The brief was empty, so there was nothing to write. Add a private note " "reading claude: draft ", dry_run) return - shown = find_draft(comments, api_user) + shown, newer = newest_draft(comments, api_user) + if any(c.get("public") for c in newer): + shown = {} previous = "\n\n".join(f"Option {n}:\n{shown[n]}" for n in sorted(shown)) or None + thread = follow_up(session, subdomain, ticket, comments) sample = api.customer_text(session, subdomain, ticket, comments, CUSTOMER_SAMPLE_CHARS) @@ -840,7 +897,7 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr try: result = compose(model, clip(sample, CUSTOMER_SAMPLE_CHARS), brief, - previous, precedent) + previous, precedent, thread) except SystemExit as exc: say(session, subdomain, ticket_id, comment_id, f"The draft could not be written: {exc} Nothing was sent. To try again, " @@ -958,16 +1015,26 @@ def run_reply(session, subdomain, ticket, comments, command, api_user, dry_run): """ ticket_id = ticket["id"] comment_id = command["id"] - options = find_draft(comments, api_user) + options, newer = newest_draft(comments, api_user) if not options: say(session, subdomain, ticket_id, comment_id, "There is no draft on this ticket to send. Add a private note reading " "claude: draft ", dry_run) return - sending, complaint = choose_option(options, asked_option(command["brief"])) + asked = asked_option(command["brief"]) + sending, complaint = choose_option(options, asked) if complaint: say(session, subdomain, ticket_id, comment_id, complaint, dry_run, error=False) return + if customer_wrote_since(session, subdomain, ticket, comments, newer) \ + and not CONFIRM.search(command["brief"] or ""): + again = f"claude: reply {asked} anyway" if asked else "claude: reply anyway" + say(session, subdomain, ticket_id, comment_id, + "The customer has written since this draft was made, so it may not answer " + "what they said last. Nothing was sent. To answer them, add a private note " + f"reading claude: draft . To send this draft as it is, add a " + f"private note reading {again}", dry_run, error=False) + return author = api.fetch_user(session, subdomain, command["author"]) who = author.get("name") or f"user {command['author']}" if dry_run: @@ -988,6 +1055,15 @@ def run_reply(session, subdomain, ticket, comments, command, api_user, dry_run): print(f"#{ticket_id}: reply sent and status -> {REPLIED_STATUS}.") +def customer_wrote_since(session, subdomain, ticket, comments, newer): + """Whether any of `newer`, a slice of `comments`, is the customer writing.""" + if not any(c.get("public") for c in newer): + return False + authors = api.customer_authors(session, subdomain, ticket, + [c for c in comments if c.get("public")]) + return any(c.get("public") and c.get("author_id") in authors for c in newer) + + def already_english(turns, translated): """Whether the translation came back as the text it was given. diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index e018934..0c6fa48 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -862,6 +862,115 @@ def test_a_public_reply_is_sent_once_and_a_note_keeps_the_retries(self): self.assertEqual([kw.get("attempts") for _, _, kw in session.calls], [1, None]) +CUSTOMER = 42 +TICKET = {"id": 7, "requester_id": CUSTOMER} + + +def said(body, author=CUSTOMER, cid=1): + return comment(body, author=author, public=True, cid=cid) + + +class FollowUps(unittest.TestCase): + """Once support has replied, a draft answers what the customer said since, and a + reply does not send a draft the customer has written past.""" + + def draft(self, comments, brief="still broken, ask for logs"): + seen = {} + def fake(model, sample, brief, previous=None, precedent=None, thread=None): + seen.update(sample=sample, previous=previous, thread=thread) + return GERMAN + with Patched(note_reply, compose=fake): + session = fake_session(*[FakeResponse({"ticket": {}})]) + note_reply.run_draft(session, "sub", "model", TICKET, + [comment(f"claude: draft - {brief}", cid=20)] + comments, + {"id": 20, "author": AGENT, "action": "draft", + "brief": brief}, API_USER, dry_run=False) + return seen + + def test_a_draft_already_sent_is_not_amended(self): + """Amending it carries its paragraphs into the next reply, so the customer + gets the first answer again.""" + seen = self.draft([said("Immer noch kaputt", cid=13), + said("AnhΓ€nge werden 14 Tage gespeichert.", author=AGENT, cid=12), + draft_note("AnhΓ€nge werden 14 Tage gespeichert.", cid=11), + said("Meine Fotos sind weg", cid=10)]) + self.assertIsNone(seen["previous"]) + + def test_a_draft_newer_than_every_public_comment_is_still_amended(self): + seen = self.draft([draft_note("Zweiter Entwurf", cid=14), + said("Immer noch kaputt", cid=13), + said("Erste Antwort", author=AGENT, cid=12), + said("Meine Fotos sind weg", cid=10)]) + self.assertIn("Zweiter Entwurf", seen["previous"]) + + def test_the_newest_message_is_separated_from_what_was_answered(self): + seen = self.draft([said("Immer noch kaputt", cid=13), + said("Erste Antwort", author=AGENT, cid=12), + said("Meine Fotos sind weg", cid=10)]) + self.assertEqual(seen["thread"], ("Customer: Meine Fotos sind weg\n\n" + "Support: Erste Antwort", "Immer noch kaputt")) + + def test_a_first_reply_has_no_thread(self): + seen = self.draft([said("Meine Fotos sind weg", cid=10)]) + self.assertIsNone(seen["thread"]) + + def test_the_earlier_conversation_keeps_its_newest_end(self): + earlier, _ = note_reply.follow_up( + None, "sub", TICKET, [said("x", cid=13), + said("NEWEST " + "b" * 5000, author=AGENT, cid=12), + said("OLDEST " + "a" * 5000, cid=10)]) + self.assertLessEqual(len(earlier), note_reply.EARLIER_CHARS + 1) + self.assertNotIn("OLDEST", earlier) + self.assertTrue(earlier.endswith("b")) + + def test_the_follow_up_prompt_leaves_out_the_pooled_customer_words(self): + built = note_reply.build_compose_prompt( + "EVERYTHING THEY EVER SAID", "ask for logs", + thread=("Customer: a\n\nSupport: b", "still broken")) + self.assertNotIn("EVERYTHING THEY EVER SAID", built) + self.assertLess(built.index("Support: b"), built.index("still broken")) + self.assertLess(built.index("still broken"), built.index("ask for logs")) + + def test_the_prompt_answers_only_the_newest_message(self): + prompt = " ".join(note_reply.COMPOSE_SYSTEM.lower().split()) + self.assertIn("answer only the customer's newest message", prompt) + self.assertIn("do not repeat its explanations", prompt) + + def reply(self, comments, brief=""): + session = fake_session(*[FakeResponse({"user": {"id": AGENT, "name": "Audric"}}), + FakeResponse({"ticket": {}}), FakeResponse({"ticket": {}})]) + note_reply.run_reply(session, "sub", TICKET, + [comment(f"claude: reply {brief}", cid=20)] + comments, + {"id": 20, "author": AGENT, "action": "reply", "brief": brief}, + API_USER, dry_run=False) + return [kw["json"]["ticket"]["comment"] for m, u, kw in session.calls + if m == "PUT" and "/tags.json" not in u], \ + [kw["json"]["tags"] for m, u, kw in session.calls + if m == "PUT" and "/tags.json" in u] + + def test_a_draft_the_customer_wrote_after_is_not_sent(self): + written, tags = self.reply([said("Immer noch kaputt", cid=13), + draft_note("Alter Entwurf", cid=12)]) + self.assertEqual([c["public"] for c in written], [False]) + self.assertIn("claude: reply anyway", written[0]["html_body"]) + self.assertNotIn([note_reply.TAG_ERROR], tags) + + def test_the_confirmation_names_the_option_asked_for(self): + written, _ = self.reply([said("Immer noch kaputt", cid=13), + draft_note("eins", "zwei", cid=12)], brief="2") + self.assertIn("claude: reply 2 anyway", written[0]["html_body"]) + + def test_anyway_sends_it(self): + written, _ = self.reply([said("Immer noch kaputt", cid=13), + draft_note("eins", "zwei", cid=12)], brief="2 anyway") + self.assertEqual(written[-1], {"body": "zwei", "public": True}) + + def test_a_draft_newer_than_the_customer_is_sent(self): + written, _ = self.reply([draft_note("Neuer Entwurf", cid=14), + said("Immer noch kaputt", cid=13)]) + self.assertEqual(written[-1], {"body": "Neuer Entwurf", "public": True}) + + class DraftNote(unittest.TestCase): def test_carries_the_back_translation_for_a_foreign_ticket(self): note = note_reply.build_draft_note(GERMAN, "keeps 14 days", 42) @@ -932,7 +1041,7 @@ def test_a_second_draft_amends_the_first(self): """An agent writing "also mention X" wants the draft they just read plus X. Regenerating from the new brief alone throws away wording they kept.""" seen = {} - def fake(model, sample, brief, previous=None, precedent=None): + def fake(model, sample, brief, previous=None, precedent=None, thread=None): seen.update(brief=brief, previous=previous, precedent=precedent) return GERMAN with Patched(note_reply, compose=fake): @@ -971,7 +1080,7 @@ def fail(*a, **k): def test_a_first_draft_has_nothing_to_amend(self): seen = {} - def fake(model, sample, brief, previous=None, precedent=None): + def fake(model, sample, brief, previous=None, precedent=None, thread=None): seen.update(previous=previous, precedent=precedent) return GERMAN with Patched(note_reply, compose=fake): From 92494cc0d86e6d3e114d4872de0db696379bc50c Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 16 Oct 2026 09:51:36 +1100 Subject: [PATCH 11/12] fix(zendesk): tighten the reply confirmation and the follow-up prompt - `anyway` only confirms on the command line itself, since the brief runs to the end of the note. - `claude: reply` also asks for confirmation when the draft was already sent. - The follow-up prompt treats earlier support replies as context, not as a fact source, and covers a follow-up the customer has not prompted. - A draft looks up the customer's side once rather than twice. --- docs/jobs/zendesk-relay.md | 2 +- src/session_ops/zendesk/api.py | 5 +- src/session_ops/zendesk/note_reply.py | 58 ++++++++++++++-------- tests/zendesk/test_note_reply.py | 69 ++++++++++++++++++++++----- 4 files changed, 98 insertions(+), 36 deletions(-) diff --git a/docs/jobs/zendesk-relay.md b/docs/jobs/zendesk-relay.md index 7ca5515..9202968 100644 --- a/docs/jobs/zendesk-relay.md +++ b/docs/jobs/zendesk-relay.md @@ -41,7 +41,7 @@ there are several. | Command | What it does | Touches the customer | | --- | --- | --- | | `claude: draft - ` | Compose the reply from the brief, in the requester's language. A second `draft` amends the one already there rather than starting over, unless a public comment came after it. Once support has replied, the draft answers only the customer's newest message | no | -| `claude: reply [n] [anyway]` | Publish the chosen option verbatim, status -> `pending`. If the customer wrote after the draft, it sends nothing until asked again with `anyway` | **yes** | +| `claude: reply [n] [anyway]` | Publish the chosen option verbatim, status -> `pending`. If the customer wrote after the draft, or it was already sent, it sends nothing until asked again with `anyway` | **yes** | | `claude: english` | Post the conversation, both sides, in English. Says so and writes nothing when the ticket is already English. Too long to send whole, it drops the oldest turns and says how many | no | | `claude: explain` | Post what support usually replied to this kind of ticket, what was actually done about it, and the caveats | no | | `claude: solve [reason]` | Solve without writing to the customer, for tickets that need no reply. The note records who decided and why | no comment, but **solving fires the CSAT automation** | diff --git a/src/session_ops/zendesk/api.py b/src/session_ops/zendesk/api.py index 4b43a07..df5e2a9 100644 --- a/src/session_ops/zendesk/api.py +++ b/src/session_ops/zendesk/api.py @@ -281,7 +281,7 @@ def customer_authors(session, subdomain, ticket, comments): return customers or {requester} -def customer_text(session, subdomain, ticket, comments, limit): +def customer_text(session, subdomain, ticket, comments, limit, authors=None): """What the customer wrote, as the signal for which language to reply in. Their words only. An agent's earlier English reply is still text on the ticket, @@ -291,7 +291,8 @@ def customer_text(session, subdomain, ticket, comments, limit): # Public only. A private note is internal annotation β€” including the `claude:` # commands and the drafts this tool writes β€” and never the customer speaking. comments = [c for c in comments if c.get("public")] - authors = customer_authors(session, subdomain, ticket, comments) + if authors is None: + authors = customer_authors(session, subdomain, ticket, comments) subject = squash(ticket.get("subject")) parts = [] description = (ticket.get("description") or "").strip() diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index 8721d77..1821336 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -27,7 +27,8 @@ language, and post it as a private note with a back-translation. reply Send the most recent draft, verbatim. It never re-composes: what was reviewed is what goes out, or the review means nothing. If the customer - wrote after that draft, it asks for `claude: reply anyway` first. + wrote after that draft, or it was already sent, it asks for + `claude: reply [n] anyway` first. english Post the whole conversation, both sides, in English as a private note. Reads only; the customer never sees it. solve Solve the ticket, writing nothing to the customer. For the ones that @@ -634,12 +635,15 @@ def render_precedent(cell, title, platform): DROPPED. The agent read this ticket; the precedent did not. FOLLOW-UPS. When you are given THE CONVERSATION SO FAR, support has already - replied on this ticket. Answer only THE CUSTOMER'S NEWEST MESSAGE, as the next - message in a thread already under way. The customer has read every earlier + replied on this ticket. Answer only THE CUSTOMER'S NEWEST MESSAGE, or, when they + have not written since, write the follow-up the brief describes. Either way it is + the next message in a thread already under way. The customer has read every earlier support reply: do not repeat its explanations, steps or apologies, and do not acknowledge the original problem again. Refer back to an earlier point in a few words, and only where the newest message needs it. A precedent describes a first answer, so on a follow-up take from it only what the newest message asks about. + The conversation so far is context, not a fact source: repeat a fact from an + earlier support reply only if the brief gives it again. """ ).strip() @@ -663,7 +667,7 @@ def build_compose_prompt(sample, brief, previous=None, precedent=None, thread=No parts += ["THE CONVERSATION SO FAR, ALREADY ANSWERED:", earlier, "", "THE CUSTOMER'S NEWEST MESSAGE, WHICH THIS REPLY ANSWERS:", newest or "(none: the customer has not written since support's " - "last reply)"] + "last reply, so write the follow-up the brief describes)"] else: parts += ["THE CUSTOMER'S OWN WORDS FROM THE TICKET:", sample] if previous: @@ -780,14 +784,13 @@ def find_draft(comments, api_user): return newest_draft(comments, api_user)[0] -def follow_up(session, subdomain, ticket, comments): +def follow_up(comments, authors): """(earlier, newest) once support has replied publicly on the ticket, else None. `earlier` is the conversation up to support's last reply, both sides, labelled. `newest` is what the customer wrote after it, or "" when they have not written. """ public = [c for c in reversed(comments) if c.get("public")] # oldest first - authors = api.customer_authors(session, subdomain, ticket, public) ours = [i for i, c in enumerate(public) if c.get("author_id") not in authors] if not ours: return None @@ -822,9 +825,13 @@ def choose_option(options, asked): OPTION_NUMBER = re.compile(r"^\s*#?(\d+)\b") -# `claude: reply anyway`, or `claude: reply 2 anyway`: send a draft the customer has -# written after. -CONFIRM = re.compile(r"\banyway\b", re.IGNORECASE) +# `claude: reply anyway` or `claude: reply 2 anyway`, on the command line itself: the +# brief runs to the end of the note, where "anyway" can be ordinary prose. +CONFIRM = re.compile(r"^\s*(?:#?\d+\s+)?anyway\b", re.IGNORECASE) + + +def confirmed(brief): + return bool(CONFIRM.match((brief or "").split("\n", 1)[0])) def asked_option(brief): @@ -874,10 +881,12 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr if any(c.get("public") for c in newer): shown = {} previous = "\n\n".join(f"Option {n}:\n{shown[n]}" for n in sorted(shown)) or None - thread = follow_up(session, subdomain, ticket, comments) + authors = api.customer_authors(session, subdomain, ticket, + [c for c in comments if c.get("public")]) + thread = follow_up(comments, authors) sample = api.customer_text(session, subdomain, ticket, comments, - CUSTOMER_SAMPLE_CHARS) + CUSTOMER_SAMPLE_CHARS, authors) book = load_house() group, platform = tagged_placement(ticket) new_tags = [] @@ -1026,12 +1035,11 @@ def run_reply(session, subdomain, ticket, comments, command, api_user, dry_run): if complaint: say(session, subdomain, ticket_id, comment_id, complaint, dry_run, error=False) return - if customer_wrote_since(session, subdomain, ticket, comments, newer) \ - and not CONFIRM.search(command["brief"] or ""): + stale = stale_draft(session, subdomain, ticket, comments, newer, api_user) + if stale and not confirmed(command["brief"]): again = f"claude: reply {asked} anyway" if asked else "claude: reply anyway" say(session, subdomain, ticket_id, comment_id, - "The customer has written since this draft was made, so it may not answer " - "what they said last. Nothing was sent. To answer them, add a private note " + f"{stale} Nothing was sent. To write a new reply, add a private note " f"reading claude: draft . To send this draft as it is, add a " f"private note reading {again}", dry_run, error=False) return @@ -1055,13 +1063,23 @@ def run_reply(session, subdomain, ticket, comments, command, api_user, dry_run): print(f"#{ticket_id}: reply sent and status -> {REPLIED_STATUS}.") -def customer_wrote_since(session, subdomain, ticket, comments, newer): - """Whether any of `newer`, a slice of `comments`, is the customer writing.""" - if not any(c.get("public") for c in newer): - return False +def stale_draft(session, subdomain, ticket, comments, newer, api_user): + """Why the draft should not go out unconfirmed, or None. + + `newer` is the slice of `comments` after the draft. A public comment by the API + user there is a reply this tool sent, and the newest draft is the one it sent. + """ + public = [c for c in newer if c.get("public")] + if any(c.get("author_id") == api_user for c in public): + return "This draft has already been sent to the customer." + if not public: + return None authors = api.customer_authors(session, subdomain, ticket, [c for c in comments if c.get("public")]) - return any(c.get("public") and c.get("author_id") in authors for c in newer) + if any(c.get("author_id") in authors for c in public): + return ("The customer has written since this draft was made, so it may not " + "answer what they said last.") + return None def already_english(turns, translated): diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index 0c6fa48..a1fb7b7 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -874,13 +874,13 @@ class FollowUps(unittest.TestCase): """Once support has replied, a draft answers what the customer said since, and a reply does not send a draft the customer has written past.""" - def draft(self, comments, brief="still broken, ask for logs"): + def draft(self, comments, brief="still broken, ask for logs", lookups=()): seen = {} def fake(model, sample, brief, previous=None, precedent=None, thread=None): seen.update(sample=sample, previous=previous, thread=thread) return GERMAN with Patched(note_reply, compose=fake): - session = fake_session(*[FakeResponse({"ticket": {}})]) + session = fake_session(*lookups, FakeResponse({"ticket": {}})) note_reply.run_draft(session, "sub", "model", TICKET, [comment(f"claude: draft - {brief}", cid=20)] + comments, {"id": 20, "author": AGENT, "action": "draft", @@ -910,15 +910,34 @@ def test_the_newest_message_is_separated_from_what_was_answered(self): self.assertEqual(seen["thread"], ("Customer: Meine Fotos sind weg\n\n" "Support: Erste Antwort", "Immer noch kaputt")) + def test_a_channel_ticket_tells_the_sides_apart_by_role(self): + """On a DM the integration authors the customer's words, never the requester.""" + seen = self.draft([said("Immer noch kaputt", author=77, cid=13), + said("Erste Antwort", author=AGENT, cid=12), + said("Meine Fotos sind weg", author=77, cid=10)], + lookups=[FakeResponse({"user": {"role": "end-user"}}), + FakeResponse({"user": {"role": "agent"}})]) + self.assertEqual(seen["thread"], ("Customer: Meine Fotos sind weg\n\n" + "Support: Erste Antwort", "Immer noch kaputt")) + self.assertIn("Immer noch kaputt", seen["sample"]) + self.assertNotIn("Erste Antwort", seen["sample"]) + + def test_a_follow_up_nobody_asked_for_is_written_from_the_brief(self): + seen = self.draft([said("Erste Antwort", author=AGENT, cid=12), + said("Meine Fotos sind weg", cid=10)]) + self.assertEqual(seen["thread"][1], "") + built = note_reply.build_compose_prompt("x", "the fix shipped", + thread=seen["thread"]) + self.assertIn("write the follow-up the brief describes", built) + def test_a_first_reply_has_no_thread(self): seen = self.draft([said("Meine Fotos sind weg", cid=10)]) self.assertIsNone(seen["thread"]) def test_the_earlier_conversation_keeps_its_newest_end(self): earlier, _ = note_reply.follow_up( - None, "sub", TICKET, [said("x", cid=13), - said("NEWEST " + "b" * 5000, author=AGENT, cid=12), - said("OLDEST " + "a" * 5000, cid=10)]) + [said("x", cid=13), said("NEWEST " + "b" * 5000, author=AGENT, cid=12), + said("OLDEST " + "a" * 5000, cid=10)], {CUSTOMER}) self.assertLessEqual(len(earlier), note_reply.EARLIER_CHARS + 1) self.assertNotIn("OLDEST", earlier) self.assertTrue(earlier.endswith("b")) @@ -931,14 +950,10 @@ def test_the_follow_up_prompt_leaves_out_the_pooled_customer_words(self): self.assertLess(built.index("Support: b"), built.index("still broken")) self.assertLess(built.index("still broken"), built.index("ask for logs")) - def test_the_prompt_answers_only_the_newest_message(self): - prompt = " ".join(note_reply.COMPOSE_SYSTEM.lower().split()) - self.assertIn("answer only the customer's newest message", prompt) - self.assertIn("do not repeat its explanations", prompt) - - def reply(self, comments, brief=""): - session = fake_session(*[FakeResponse({"user": {"id": AGENT, "name": "Audric"}}), - FakeResponse({"ticket": {}}), FakeResponse({"ticket": {}})]) + def reply(self, comments, brief="", lookups=()): + session = fake_session(*lookups, + FakeResponse({"user": {"id": AGENT, "name": "Audric"}}), + FakeResponse({"ticket": {}}), FakeResponse({"ticket": {}})) note_reply.run_reply(session, "sub", TICKET, [comment(f"claude: reply {brief}", cid=20)] + comments, {"id": 20, "author": AGENT, "action": "reply", "brief": brief}, @@ -965,6 +980,34 @@ def test_anyway_sends_it(self): draft_note("eins", "zwei", cid=12)], brief="2 anyway") self.assertEqual(written[-1], {"body": "zwei", "public": True}) + def test_anyway_only_counts_on_the_command_line(self): + """The brief runs to the end of the note, where "anyway" is ordinary prose.""" + written, _ = self.reply([said("Immer noch kaputt", cid=13), + draft_note("eins", "zwei", cid=12)], + brief="2\nanyway, they said thanks") + self.assertEqual([c["public"] for c in written], [False]) + + def test_a_draft_already_sent_is_not_sent_again(self): + written, _ = self.reply([said("Alter Entwurf", author=API_USER, cid=14), + draft_note("Alter Entwurf", cid=12)]) + self.assertEqual([c["public"] for c in written], [False]) + self.assertIn("already been sent", written[0]["html_body"]) + + def test_a_support_comment_after_the_draft_does_not_block_it(self): + written, _ = self.reply([said("Wir schauen nach", author=AGENT, cid=13), + draft_note("Entwurf", cid=12), + said("Meine Fotos sind weg", cid=10)]) + self.assertEqual(written[-1], {"body": "Entwurf", "public": True}) + + def test_a_channel_customer_writing_after_the_draft_blocks_it(self): + written, _ = self.reply([said("Immer noch kaputt", author=77, cid=13), + draft_note("Entwurf", cid=12), + said("Erste Antwort", author=AGENT, cid=11)], + lookups=[FakeResponse({"user": {"role": "end-user"}}), + FakeResponse({"user": {"role": "agent"}})]) + self.assertEqual([c["public"] for c in written], [False]) + self.assertIn("customer has written", written[0]["html_body"]) + def test_a_draft_newer_than_the_customer_is_sent(self): written, _ = self.reply([draft_note("Neuer Entwurf", cid=14), said("Immer noch kaputt", cid=13)]) From a8710341f78a5d418aefeee260e1b4f90b3c7902 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 16 Oct 2026 09:55:47 +1100 Subject: [PATCH 12/12] fix(zendesk): keep a draft open to amend after an agent's manual reply Only a reply this tool sent, or the customer writing, makes the next draft start fresh. --- docs/jobs/zendesk-relay.md | 2 +- src/session_ops/zendesk/note_reply.py | 11 ++++++----- tests/zendesk/test_note_reply.py | 16 ++++++++++++++-- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/docs/jobs/zendesk-relay.md b/docs/jobs/zendesk-relay.md index 9202968..cf94bbf 100644 --- a/docs/jobs/zendesk-relay.md +++ b/docs/jobs/zendesk-relay.md @@ -40,7 +40,7 @@ there are several. | Command | What it does | Touches the customer | | --- | --- | --- | -| `claude: draft - ` | Compose the reply from the brief, in the requester's language. A second `draft` amends the one already there rather than starting over, unless a public comment came after it. Once support has replied, the draft answers only the customer's newest message | no | +| `claude: draft - ` | Compose the reply from the brief, in the requester's language. A second `draft` amends the one already there rather than starting over, unless it was sent or the customer wrote after it. Once support has replied, the draft answers only the customer's newest message | no | | `claude: reply [n] [anyway]` | Publish the chosen option verbatim, status -> `pending`. If the customer wrote after the draft, or it was already sent, it sends nothing until asked again with `anyway` | **yes** | | `claude: english` | Post the conversation, both sides, in English. Says so and writes nothing when the ticket is already English. Too long to send whole, it drops the oldest turns and says how many | no | | `claude: explain` | Post what support usually replied to this kind of ticket, what was actually done about it, and the caveats | no | diff --git a/src/session_ops/zendesk/note_reply.py b/src/session_ops/zendesk/note_reply.py index 1821336..14c2748 100644 --- a/src/session_ops/zendesk/note_reply.py +++ b/src/session_ops/zendesk/note_reply.py @@ -867,8 +867,8 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr A second `draft` on a ticket that already has one is an amendment, not a fresh start: an agent writing "also mention X" wants the draft they just read plus X, and regenerating from the new brief alone would throw away the wording they kept. - Only while no public comment is newer than that draft: after one, the draft was - sent or overtaken, and amending it would send its paragraphs a second time. + Not once the draft was sent or the customer wrote after it: amending then would + send its paragraphs a second time. A manual reply by an agent leaves it open. """ ticket_id = ticket["id"] comment_id, brief = command["id"], command["brief"] @@ -877,12 +877,13 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr "The brief was empty, so there was nothing to write. Add a private note " "reading claude: draft ", dry_run) return + authors = api.customer_authors(session, subdomain, ticket, + [c for c in comments if c.get("public")]) shown, newer = newest_draft(comments, api_user) - if any(c.get("public") for c in newer): + if any(c.get("public") and c.get("author_id") in authors | {api_user} + for c in newer): shown = {} previous = "\n\n".join(f"Option {n}:\n{shown[n]}" for n in sorted(shown)) or None - authors = api.customer_authors(session, subdomain, ticket, - [c for c in comments if c.get("public")]) thread = follow_up(comments, authors) sample = api.customer_text(session, subdomain, ticket, comments, diff --git a/tests/zendesk/test_note_reply.py b/tests/zendesk/test_note_reply.py index a1fb7b7..adb5896 100644 --- a/tests/zendesk/test_note_reply.py +++ b/tests/zendesk/test_note_reply.py @@ -890,12 +890,24 @@ def fake(model, sample, brief, previous=None, precedent=None, thread=None): def test_a_draft_already_sent_is_not_amended(self): """Amending it carries its paragraphs into the next reply, so the customer gets the first answer again.""" - seen = self.draft([said("Immer noch kaputt", cid=13), - said("AnhΓ€nge werden 14 Tage gespeichert.", author=AGENT, cid=12), + seen = self.draft([said("AnhΓ€nge werden 14 Tage gespeichert.", author=API_USER, + cid=12), draft_note("AnhΓ€nge werden 14 Tage gespeichert.", cid=11), said("Meine Fotos sind weg", cid=10)]) self.assertIsNone(seen["previous"]) + def test_a_draft_the_customer_wrote_after_is_not_amended(self): + seen = self.draft([said("Immer noch kaputt", cid=13), + draft_note("Alter Entwurf", cid=11), + said("Meine Fotos sind weg", cid=10)]) + self.assertIsNone(seen["previous"]) + + def test_a_manual_agent_reply_leaves_the_draft_open_to_amend(self): + seen = self.draft([said("Wir schauen nach", author=AGENT, cid=12), + draft_note("Entwurf", cid=11), + said("Meine Fotos sind weg", cid=10)]) + self.assertIn("Entwurf", seen["previous"]) + def test_a_draft_newer_than_every_public_comment_is_still_amended(self): seen = self.draft([draft_note("Zweiter Entwurf", cid=14), said("Immer noch kaputt", cid=13),