From bca88211426a0c6094cf480c469effe45ea77f71 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 24 Sep 2026 16:14:08 +1000 Subject: [PATCH 1/5] feat(shared): text helpers, a dedup Tracker, get_env defaults and richer fakes shared.text holds clip, squash and window_label, the string work every tool does before it talks to anything. shared.state.Tracker binds a digest's schema version, item keys and retention once, so the partition and the record-building both digests carry live in one place, and shared.discord.delivered_ids names what a partial post covered. get_env takes a default for settings that are optional with a fallback, and the fakes gain raise_for_status, a session context manager and an Env swap so no test file has to define its own. --- shared/discord.py | 10 ++++---- shared/env.py | 14 +++++++++--- shared/state.py | 52 ++++++++++++++++++++++++++++++++++++++++++ shared/test_discord.py | 8 +++++++ shared/test_env.py | 31 +++++++++++++++++++++++++ shared/test_state.py | 33 +++++++++++++++++++++++++++ shared/test_text.py | 37 ++++++++++++++++++++++++++++++ shared/testing.py | 36 +++++++++++++++++++++++++++++ shared/text.py | 21 +++++++++++++++++ 9 files changed, 234 insertions(+), 8 deletions(-) create mode 100644 shared/test_env.py create mode 100644 shared/test_text.py create mode 100644 shared/text.py diff --git a/shared/discord.py b/shared/discord.py index d9662e1..4fceaae 100644 --- a/shared/discord.py +++ b/shared/discord.py @@ -22,11 +22,6 @@ MAX_MESSAGE_TEXT_CHARS = 4000 -def clip(text, limit): - text = (text or "").strip() - return text if len(text) <= limit else text[: limit - 1] + "…" - - def text_display(content): return {"type": TEXT_DISPLAY, "content": content} @@ -94,6 +89,11 @@ def messages_from_entries(header, entries, max_items, max_chars=MAX_MESSAGE_TEXT return messages, coverage +def delivered_ids(coverage, posted): + """The ids carried by the first `posted` messages: what Discord accepted.""" + return set().union(*coverage[:posted]) if posted else set() + + def components_webhook_url(webhook_url): """The webhook, told to respect the components field, which it ignores without.""" parts = urlsplit(webhook_url) diff --git a/shared/env.py b/shared/env.py index 4d6fcbf..278ce91 100644 --- a/shared/env.py +++ b/shared/env.py @@ -2,12 +2,20 @@ import sys -def get_env(name, cli_value=None, required=True): - if cli_value: - return cli_value +def get_env(name, override=None, required=True, default=None): + """A flag's value if given, else the environment, else `default`. + + An empty value counts as unset. A required setting that is missing exits with + the variable to set, so an unattended run fails on config before it spends + anything. + """ + if override: + return override value = os.environ.get(name) if value: return value + if default is not None: + return default if required: sys.exit(f"Missing required config: set the {name} environment variable (or pass the matching flag).") return None diff --git a/shared/state.py b/shared/state.py index 90a8bb5..f06d8af 100644 --- a/shared/state.py +++ b/shared/state.py @@ -9,6 +9,7 @@ from datetime import datetime, timedelta, timezone STAMP = "%Y-%m-%dT%H:%M:%SZ" +DEFAULT_RETENTION_DAYS = 30 def empty_state(version): @@ -71,3 +72,54 @@ def save_state(path, state, records, retention_days, version): json.dump({"version": version, "updated_at": stamp, "seen": kept}, handle, indent=2) os.replace(temporary, path) # atomic: a crash mid-write cannot corrupt the state return len(kept), len(seen) - len(kept) + + +class Tracker: + """One digest's dedup: its schema version, and how an item is keyed and stamped. + + `key_of(item)` names an item across runs and `activity_of(item)` is the value a + re-report is judged against, stored under `field`. `describe(item)`, if given, + adds fields written for whoever opens the file and never read back. + """ + + def __init__(self, version, noun, key_of, activity_of, field, describe=None, + retention_days=DEFAULT_RETENTION_DAYS): + self.version = version + self.noun = noun + self.key_of = key_of + self.activity_of = activity_of + self.field = field + self.describe = describe + self.retention_days = retention_days + + def empty(self): + return empty_state(self.version) + + def load(self, path): + return load_state(path, self.version, self.noun) + + def partition(self, items, state): + """Split into (new, changed, unchanged) against what was last reported.""" + seen = state.get("seen", {}) + new, changed, unchanged = [], [], [] + for item in items: + previous = seen.get(self.key_of(item)) + if previous is None: + new.append(item) + elif previous.get(self.field) != self.activity_of(item): + changed.append(item) + else: + unchanged.append(item) + return new, changed, unchanged + + def save(self, path, state, reported, retention_days=None): + """Record `reported` as seen now. Returns (kept, pruned).""" + if retention_days is None: + retention_days = self.retention_days + records = {} + for item in reported: + record = {self.field: self.activity_of(item)} + if self.describe: + record.update(self.describe(item)) + records[self.key_of(item)] = record + return save_state(path, state, records, retention_days, self.version) diff --git a/shared/test_discord.py b/shared/test_discord.py index 41330c4..da0770d 100644 --- a/shared/test_discord.py +++ b/shared/test_discord.py @@ -126,5 +126,13 @@ def test_the_header_counts_against_the_first_budget(self): self.assertEqual(len(messages), 2) +class TestDeliveredIds(unittest.TestCase): + def test_only_the_accepted_messages_count(self): + coverage = [{1, 2}, {3}, {4}] + self.assertEqual(discord.delivered_ids(coverage, 2), {1, 2, 3}) + self.assertEqual(discord.delivered_ids(coverage, 0), set()) + self.assertEqual(discord.delivered_ids([], 0), set()) + + if __name__ == "__main__": unittest.main() diff --git a/shared/test_env.py b/shared/test_env.py new file mode 100644 index 0000000..feff7bc --- /dev/null +++ b/shared/test_env.py @@ -0,0 +1,31 @@ +"""Run with: cd shared && python -m unittest discover""" +import os +import sys +import unittest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from shared.env import get_env # noqa: E402 +from shared.testing import Env # noqa: E402 + + +class TestGetEnv(unittest.TestCase): + def test_precedence_is_flag_then_environment_then_default(self): + with Env(SHARED_TEST_VALUE="from-env"): + self.assertEqual(get_env("SHARED_TEST_VALUE", "from-flag"), "from-flag") + self.assertEqual(get_env("SHARED_TEST_VALUE", default="d"), "from-env") + with Env(SHARED_TEST_VALUE=None): + self.assertEqual(get_env("SHARED_TEST_VALUE", default="d"), "d") + + def test_empty_counts_as_unset(self): + with Env(SHARED_TEST_VALUE=""): + self.assertEqual(get_env("SHARED_TEST_VALUE", "", default="d"), "d") + self.assertIsNone(get_env("SHARED_TEST_VALUE", required=False)) + + def test_missing_and_required_exits_naming_the_variable(self): + with Env(SHARED_TEST_VALUE=None), self.assertRaises(SystemExit) as caught: + get_env("SHARED_TEST_VALUE") + self.assertIn("SHARED_TEST_VALUE", str(caught.exception)) + + +if __name__ == "__main__": + unittest.main() diff --git a/shared/test_state.py b/shared/test_state.py index 1097085..16485e3 100644 --- a/shared/test_state.py +++ b/shared/test_state.py @@ -81,5 +81,38 @@ def test_the_directory_is_created_and_the_write_is_atomic(self): self.assertEqual(sorted(os.listdir(os.path.dirname(self.path))), ["seen.json"]) +class TestTracker(unittest.TestCase): + def setUp(self): + self.directory = tempfile.TemporaryDirectory() + self.addCleanup(self.directory.cleanup) + self.path = os.path.join(self.directory.name, "seen.json") + self.tracker = dedup.Tracker( + VERSION, "thing", key_of=lambda t: str(t["id"]), + activity_of=lambda t: t["touched"], field="touched", + describe=lambda t: {"name": t.get("name", "")}) + + def test_partition_against_what_was_reported(self): + state = self.tracker.empty() + with contextlib.redirect_stdout(io.StringIO()): + self.tracker.save(self.path, state, [{"id": 1, "touched": "A"}, + {"id": 2, "touched": "A"}], 30) + state = self.tracker.load(self.path) + new, changed, unchanged = self.tracker.partition( + [{"id": 1, "touched": "B"}, {"id": 2, "touched": "A"}, {"id": 3, "touched": "A"}], + state) + self.assertEqual([t["id"] for t in new], [3]) + self.assertEqual([t["id"] for t in changed], [1]) + self.assertEqual([t["id"] for t in unchanged], [2]) + + def test_describe_fields_are_written_beside_the_activity(self): + with contextlib.redirect_stdout(io.StringIO()): + self.tracker.save(self.path, self.tracker.empty(), + [{"id": 1, "touched": "A", "name": "one"}], 30) + record = self.tracker.load(self.path)["seen"]["1"] + self.assertEqual(record["touched"], "A") + self.assertEqual(record["name"], "one") + self.assertIn("last_reported", record) + + if __name__ == "__main__": unittest.main() diff --git a/shared/test_text.py b/shared/test_text.py new file mode 100644 index 0000000..a938636 --- /dev/null +++ b/shared/test_text.py @@ -0,0 +1,37 @@ +"""Run with: cd shared && python -m unittest discover""" +import os +import sys +import unittest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from shared import text # noqa: E402 + + +class TestClip(unittest.TestCase): + def test_fits_within_the_limit_with_the_ellipsis(self): + self.assertEqual(text.clip("abcdef", 4), "abc…") + self.assertEqual(len(text.clip("abcdef", 4)), 4) + + def test_short_text_and_none_pass_through(self): + self.assertEqual(text.clip(" abc ", 10), "abc") + self.assertEqual(text.clip(None, 10), "") + + +class TestSquash(unittest.TestCase): + def test_collapses_every_kind_of_whitespace(self): + self.assertEqual(text.squash(" a\n\t b c "), "a b c") + self.assertEqual(text.squash(None), "") + + +class TestWindowLabel(unittest.TestCase): + def test_whole_days_read_as_days(self): + self.assertEqual(text.window_label(24), "1 day") + self.assertEqual(text.window_label(72), "3 days") + + def test_anything_else_stays_in_hours(self): + self.assertEqual(text.window_label(36), "36h") + self.assertEqual(text.window_label(12), "12h") + + +if __name__ == "__main__": + unittest.main() diff --git a/shared/testing.py b/shared/testing.py index c5e3677..da98e4b 100644 --- a/shared/testing.py +++ b/shared/testing.py @@ -1,5 +1,6 @@ """Fakes for the tests of every script that talks HTTP through `shared`.""" import json +import os import time import requests @@ -17,6 +18,10 @@ def __init__(self, payload, status_code=200, retry_after="0"): def json(self): return self._payload + def raise_for_status(self): + if self.status_code >= 400: + raise requests.HTTPError(f"{self.status_code}", response=self) + class NonJsonResponse(FakeResponse): """A 200 whose body isn't JSON — a proxy error page, say.""" @@ -47,6 +52,12 @@ def request(self, method, url, **kwargs): raise item return item + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + class Patched: """Swap module attributes for the duration of a block, then put them back.""" @@ -86,3 +97,28 @@ def __enter__(self): def __exit__(self, *exc): time.sleep = self._real return False + + +class Env: + """Replace process environment variables for the duration of a block. + + A value of None unsets the variable, so a test can exercise the missing case + on a machine where the real setting is present. + """ + + def __init__(self, **overrides): + self.overrides = overrides + + def __enter__(self): + self.saved = dict(os.environ) + for key, value in self.overrides.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value + return self + + def __exit__(self, *exc): + os.environ.clear() + os.environ.update(self.saved) + return False diff --git a/shared/text.py b/shared/text.py new file mode 100644 index 0000000..af14e6b --- /dev/null +++ b/shared/text.py @@ -0,0 +1,21 @@ +"""String helpers with no transport in them: prompts, ticket fields and Discord lines +all clip and compare text the same way.""" +import re + + +def clip(text, limit): + text = (text or "").strip() + return text if len(text) <= limit else text[: limit - 1] + "…" + + +def squash(value): + """Collapse whitespace so two renderings of the same prose compare equal.""" + return re.sub(r"\s+", " ", value or "").strip() + + +def window_label(hours): + """`3 days` for a whole number of days, else `36h`.""" + if hours % 24 == 0 and hours >= 24: + days = hours // 24 + return f"{days} day{'s' if days > 1 else ''}" + return f"{hours}h" From c452c4a44a1d4d8fdacaeaf69a0aa3d77835dd5d Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 24 Sep 2026 16:20:28 +1000 Subject: [PATCH 2/5] refactor: import shared directly, and dedup both digests through one Tracker note_reply, resolve_reviews and the alert reached the shared helpers as attributes of triage, so a 1900-line classifier was the import path for a retry loop; relay imported it and used nothing. Each now imports shared itself, and the tests take their fakes from shared.testing rather than from test_triage. The digest and the triage carried the same new/changed/unchanged partition, state wrappers and window label side by side. Both now configure a shared.state.Tracker and take clip, squash and window_label from shared.text; the crowdin report's private clip and its unused snippet go the same way. --- crowdin/report_multiple_translations.py | 11 +--- crowdin/test_report_multiple_translations.py | 22 ++----- deploy/alert.py | 16 ++--- github_prs/digest.py | 59 ++++------------- github_prs/test_digest.py | 16 ++--- zendesk_triage/note_reply.py | 29 +++++---- zendesk_triage/relay.py | 3 - zendesk_triage/resolve_reviews.py | 22 ++++--- zendesk_triage/test_note_reply.py | 10 +-- zendesk_triage/test_relay.py | 25 ++------ zendesk_triage/test_resolve_reviews.py | 13 ++-- zendesk_triage/test_triage.py | 66 +++++++++----------- zendesk_triage/triage.py | 66 +++++--------------- 13 files changed, 122 insertions(+), 236 deletions(-) diff --git a/crowdin/report_multiple_translations.py b/crowdin/report_multiple_translations.py index 18e1c37..f31e586 100644 --- a/crowdin/report_multiple_translations.py +++ b/crowdin/report_multiple_translations.py @@ -50,6 +50,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from shared import discord, retry # noqa: E402 +from shared.text import clip # noqa: E402 API = "https://api.crowdin.com/api/v2" DEFAULT_PROJECT = "618696" @@ -111,16 +112,6 @@ def user_label(u): return f"{u.get('id')}:{u.get('username') or u.get('fullName') or '?'}" -def snippet(text, n=70): - text = (text or "").replace("\n", " ") - return text[:n] + ("…" if len(text) > n else "") - - -def clip(text, n): - text = text or "" - return text if len(text) <= n else text[: n - 1] + "…" - - # --------------------------------------------------------------------------- # # Scanning # --------------------------------------------------------------------------- # diff --git a/crowdin/test_report_multiple_translations.py b/crowdin/test_report_multiple_translations.py index 50a0381..6f8f347 100644 --- a/crowdin/test_report_multiple_translations.py +++ b/crowdin/test_report_multiple_translations.py @@ -15,29 +15,15 @@ from shared.testing import FakeResponse, FakeSession, NoSleep, Patched # noqa: E402 -class ApiResponse(FakeResponse): - def raise_for_status(self): - if self.status_code >= 400: - raise requests.HTTPError(f"{self.status_code}", response=self) - - -class WebhookSession(FakeSession): - def __enter__(self): - return self - - def __exit__(self, *exc): - return False - - class TestRequestWithRetry(unittest.TestCase): def test_a_client_error_raises_rather_than_returning(self): """Callers read the body straight off the response, so a 4xx has to stop them.""" with self.assertRaises(requests.HTTPError): - report.request_with_retry(FakeSession([ApiResponse({}, status_code=404)]), + report.request_with_retry(FakeSession([FakeResponse({}, status_code=404)]), "GET", "https://x") def test_crowdins_budget_is_ten_attempts_at_sixty_seconds(self): - session = FakeSession([ApiResponse({}, status_code=503)] * 9 + [ApiResponse({"ok": 1})]) + session = FakeSession([FakeResponse({}, status_code=503)] * 9 + [FakeResponse({"ok": 1})]) with NoSleep(): self.assertEqual(report.request_with_retry(session, "GET", "https://x").json(), {"ok": 1}) self.assertEqual(len(session.calls), 10) @@ -46,7 +32,7 @@ def test_crowdins_budget_is_ten_attempts_at_sixty_seconds(self): class TestPostToDiscord(unittest.TestCase): def post(self, responses, messages): - session = WebhookSession(responses) + session = FakeSession(responses) with Patched(report.requests, Session=lambda: session), \ contextlib.redirect_stdout(io.StringIO()): report.post_to_discord("https://hook", messages) @@ -64,7 +50,7 @@ def test_a_rejection_posts_a_plain_warning_then_exits(self): self.assertIn("1 of 3", str(caught.exception)) def test_the_warning_is_plain_content_the_webhook_cannot_reject_for_size(self): - session = WebhookSession([FakeResponse({}, status_code=400), FakeResponse({}, status_code=204)]) + session = FakeSession([FakeResponse({}, status_code=400), FakeResponse({}, status_code=204)]) with Patched(report.requests, Session=lambda: session), \ contextlib.redirect_stdout(io.StringIO()), self.assertRaises(SystemExit): report.post_to_discord("https://hook", [{"embeds": [{"title": "x" * 9000}]}]) diff --git a/deploy/alert.py b/deploy/alert.py index e8254b7..a773c21 100644 --- a/deploy/alert.py +++ b/deploy/alert.py @@ -25,10 +25,13 @@ import subprocess import sys -sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname( - os.path.abspath(__file__))), "zendesk_triage")) +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, ROOT) +sys.path.insert(0, os.path.join(ROOT, "zendesk_triage")) import requests # noqa: E402 -import triage # noqa: E402 +from shared.discord import post_to_discord # noqa: E402 +from shared.env import get_env # noqa: E402 +from triage import CLAUDE_CLI # noqa: E402 # A dead login reads as a broken job unless the alert names it: the job is fine and @@ -37,7 +40,7 @@ # checked: Zendesk's own 401 body says "Couldn't authenticate you". AUTH_SIGNATURES = ("oauth", "/login", "authenticate", "invalid api key", "unauthorized", "credit balance", "signed in") -CLI_FAILURE_PREFIX = f"{triage.CLAUDE_CLI} exited" +CLI_FAILURE_PREFIX = f"{CLAUDE_CLI} exited" # Where the Claude Code CLI lives for the account the units run as; see # deploy/README.md. Spelled out because an alert that says "log in again" without # saying how sends whoever is on call to the README first. @@ -104,13 +107,12 @@ def main(): args = [arg.strip() for arg in sys.argv[1:]] if not args or len(args) > 2 or not args[0]: sys.exit("usage: alert.py [journal-unit]") - webhook = (os.environ.get("ALERT_DISCORD_WEBHOOK_URL") - or triage.get_env("ZENDESK_DISCORD_WEBHOOK_URL")) + webhook = get_env("ZENDESK_DISCORD_WEBHOOK_URL", os.environ.get("ALERT_DISCORD_WEBHOOK_URL")) detail = last_job_line(journal_tail(args[-1])) message = build_message(args[0], socket.gethostname(), *args[1:], detail=detail) # A fresh session, never a Zendesk one — that carries the API-token auth header, # and Discord has no business receiving it. - if not triage.post_to_discord(requests.Session(), webhook, [{"content": message}]): + if not post_to_discord(requests.Session(), webhook, [{"content": message}]): sys.exit("Could not post the failure to Discord.") diff --git a/github_prs/digest.py b/github_prs/digest.py index e6be7e8..78a0068 100755 --- a/github_prs/digest.py +++ b/github_prs/digest.py @@ -48,16 +48,16 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from shared import discord, state as dedup # noqa: E402 -from shared.discord import MAX_MESSAGE_TEXT_CHARS, clip # noqa: E402 +from shared.discord import MAX_MESSAGE_TEXT_CHARS # noqa: E402 from shared.env import get_env # noqa: E402 from shared.retry import request_with_retry # noqa: E402 +from shared.text import clip, window_label # noqa: E402 API = "https://api.github.com" DEFAULT_ORG = "session-foundation" # Three days, because the timer runs on weekdays: Monday's window has to reach back # over the weekend. Overlap between consecutive runs is what --state absorbs. DEFAULT_WINDOW_HOURS = 72 -DEFAULT_RETENTION_DAYS = 30 STATE_VERSION = 1 # The Search API caps a query at 1000 results and returns 422 for any page past it # (at per_page=100 that is page 11). Past the cap the digest reports truncation @@ -197,39 +197,11 @@ def activity_key(pr): # ---- Dedup state ----------------------------------------------------------- - -def empty_state(): - return dedup.empty_state(STATE_VERSION) - - -def load_state(path): - return dedup.load_state(path, STATE_VERSION, "PR") - - -def partition_by_state(prs, state): - """Split into (new, changed, unchanged) against what was last reported.""" - seen = state.get("seen", {}) - new, changed, unchanged = [], [], [] - for pr in prs: - previous = seen.get(pr_id(pr)) - if previous is None: - new.append(pr) - elif previous.get("updated_at") != activity_key(pr): - changed.append(pr) - else: - unchanged.append(pr) - return new, changed, unchanged - - -def save_state(path, state, reported, retention_days=DEFAULT_RETENTION_DAYS): - """Record `reported` as seen. Returns (kept, pruned).""" - records = {pr_id(pr): {"updated_at": activity_key(pr), - # Not read back. The file is the first thing anyone opens - # when the digest reports the wrong thing, and an id - # alone identifies nothing. - "pr": f"{repo_name(pr)}#{pr.get('number')}"} - for pr in reported} - return dedup.save_state(path, state, records, retention_days, STATE_VERSION) +# The repo#number is never read back. The file is the first thing anyone opens when +# the digest reports the wrong thing, and an id alone identifies nothing. +STATE = dedup.Tracker(STATE_VERSION, "PR", pr_id, activity_key, "updated_at", + describe=lambda pr: {"pr": f"{repo_name(pr)}#{pr.get('number')}"}) +DEFAULT_RETENTION_DAYS = STATE.retention_days # ---- Discord rendering ----------------------------------------------------- @@ -309,13 +281,6 @@ def group_by_repo(new, updated, now, max_chars=MAX_MESSAGE_TEXT_CHARS): return [block for blocks, _ in repos for block in blocks] -def window_label(hours): - if hours % 24 == 0 and hours >= 24: - days = hours // 24 - return f"{days} day{'s' if days > 1 else ''}" - return f"{hours}h" - - def build_header(new, updated, backlog, window_hours, truncated): lines = [f"**Contributor pull requests** · last {window_label(window_hours)}"] if new or updated: @@ -362,7 +327,7 @@ def main(): if args.window_hours < 1: sys.exit("--window-hours must be at least 1.") - org = args.org or os.environ.get("GITHUB_PRS_ORG") or DEFAULT_ORG + org = get_env("GITHUB_PRS_ORG", args.org, default=DEFAULT_ORG) token = get_env("GITHUB_PRS_TOKEN", args.token) webhook = get_env("GITHUB_PRS_DISCORD_WEBHOOK_URL", args.webhook, required=not args.dry_run) @@ -374,8 +339,8 @@ def main(): prs = contributor_prs(items, repos, maintainers) now = datetime.now(timezone.utc) - state = load_state(args.state) - new, changed, unchanged = partition_by_state( + state = STATE.load(args.state) + new, changed, unchanged = STATE.partition( in_window(prs, now - timedelta(hours=args.window_hours)), state) print(f"{len(items)} open PRs in {org}, {len(prs)} from contributors across " f"{len(repos)} repos: {len(new)} new, {len(changed)} changed since last " @@ -391,9 +356,9 @@ def main(): discord.components_webhook_url(webhook), messages) # Only what Discord accepted. A PR in a message that never landed stays eligible. if args.state: - landed = set().union(*coverage[:posted]) if posted else set() + landed = discord.delivered_ids(coverage, posted) reported = [pr for pr in new + changed if pr_id(pr) in landed] - kept, pruned = save_state(args.state, state, reported, + kept, pruned = STATE.save(args.state, state, reported, args.state_retention_days) print(f"State: {len(reported)} recorded, {kept} tracked " f"({pruned} pruned beyond {args.state_retention_days} days).") diff --git a/github_prs/test_digest.py b/github_prs/test_digest.py index 0cc2b82..5e9043d 100644 --- a/github_prs/test_digest.py +++ b/github_prs/test_digest.py @@ -93,20 +93,20 @@ def state(self, *records): "seen": {key: {"updated_at": stamp} for key, stamp in records}} def test_a_pr_never_reported_is_new(self): - new, changed, unchanged = digest.partition_by_state([pr(1)], digest.empty_state()) + new, changed, unchanged = digest.STATE.partition([pr(1)], digest.STATE.empty()) self.assertEqual([p["number"] for p in new], [1]) self.assertEqual((changed, unchanged), ([], [])) def test_a_pr_that_moved_since_it_was_reported_is_changed(self): item = pr(1, updated="2026-09-24T09:00:00Z") - new, changed, unchanged = digest.partition_by_state( + new, changed, unchanged = digest.STATE.partition( [item], self.state((digest.pr_id(item), "2026-09-20T09:00:00Z"))) self.assertEqual([p["number"] for p in changed], [1]) self.assertEqual((new, unchanged), ([], [])) def test_a_pr_that_has_not_moved_is_dropped(self): item = pr(1, updated="2026-09-24T09:00:00Z") - new, changed, unchanged = digest.partition_by_state( + new, changed, unchanged = digest.STATE.partition( [item], self.state((digest.pr_id(item), "2026-09-24T09:00:00Z"))) self.assertEqual([p["number"] for p in unchanged], [1]) self.assertEqual((new, changed), ([], [])) @@ -120,21 +120,21 @@ def setUp(self): def save(self, *prs, **kwargs): with contextlib.redirect_stdout(io.StringIO()): - return digest.save_state(self.path, digest.empty_state(), list(prs), **kwargs) + return digest.STATE.save(self.path, digest.STATE.empty(), list(prs), **kwargs) def load(self): with contextlib.redirect_stdout(io.StringIO()): - return digest.load_state(self.path) + return digest.STATE.load(self.path) def test_a_reported_pr_comes_back_unchanged_next_run(self): item = pr(1, updated="2026-09-24T09:00:00Z") self.save(item) - _, _, unchanged = digest.partition_by_state([item], self.load()) + _, _, unchanged = digest.STATE.partition([item], self.load()) self.assertEqual(len(unchanged), 1) def test_the_same_pr_moved_comes_back_changed(self): self.save(pr(1, updated="2026-09-24T09:00:00Z")) - _, changed, _ = digest.partition_by_state( + _, changed, _ = digest.STATE.partition( [pr(1, updated="2026-09-24T11:00:00Z")], self.load()) self.assertEqual(len(changed), 1) @@ -144,7 +144,7 @@ def test_the_file_names_the_pr_for_whoever_opens_it(self): self.assertIn("session-desktop#1958", handle.read()) def test_no_path_means_no_state_and_no_complaint(self): - self.assertEqual(digest.load_state(None), digest.empty_state()) + self.assertEqual(digest.STATE.load(None), digest.STATE.empty()) class TestAge(unittest.TestCase): diff --git a/zendesk_triage/note_reply.py b/zendesk_triage/note_reply.py index 6d9f7a9..ffaa6d0 100644 --- a/zendesk_triage/note_reply.py +++ b/zendesk_triage/note_reply.py @@ -68,7 +68,11 @@ import textwrap sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) -import triage # noqa: E402 (needs the path insert above) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import triage # noqa: E402 (needs the path inserts above) +from shared.env import get_env # noqa: E402 +from shared.retry import request_with_retry # noqa: E402 +from shared.text import clip, squash # noqa: E402 DEFAULT_MODEL = "claude-sonnet-5" COMPOSE_TIMEOUT_SECONDS = 240 @@ -205,7 +209,7 @@ def api_user_id(session, subdomain): this same user so a draft never fires the webhook at all — see the README. """ url = f"https://{subdomain}.zendesk.com/api/v2/users/me.json" - resp = triage.request_with_retry(session, "GET", url) + resp = request_with_retry(session, "GET", url) if resp.status_code >= 400: sys.exit(f"Zendesk refused to identify the API user ({resp.status_code}).") return ((resp.json() or {}).get("user") or {}).get("id") @@ -242,7 +246,7 @@ def change_tags(session, subdomain, ticket_id, add=(), drop=()): ("DELETE", [t for t in drop if t])): if not names: continue - resp = triage.request_with_retry(session, method, url, json={"tags": names}) + resp = request_with_retry(session, method, url, json={"tags": names}) if resp.status_code >= 400: # Never worth failing a run over: tags are a dashboard light, not the work. print(f"Note: could not {method.lower()} tags on #{ticket_id} " @@ -354,7 +358,7 @@ def write_to_ticket(session, subdomain, ticket_id, body, public, if status: fields["status"] = status url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket_id}.json" - resp = triage.request_with_retry(session, "PUT", url, json={"ticket": fields}) + resp = request_with_retry(session, "PUT", url, json={"ticket": fields}) if resp.status_code >= 400: sys.exit(f"Zendesk rejected the {'reply' if public else 'note'} on " f"#{ticket_id} ({resp.status_code}).") @@ -448,7 +452,7 @@ def tagged_placement(ticket): def place_ticket(model, book, ticket, sample): """Which group and platform this ticket belongs to. (None, None) if unplaceable.""" catalogue = "\n".join(f"- {g['key']}: {g['title']}" for g in book["groups"]) - body = triage.clip(sample, CUSTOMER_SAMPLE_CHARS) + body = clip(sample, CUSTOMER_SAMPLE_CHARS) try: found = triage.claude_cli_json( model, "medium", PLACEMENT_SYSTEM, PLACEMENT_SCHEMA, @@ -801,7 +805,7 @@ def first_line(text, limit=90): whole reply, which is already on the ticket one comment above. """ line = next((part.strip() for part in (text or "").splitlines() if part.strip()), "") - return triage.clip(line, limit) + return clip(line, limit) def build_sent_note(user, comment_id, sent): @@ -852,7 +856,7 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr f"({cell['n']} solved, {cell['consistency']} consistency).") try: - result = compose(model, triage.clip(sample, CUSTOMER_SAMPLE_CHARS), brief, + result = compose(model, clip(sample, CUSTOMER_SAMPLE_CHARS), brief, previous, precedent) except SystemExit as exc: say(session, subdomain, ticket_id, comment_id, @@ -1019,9 +1023,8 @@ def already_english(turns, translated): english[int(item.get("index"))] = (item.get("english") or "").strip() except (TypeError, ValueError): continue - squash = lambda text: " ".join((text or "").split()).lower() return all(english.get(turn["index"]) - and squash(english[turn["index"]]) == squash(turn["body"]) + and squash(english[turn["index"]]).lower() == squash(turn["body"]).lower() for turn in turns) @@ -1055,7 +1058,7 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): try: rendered = triage.claude_cli_json( model, "medium", triage.TRANSCRIPT_SYSTEM_PROMPT, triage.TRANSCRIPT_SCHEMA, - triage.clip(payload, triage.TRANSCRIPT_INPUT_CHARS), + clip(payload, triage.TRANSCRIPT_INPUT_CHARS), triage.ENGLISH_TIMEOUT_SECONDS, f"the English transcript of #{ticket_id}") except SystemExit as exc: say(session, subdomain, ticket_id, command["id"], @@ -1138,9 +1141,9 @@ def main(): help="do everything except write to Zendesk") args = parser.parse_args() - subdomain = triage.get_env("ZENDESK_SUBDOMAIN") - session = triage.zendesk_session(triage.get_env("ZENDESK_EMAIL"), - triage.get_env("ZENDESK_API_TOKEN")) + subdomain = get_env("ZENDESK_SUBDOMAIN") + session = triage.zendesk_session(get_env("ZENDESK_EMAIL"), + get_env("ZENDESK_API_TOKEN")) api_user = api_user_id(session, subdomain) ticket = triage.fetch_ticket(session, subdomain, args.ticket) diff --git a/zendesk_triage/relay.py b/zendesk_triage/relay.py index 75196fd..2a29120 100644 --- a/zendesk_triage/relay.py +++ b/zendesk_triage/relay.py @@ -54,9 +54,6 @@ from fastapi import BackgroundTasks, FastAPI, Request, Response from starlette.concurrency import run_in_threadpool -sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) -import triage # noqa: E402 (needs the path insert above) - NOTE_SCRIPT = os.path.join(os.path.dirname(os.path.abspath(__file__)), "note_reply.py") # How stale a signed request may be. The signature covers the timestamp, so this diff --git a/zendesk_triage/resolve_reviews.py b/zendesk_triage/resolve_reviews.py index b3fffa7..ba63391 100644 --- a/zendesk_triage/resolve_reviews.py +++ b/zendesk_triage/resolve_reviews.py @@ -80,7 +80,11 @@ import requests sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) -import triage # noqa: E402 (needs the path insert above) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import triage # noqa: E402 (needs the path inserts above) +from shared.discord import post_to_discord # noqa: E402 +from shared.env import get_env # noqa: E402 +from shared.retry import request_with_retry # noqa: E402 # Reviews at or above this rating carry nothing to act on. Fixed rather than a flag: # 3★ and below are what the triage treats as bug reports in disguise, so lowering the @@ -188,7 +192,7 @@ def solve_batch(session, subdomain, ids, tag, note): payload = {"ticket": {"status": "solved", "additional_tags": [tag]}} if note: payload["ticket"]["comment"] = {"body": note, "public": False} - resp = triage.request_with_retry( + resp = request_with_retry( session, "PUT", url, params={"ids": ",".join(str(i) for i in ids)}, json=payload ) if resp.status_code >= 400: @@ -214,7 +218,7 @@ def wait_for_job(session, subdomain, job_id, timeout=JOB_TIMEOUT_SECONDS): url = f"https://{subdomain}.zendesk.com/api/v2/job_statuses/{job_id}.json" deadline = time.monotonic() + timeout while True: - resp = triage.request_with_retry(session, "GET", url) + resp = request_with_retry(session, "GET", url) if resp.status_code >= 400: sys.exit(f"could not read job {job_id} ({resp.status_code}): {resp.text[:200]}") job = (resp.json() or {}).get("job_status") or {} @@ -318,8 +322,7 @@ def post_summary(webhook_url, message): A fresh session, never the Zendesk one — that carries the API-token auth header, and Discord has no business receiving it. """ - return bool(triage.post_to_discord(requests.Session(), webhook_url, - [{"content": message}])) + return bool(post_to_discord(requests.Session(), webhook_url, [{"content": message}])) def main(): @@ -346,9 +349,9 @@ def main(): help="Discord webhook URL (else ZENDESK_DISCORD_WEBHOOK_URL).") args = parser.parse_args() - subdomain = triage.get_env("ZENDESK_SUBDOMAIN", args.subdomain) - email = triage.get_env("ZENDESK_EMAIL", args.email) - api_token = triage.get_env("ZENDESK_API_TOKEN", args.api_token) + subdomain = get_env("ZENDESK_SUBDOMAIN", args.subdomain) + email = get_env("ZENDESK_EMAIL", args.email) + api_token = get_env("ZENDESK_API_TOKEN", args.api_token) # The triage channel's own webhook, the one the digest posts to — a Discord # webhook is bound to the channel it was created in, so posting alongside the # digest means using its secret rather than the shared DISCORD_WEBHOOK_URL. @@ -357,8 +360,7 @@ def main(): # should stop the run rather than have it bulk-edit tickets it cannot report. A # dry run posts nothing, so it never needs one. needs_webhook = args.apply and not args.no_discord - webhook = triage.get_env("ZENDESK_DISCORD_WEBHOOK_URL", args.webhook, - required=needs_webhook) + webhook = get_env("ZENDESK_DISCORD_WEBHOOK_URL", args.webhook, required=needs_webhook) session = triage.zendesk_session(email, api_token) query = build_query() diff --git a/zendesk_triage/test_note_reply.py b/zendesk_triage/test_note_reply.py index 54abf11..26a72bb 100644 --- a/zendesk_triage/test_note_reply.py +++ b/zendesk_triage/test_note_reply.py @@ -22,9 +22,10 @@ import unittest sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import note_reply # noqa: E402 import triage # noqa: E402 -from test_triage import FakeResponse, FakeSession, Patched # noqa: E402 +from shared.testing import Env, FakeResponse, FakeSession, Patched # noqa: E402 API_USER = 901790886886 AGENT = 555 @@ -289,8 +290,7 @@ def test_the_transcript_note_is_never_public(self): class HouseAnswers(unittest.TestCase): def test_absent_config_turns_the_feature_off(self): """Drafting must work exactly as before on a host with no knowledge file.""" - with Patched(os, environ={k: v for k, v in os.environ.items() - if k != note_reply.HOUSE_ENV}): + with Env(**{note_reply.HOUSE_ENV: None}): self.assertIsNone(note_reply.load_house()) def test_a_corrupt_file_degrades_rather_than_fails(self): @@ -633,12 +633,12 @@ def test_end_users_may_not(self): self.assertFalse(note_reply.may_command({"id": AGENT, "role": "end-user"})) def test_allowlist_narrows_further(self): - with Patched(os, environ={**os.environ, "ZENDESK_NOTE_AUTHORS": "1,2"}): + with Env(ZENDESK_NOTE_AUTHORS="1,2"): self.assertFalse(note_reply.may_command({"id": AGENT, "role": "agent"})) self.assertTrue(note_reply.may_command({"id": 2, "role": "agent"})) def test_allowlist_does_not_override_the_role_check(self): - with Patched(os, environ={**os.environ, "ZENDESK_NOTE_AUTHORS": str(AGENT)}): + with Env(ZENDESK_NOTE_AUTHORS=str(AGENT)): self.assertFalse(note_reply.may_command({"id": AGENT, "role": "end-user"})) diff --git a/zendesk_triage/test_relay.py b/zendesk_triage/test_relay.py index 936593e..44d98a6 100644 --- a/zendesk_triage/test_relay.py +++ b/zendesk_triage/test_relay.py @@ -24,8 +24,10 @@ from fastapi.testclient import TestClient sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import relay # noqa: E402 -from test_triage import Patched # noqa: E402 +from shared import testing # noqa: E402 +from shared.testing import Patched # noqa: E402 # The relay only ever compares this against what it computes, so its value is # arbitrary — but it must not be empty, since an unset secret refuses everything. @@ -35,26 +37,11 @@ "ZENDESK_API_TOKEN": "tok", "RELAY_DRY_RUN": None} -class Env: - """Replace the process environment for the duration of a block.""" +class Env(testing.Env): + """The relay's working environment, with `overrides` on top.""" def __init__(self, **overrides): - self.overrides = {**BASE_ENV, **overrides} - self.saved = None - - def __enter__(self): - self.saved = dict(os.environ) - for key, value in self.overrides.items(): - if value is None: - os.environ.pop(key, None) - else: - os.environ[key] = value - return self - - def __exit__(self, *exc): - os.environ.clear() - os.environ.update(self.saved) - return False + super().__init__(**{**BASE_ENV, **overrides}) def zendesk_post(body=None, *, secret=ZENDESK_SECRET, sign=True, age=0, tamper=False): diff --git a/zendesk_triage/test_resolve_reviews.py b/zendesk_triage/test_resolve_reviews.py index 5f9e749..7c82b65 100644 --- a/zendesk_triage/test_resolve_reviews.py +++ b/zendesk_triage/test_resolve_reviews.py @@ -19,10 +19,11 @@ from urllib.parse import quote sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import resolve_reviews # noqa: E402 import triage # noqa: E402 -from test_triage import ( # noqa: E402 - ROOT, FakeResponse, FakeSession, NoSleep, unit_commands) +from shared.testing import FakeResponse, FakeSession, NoSleep, Patched # noqa: E402 +from test_triage import ROOT, unit_commands # noqa: E402 def review(ticket_id, stars=5, channel="any_channel", subject=None): @@ -349,13 +350,9 @@ def test_the_message_fits_one_discord_post(self): def test_the_summary_does_not_reuse_the_zendesk_session(self): """That session carries the API-token auth header; Discord must not see it.""" posted = [] - original = triage.post_to_discord - triage.post_to_discord = lambda session, url, messages: ( - posted.append((session, url, messages)) or len(messages)) - try: + with Patched(resolve_reviews, post_to_discord=lambda session, url, messages: ( + posted.append((session, url, messages)) or len(messages))): self.assertTrue(resolve_reviews.post_summary("https://hook", "hi")) - finally: - triage.post_to_discord = original session, url, messages = posted[0] self.assertIsNone(session.auth) self.assertEqual((url, messages), ("https://hook", [{"content": "hi"}])) diff --git a/zendesk_triage/test_triage.py b/zendesk_triage/test_triage.py index f9e3151..c2ac3db 100644 --- a/zendesk_triage/test_triage.py +++ b/zendesk_triage/test_triage.py @@ -193,20 +193,14 @@ def test_a_longer_window_reaches_further_back(self): long = triage.build_window_query(168).split("updated>")[1].split(" ")[0] self.assertLess(long, short) # ISO-8601 sorts chronologically - def test_window_label_reads_naturally(self): - self.assertEqual(triage.window_label(24), "updated in the past 1 day") - self.assertEqual(triage.window_label(48), "updated in the past 2 days") - self.assertEqual(triage.window_label(168), "updated in the past 7 days") - self.assertEqual(triage.window_label(36), "updated in the past 36h") - # ---- Dedup state ----------------------------------------------------------- class TestPartitionByState(unittest.TestCase): def test_empty_state_makes_everything_new(self): - new, changed, unchanged = triage.partition_by_state( - [ticket(1), ticket(2)], triage.empty_state() + new, changed, unchanged = triage.STATE.partition( + [ticket(1), ticket(2)], triage.STATE.empty() ) self.assertEqual([t["id"] for t in new], [1, 2]) self.assertEqual(changed, []) @@ -214,7 +208,7 @@ def test_empty_state_makes_everything_new(self): def test_same_requester_activity_is_unchanged(self): state = {"seen": {"1": {"requester_updated_at": "2026-08-03T12:00:00Z"}}} - new, changed, unchanged = triage.partition_by_state( + new, changed, unchanged = triage.STATE.partition( [ticket(1, updated_at="2026-08-03T12:00:00Z")], state ) self.assertEqual((new, changed), ([], [])) @@ -222,7 +216,7 @@ def test_same_requester_activity_is_unchanged(self): def test_moved_requester_activity_is_changed(self): state = {"seen": {"1": {"requester_updated_at": "2026-08-03T12:00:00Z"}}} - new, changed, unchanged = triage.partition_by_state( + new, changed, unchanged = triage.STATE.partition( [ticket(1, updated_at="2026-08-04T09:00:00Z")], state ) self.assertEqual((new, unchanged), ([], [])) @@ -234,14 +228,14 @@ def test_our_own_reply_does_not_re_report(self): state = {"seen": {"1": {"requester_updated_at": "2026-08-03T12:00:00Z"}}} touched = ticket(1, updated_at="2026-08-04T09:01:00Z") touched["requester_updated_at"] = "2026-08-03T12:00:00Z" - _, changed, unchanged = triage.partition_by_state([touched], state) + _, changed, unchanged = triage.STATE.partition([touched], state) self.assertEqual(changed, []) self.assertEqual([t["id"] for t in unchanged], [1]) def test_a_missing_metric_set_falls_back_to_updated_at(self): """A failed sideload degrades to the old noisy behaviour, never to silence.""" state = {"seen": {"1": {"requester_updated_at": "2026-08-03T12:00:00Z"}}} - _, changed, _ = triage.partition_by_state( + _, changed, _ = triage.STATE.partition( [ticket(1, updated_at="2026-08-04T09:00:00Z")], state ) self.assertEqual([t["id"] for t in changed], [1]) @@ -258,7 +252,7 @@ def test_mixed_batch_splits_three_ways(self): ticket(2, updated_at="2026-08-04T09:00:00Z"), # changed ticket(3), # new ] - new, changed, unchanged = triage.partition_by_state(batch, state) + new, changed, unchanged = triage.STATE.partition(batch, state) self.assertEqual([t["id"] for t in new], [3]) self.assertEqual([t["id"] for t in changed], [2]) self.assertEqual([t["id"] for t in unchanged], [1]) @@ -266,7 +260,7 @@ def test_mixed_batch_splits_three_ways(self): def test_ids_are_matched_as_strings_not_ints(self): """State comes back from JSON, where keys are always strings.""" state = {"seen": {"27564": {"requester_updated_at": "2026-08-03T12:00:00Z"}}} - _, _, unchanged = triage.partition_by_state( + _, _, unchanged = triage.STATE.partition( [ticket(27564, updated_at="2026-08-03T12:00:00Z")], state ) self.assertEqual(len(unchanged), 1) @@ -331,27 +325,27 @@ def setUp(self): self.addCleanup(self.dir.cleanup) def test_save_then_load_recovers_reported_tickets(self): - triage.save_state(self.path, triage.empty_state(), [ticket(1), ticket(2)], 30) - state = triage.load_state(self.path) + triage.STATE.save(self.path, triage.STATE.empty(), [ticket(1), ticket(2)], 30) + state = triage.STATE.load(self.path) self.assertEqual(sorted(state["seen"]), ["1", "2"]) self.assertEqual(state["seen"]["1"]["requester_updated_at"], "2026-08-03T12:00:00Z") self.assertEqual(state["version"], triage.STATE_VERSION) def test_save_creates_missing_parent_directories(self): - triage.save_state(self.path, triage.empty_state(), [ticket(1)], 30) + triage.STATE.save(self.path, triage.STATE.empty(), [ticket(1)], 30) self.assertTrue(os.path.exists(self.path)) def test_save_leaves_no_temp_file_behind(self): - triage.save_state(self.path, triage.empty_state(), [ticket(1)], 30) + triage.STATE.save(self.path, triage.STATE.empty(), [ticket(1)], 30) siblings = os.listdir(os.path.dirname(self.path)) self.assertEqual(siblings, ["seen.json"]) def test_resaving_updates_an_existing_entry(self): - triage.save_state(self.path, triage.empty_state(), [ticket(1, updated_at="A")], 30) - state = triage.load_state(self.path) - triage.save_state(self.path, state, [ticket(1, updated_at="B")], 30) + triage.STATE.save(self.path, triage.STATE.empty(), [ticket(1, updated_at="A")], 30) + state = triage.STATE.load(self.path) + triage.STATE.save(self.path, state, [ticket(1, updated_at="B")], 30) self.assertEqual( - triage.load_state(self.path)["seen"]["1"]["requester_updated_at"], "B") + triage.STATE.load(self.path)["seen"]["1"]["requester_updated_at"], "B") def test_entries_past_retention_are_pruned(self): old = (datetime.now(timezone.utc) - timedelta(days=40)).strftime(STAMP) @@ -363,21 +357,21 @@ def test_entries_past_retention_are_pruned(self): "2": {"requester_updated_at": "B", "last_reported": recent}, }, } - kept, pruned = triage.save_state(self.path, state, [], 30) + kept, pruned = triage.STATE.save(self.path, state, [], 30) self.assertEqual((kept, pruned), (1, 1)) - self.assertEqual(list(triage.load_state(self.path)["seen"]), ["2"]) + self.assertEqual(list(triage.STATE.load(self.path)["seen"]), ["2"]) def test_entries_with_unparseable_timestamps_are_dropped(self): state = {"version": triage.STATE_VERSION, "seen": {"1": {"requester_updated_at": "A", "last_reported": "nonsense"}}} - kept, pruned = triage.save_state(self.path, state, [], 30) + kept, pruned = triage.STATE.save(self.path, state, [], 30) self.assertEqual((kept, pruned), (0, 1)) def test_a_ticket_reported_now_survives_pruning(self): old = (datetime.now(timezone.utc) - timedelta(days=40)).strftime(STAMP) state = {"version": triage.STATE_VERSION, "seen": {"1": {"requester_updated_at": "A", "last_reported": old}}} - kept, _ = triage.save_state(self.path, state, [ticket(1, updated_at="B")], 30) + kept, _ = triage.STATE.save(self.path, state, [ticket(1, updated_at="B")], 30) self.assertEqual(kept, 1) @@ -397,45 +391,45 @@ def _write(self, name, content): def test_missing_file(self): path = os.path.join(self.dir.name, "absent.json") - self.assertEqual(triage.load_state(path), triage.empty_state()) + self.assertEqual(triage.STATE.load(path), triage.STATE.empty()) def test_unparseable_json(self): - self.assertEqual(triage.load_state(self._write("c.json", "{{{")), triage.empty_state()) + self.assertEqual(triage.STATE.load(self._write("c.json", "{{{")), triage.STATE.empty()) def test_json_that_is_not_an_object(self): - self.assertEqual(triage.load_state(self._write("l.json", "[]")), triage.empty_state()) + self.assertEqual(triage.STATE.load(self._write("l.json", "[]")), triage.STATE.empty()) def test_object_without_a_seen_map(self): self.assertEqual( - triage.load_state(self._write("n.json", '{"version": 1}')), triage.empty_state() + triage.STATE.load(self._write("n.json", '{"version": 1}')), triage.STATE.empty() ) def test_seen_of_the_wrong_type(self): self.assertEqual( - triage.load_state(self._write("w.json", '{"seen": []}')), triage.empty_state() + triage.STATE.load(self._write("w.json", '{"seen": []}')), triage.STATE.empty() ) def test_absent_version_is_a_cache_miss(self): """Without a version we can't know the fields mean what we think.""" path = self._write("v.json", '{"seen": {"1": {"updated_at": "A"}}}') - self.assertEqual(triage.load_state(path), triage.empty_state()) + self.assertEqual(triage.STATE.load(path), triage.STATE.empty()) def test_unknown_version_is_a_cache_miss(self): path = self._write("v2.json", '{"version": 99, "seen": {"1": {"updated_at": "A"}}}') - self.assertEqual(triage.load_state(path), triage.empty_state()) + self.assertEqual(triage.STATE.load(path), triage.STATE.empty()) def test_matching_version_loads_normally(self): path = self._write( "ok.json", json.dumps({"version": triage.STATE_VERSION, "seen": {"1": {"updated_at": "A"}}}), ) - self.assertEqual(list(triage.load_state(path)["seen"]), ["1"]) + self.assertEqual(list(triage.STATE.load(path)["seen"]), ["1"]) def test_state_written_by_save_state_round_trips_the_version(self): """Guards against save_state and load_state disagreeing on the version.""" path = os.path.join(self.dir.name, "rt.json") - triage.save_state(path, triage.empty_state(), [ticket(1)], 30) - self.assertEqual(list(triage.load_state(path)["seen"]), ["1"]) + triage.STATE.save(path, triage.STATE.empty(), [ticket(1)], 30) + self.assertEqual(list(triage.STATE.load(path)["seen"]), ["1"]) # ---- Discord rendering ----------------------------------------------------- diff --git a/zendesk_triage/triage.py b/zendesk_triage/triage.py index 9956dce..4e44201 100644 --- a/zendesk_triage/triage.py +++ b/zendesk_triage/triage.py @@ -83,9 +83,10 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from shared import discord, state as dedup # noqa: E402 -from shared.discord import clip, post_to_discord # noqa: E402 +from shared.discord import post_to_discord # noqa: E402 from shared.env import get_env # noqa: E402 from shared.retry import request_with_retry # noqa: E402 +from shared.text import clip, squash, window_label # noqa: E402 # The channel AppFollow imports app-store reviews on. Identified reviews with no # false positives in a 3,662-ticket sample; tags did not (only 287 carried one). @@ -169,11 +170,6 @@ def drop_quiet_tickets(tickets, cutoff): return fresh, quiet -def window_label(hours): - if hours % 24 == 0 and hours >= 24: - days = hours // 24 - return f"updated in the past {days} day{'s' if days > 1 else ''}" - return f"updated in the past {hours}h" # A pinned id rather than the `opus` alias, deliberately. This is an unattended # digest a human skims: the batch-wide fields (`cluster`, `priority_rank`) and the # severity calibration shift when the model underneath changes, and an alias would @@ -540,14 +536,6 @@ def fetch_total_unsolved(session, subdomain, query=BACKLOG_QUERY): # file is a real one, because it is what makes losing it merely noisy. -def empty_state(): - return dedup.empty_state(STATE_VERSION) - - -def load_state(path): - return dedup.load_state(path, STATE_VERSION, "ticket") - - def activity_key(ticket): """The timestamp a re-report is judged against. @@ -598,30 +586,8 @@ def hydrate_requester_activity(session, subdomain, tickets): return hydrated -def partition_by_state(tickets, state): - """Split into (new, changed, unchanged) against saved state. - - `changed` means the requester has touched the ticket since we last reported it — - see activity_key. An agent reply or an automation firing is not a change here. - """ - seen = state.get("seen", {}) - new, changed, unchanged = [], [], [] - for ticket in tickets: - previous = seen.get(str(ticket.get("id"))) - if previous is None: - new.append(ticket) - elif previous.get("requester_updated_at") != activity_key(ticket): - changed.append(ticket) - else: - unchanged.append(ticket) - return new, changed, unchanged - - -def save_state(path, state, reported, retention_days): - """Record `reported` as seen. Returns (kept, pruned).""" - records = {str(t.get("id")): {"requester_updated_at": activity_key(t)} - for t in reported} - return dedup.save_state(path, state, records, retention_days, STATE_VERSION) +STATE = dedup.Tracker(STATE_VERSION, "ticket", lambda t: str(t.get("id")), activity_key, + "requester_updated_at") # ---- App-store review filtering -------------------------------------------- @@ -762,11 +728,6 @@ def customer_text(session, subdomain, ticket, comments, limit): return clip("\n\n".join(parts), limit) or "(no text)" -def squash(value): - """Collapse whitespace so subject/description can be compared meaningfully.""" - return re.sub(r"\s+", " ", value or "").strip() - - def review_stars(ticket): """Star count from an AppFollow review subject, or None if not a review subject.""" match = STAR_SUBJECT.match(ticket.get("subject") or "") @@ -1707,8 +1668,9 @@ def main(): "unchanged (same Zendesk updated_at) are skipped entirely; " "changed ones are re-reported and flagged. Written only on a " "real run, after Discord accepts the post.") - parser.add_argument("--state-retention-days", type=int, default=30, metavar="N", - help="Forget state entries older than N days (default: 30).") + parser.add_argument("--state-retention-days", type=int, default=STATE.retention_days, + metavar="N", + help=f"Forget state entries older than N days (default: {STATE.retention_days}).") parser.add_argument("--dump-batch", metavar="PATH", help="Write the batch (tickets, prompt, schema) to PATH and exit, for " "hand-classification. WARNING: writes ticket content to disk.") @@ -1722,7 +1684,7 @@ def main(): # cannot deliver. A dump exits before rendering, so it never needs them either. needs_discord = not (args.dry_run or args.no_discord or args.dump_batch) webhook = get_env("ZENDESK_DISCORD_WEBHOOK_URL", args.webhook, required=needs_discord) - model = args.model or os.environ.get("ZENDESK_TRIAGE_MODEL") or DEFAULT_MODEL + model = get_env("ZENDESK_TRIAGE_MODEL", args.model, default=DEFAULT_MODEL) stats = {} state = None @@ -1740,7 +1702,7 @@ def main(): # An explicit query wins over --window-hours; warn rather than silently drop it. window_start = None - explicit_query = args.query or os.environ.get("ZENDESK_QUERY") + explicit_query = get_env("ZENDESK_QUERY", args.query, required=False) if explicit_query: if args.window_hours: print("Note: --window-hours ignored because an explicit query was given.") @@ -1748,7 +1710,7 @@ def main(): elif args.window_hours: window_start = window_cutoff(args.window_hours) query = build_window_query(args.window_hours, window_start) - stats["scope"] = window_label(args.window_hours) + stats["scope"] = f"updated in the past {window_label(args.window_hours)}" else: query = DEFAULT_QUERY @@ -1802,8 +1764,8 @@ def main(): return if args.state: - state = load_state(args.state) - new, changed, unchanged = partition_by_state(tickets, state) + state = STATE.load(args.state) + new, changed, unchanged = STATE.partition(tickets, state) print(f"{len(new)} new, {len(changed)} changed since last reported, " f"{len(unchanged)} unchanged (skipped).") stats["skipped_unchanged"] = len(unchanged) @@ -1898,9 +1860,9 @@ def main(): # Record only tickets covered by messages Discord actually accepted, so a partial # failure neither reposts what landed nor suppresses what didn't. if args.state and state is not None: - delivered = set().union(*coverage[:posted]) if posted else set() + delivered = discord.delivered_ids(coverage, posted) recorded = [t for t in classified if t.get("id") in delivered] - kept, pruned = save_state(args.state, state, recorded, args.state_retention_days) + kept, pruned = STATE.save(args.state, state, recorded, args.state_retention_days) print(f"Recorded {len(recorded)} tickets; state now tracks {kept} " f"({pruned} pruned beyond {args.state_retention_days} days).") From d0df66ded6b8a609cf0d183dc5bd7ad80314a0ad Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 24 Sep 2026 16:51:16 +1000 Subject: [PATCH 3/5] refactor: split the Zendesk API, the Claude CLI and the transcript out of triage.py note_reply, resolve_reviews and the alert imported the 1900-line classifier to reach a ticket fetch, a marker format or the CLI's name. Those now live in modules of their own: zendesk.py the API session, search and its 1000-result ceiling, one ticket, its comments and users, the one PUT every write is, the markers, who the customer is on a channel integration, and what an imported store review looks like claude_cli.py run_json, and try_run_json for an enrichment that hands the failure back instead of exiting transcript.py the English transcript: translate, render, write, attach The three comment fetchers in triage.py differed only in page size, order and whether a failure exits or skips the ticket; those are now parameters of one fetch_comments. note_reply's model flag goes through resolve_api_model like the triage's does, so an alias works for both. undash_english is pure text and moves to shared.text. triage.py keeps the queries, the taxonomy, the dedup, the review filter, the analysis and the rendering: 1091 lines, from 1913. --- README.md | 2 +- deploy/alert.py | 2 +- shared/text.py | 30 + zendesk_triage/claude_cli.py | 191 ++++++ zendesk_triage/note_reply.py | 148 ++--- zendesk_triage/resolve_reviews.py | 15 +- zendesk_triage/test_note_reply.py | 39 +- zendesk_triage/test_resolve_reviews.py | 7 +- zendesk_triage/test_triage.py | 214 +++---- zendesk_triage/transcript.py | 198 ++++++ zendesk_triage/triage.py | 841 +------------------------ zendesk_triage/zendesk.py | 493 +++++++++++++++ 12 files changed, 1134 insertions(+), 1046 deletions(-) create mode 100644 zendesk_triage/claude_cli.py create mode 100644 zendesk_triage/transcript.py create mode 100644 zendesk_triage/zendesk.py diff --git a/README.md b/README.md index 4a3e689..d2eca62 100644 --- a/README.md +++ b/README.md @@ -280,7 +280,7 @@ The triage's opening act: it solves the 4-5★ AppFollow reviews that were never Deliberately narrow, because a mis-aimed bulk status change is not recoverable by re-running: -- **App-store reviews only**, by the same detection the triage uses — `triage.is_store_review`, so the two can't drift apart. Every fetched ticket is re-checked locally, since the query can't express the rating. +- **App-store reviews only**, by the same detection the triage uses — `zendesk.is_store_review`, so the two can't drift apart. Every fetched ticket is re-checked locally, since the query can't express the rating. - **Rated 4★ or better.** A fixed floor (`MIN_STARS`), not a flag — 3★ and below are what the triage reads as bug reports in disguise, so a lower floor would have this job close the reviews most worth looking at. A review whose stars can't be parsed from the subject is skipped, never solved. - **`new` or `open`** (`status 1 else ''}" return f"{hours}h" + + +# A long dash is the clearest tell that text was machine-written, and no reply this +# team has sent uses one. The prompts that write for customers forbid it; this is the +# failsafe, because a prompt rule is advisory and the text reaches a real person. +# The spaced form is punctuation and becomes a comma; anything left is joining two +# things, like a range, and becomes the hyphen a person would have typed. +PUNCTUATING_DASH = re.compile(r"(?:\s+[—–]\s*|\s*[—–]\s+)") + + +ANY_LONG_DASH = re.compile(r"[—–]") + + +def undash_english(text): + """Replace every em and en dash: punctuation with a comma, the rest with a hyphen. + + ENGLISH ONLY, and the name says so because passing anything else corrupts it. In + Russian and the other East Slavic languages the long dash carries the present-tense + copula that the grammar omits: "Москва — столица России" IS the verb, and the comma + this produces leaves a subject with no predicate. Spanish, French, Polish and + Chinese give it dialogue and parenthetical duty that a comma does not carry either. + + Only for text a model wrote. Rewriting punctuation somebody typed themselves would + be wrong even in English. + + A failsafe, not a style pass: the substitution is blunt enough to turn a legitimate + strong break into a comma splice ("I checked the logs — nothing was uploaded"), so + the prompt is what should keep dashes out and this is what catches the misses. + """ + return ANY_LONG_DASH.sub("-", PUNCTUATING_DASH.sub(", ", text or "")) diff --git a/zendesk_triage/claude_cli.py b/zendesk_triage/claude_cli.py new file mode 100644 index 0000000..bb5ab81 --- /dev/null +++ b/zendesk_triage/claude_cli.py @@ -0,0 +1,191 @@ +"""One schema-enforced request to the Claude Code CLI. + +Authentication is whatever `claude` is already logged in as, so no caller holds a +key. run_json exits on any failure, which is right for a classification the run +cannot continue without; try_run_json is for an enrichment, and hands the failure +back instead. +""" +import json +import os +import subprocess +import sys + +# Shorthands for the model override, so ZENDESK_TRIAGE_MODEL=sonnet works for a big +# backfill without anyone looking up an id. The API takes ids only, so they are +# mapped here; each is the newest model in its family, and a full id passes through +# untouched. +API_MODEL_ALIASES = { + "opus": "claude-opus-5", + "sonnet": "claude-sonnet-5", + "haiku": "claude-haiku-4-5", +} +# The classifier is the local Claude Code CLI rather than the Anthropic SDK, so +# authentication is whatever `claude` is already logged in as and no key lives here. +CLAUDE_CLI = "claude" + +# Dropped from the CLI's environment. Each one silently outranks whatever `claude` is +# logged in as, and each is API-backend configuration — a box that once ran that way +# still has the key in its EnvironmentFile, where it is now dead config that would +# otherwise pick the credential, and the billing, for every classification. +# +# CLAUDE_CODE_OAUTH_TOKEN is deliberately not in this list. It is a subscription +# credential like the interactive login, not an API key, and it is the only one of +# these an unattended host can renew on a yearly rather than weekly cadence — see +# deploy/README.md. Nothing else can set it: it has never been written by anything +# this repo deploys, so it reaches the CLI only because somebody put it there. +CLAUDE_AUTH_OVERRIDES = ( + "ANTHROPIC_API_KEY", + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_BASE_URL", +) +# How much of a failed call's output reaches the log. The CLI can echo input back and +# this log must not carry ticket text, so nothing here is ever passed through whole. +CLI_FAILURE_CHARS = 300 + + +def resolve_api_model(model): + """Map a shorthand model name onto the id `claude --model` expects. + + Anything that isn't a known shorthand passes through untouched, so a pinned id + (`claude-opus-4-8`) or a model newer than this table still works. + """ + return API_MODEL_ALIASES.get(model, model) + + +def cli_failure_detail(stdout, stderr, limit=CLI_FAILURE_CHARS): + """The readable half of a failed `claude --print` run, clipped for the log. + + stderr wins, but the failures that matter most — a refused login, an exhausted + limit — leave it empty and put their message in the `--output-format json` + envelope on stdout, where it sits behind enough usage boilerplate to survive no + clip at all. Hence parsing the envelope rather than clipping it. `terminal_reason` + rides along when it fits: it is what separates an auth failure from a limit. + + Output that is not that envelope is reported raw: a CLI that dies before emitting + one has still said the only thing anybody will get. + """ + detail = (stderr or "").strip() + if detail: + return detail[:limit] + raw = (stdout or "").strip() + try: + envelope = json.loads(raw) + except ValueError: + envelope = None + if isinstance(envelope, dict): + message = "" + for name in ("result", "error"): + value = envelope.get(name) + if isinstance(value, dict): + value = value.get("message") + if isinstance(value, str) and value.strip(): + message = value.strip() + break + reason = envelope.get("terminal_reason") or envelope.get("subtype") + reason = reason.strip() if isinstance(reason, str) else "" + if message and reason and len(message) + len(reason) + 3 <= limit: + return f"{message} ({reason})" + if message: + return message[:limit] + if reason: + return f"it reported {reason!r} and no message." + return raw[:limit] + + +def run_json(model, effort, system_prompt, schema, prompt, timeout, label): + """Run one schema-enforced Claude Code request. Returns the parsed payload. + + `--json-schema` enforces the schema the way the API's structured outputs did. + Authentication is whatever `claude` is already logged in as, so neither caller + holds a Claude key. + + The prompt goes over **stdin**, not argv. Linux caps one argument at 128KB + (MAX_ARG_STRLEN) and a full --batch-size 400 chunk is around 685KB, so passing it + as an argument would work on a normal day and die with "Argument list too long" + on a backfill. It is also the more private channel: argv is world-readable + through /proc, and these prompts carry ticket text. + """ + command = [ + CLAUDE_CLI, "--print", + "--model", model, + "--effort", effort, + "--system-prompt", system_prompt, + "--json-schema", json.dumps(schema), + "--output-format", "json", + "--no-session-persistence", + # Nothing outside this call may change what the model is told. The two flags + # cover different halves of that and neither implies the other: + # --setting-sources "" drops the user and project settings — and the hooks + # inside them — while --tools "" removes the tools. Without the first, a + # .claude/settings.json next to this file, or one in the service account's + # home, silently joins every classification and every translation. + # + # --tools stays last: it is variadic, so it swallows any following argument + # that does not begin with a dash. + "--setting-sources", "", + "--tools", "", + ] + child_env = {name: value for name, value in os.environ.items() + if name not in CLAUDE_AUTH_OVERRIDES} + try: + done = subprocess.run(command, input=prompt, capture_output=True, text=True, + check=False, timeout=timeout, env=child_env) + except FileNotFoundError: + sys.exit(f"{CLAUDE_CLI} is not on PATH. {label} runs through the Claude Code " + f"CLI, so it has to be installed and logged in.") + except subprocess.TimeoutExpired: + sys.exit(f"{CLAUDE_CLI} did not finish {label} within {timeout}s.") + + if done.returncode != 0: + detail = cli_failure_detail(done.stdout, done.stderr) + if not detail: + detail = ("it printed nothing, which is what a login it can no longer " + "use looks like; check that `claude` is still signed in.") + sys.exit(f"{CLAUDE_CLI} exited {done.returncode} on {label}: {detail}") + try: + response = json.loads(done.stdout) + except ValueError as exc: + sys.exit(f"{CLAUDE_CLI} returned output that is not JSON on {label} ({exc}).") + if not isinstance(response, dict): + sys.exit(f"{CLAUDE_CLI} returned {type(response).__name__} on {label}, " + f"expected an object.") + # is_error and subtype are the CLI's signals for success; stop_reason deliberately + # is not — a successful structured-output run reports "tool_use", because that is + # how the schema is enforced underneath. + if response.get("is_error") or response.get("subtype") != "success": + detail = cli_failure_detail(done.stdout, done.stderr) + sys.exit(f"{CLAUDE_CLI} reported failure on {label} " + f"(subtype={response.get('subtype')!r}, " + f"api_error_status={response.get('api_error_status')!r})" + f"{': ' + detail if detail else '.'}") + # stop_reason is worth reading for this one value. There is no --max-tokens to + # raise, so an answer too long to finish comes back as JSON that stops mid-object, + # and the parse below would report a baffling syntax error for something whose + # only fix is a smaller batch. + if response.get("stop_reason") == "max_tokens": + sys.exit(f"{CLAUDE_CLI} ran out of output tokens on {label}, so the JSON is " + f"incomplete. Ask for less per call; for the digest that is a lower " + f"--batch-size.") + + # structured_output is the object --json-schema produced, so it beats re-parsing + # the `result` string: one less decode, and immune to prose alongside the JSON. + payload = response.get("structured_output") + if payload is not None: + return payload + raw = response.get("result") + if not raw: + sys.exit(f"{CLAUDE_CLI} returned neither structured_output nor a result " + f"on {label}.") + try: + return json.loads(raw) + except ValueError as exc: + sys.exit(f"{CLAUDE_CLI} result on {label} is not the JSON the schema asked " + f"for ({exc}).") + + +def try_run_json(*args, **kwargs): + """run_json for an enrichment: (payload, None), or (None, why) when the call failed.""" + try: + return run_json(*args, **kwargs), None + except SystemExit as exc: + return None, str(exc) diff --git a/zendesk_triage/note_reply.py b/zendesk_triage/note_reply.py index ffaa6d0..c1f73df 100644 --- a/zendesk_triage/note_reply.py +++ b/zendesk_triage/note_reply.py @@ -69,10 +69,12 @@ sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -import triage # noqa: E402 (needs the path inserts above) from shared.env import get_env # noqa: E402 -from shared.retry import request_with_retry # noqa: E402 -from shared.text import clip, squash # noqa: E402 +from shared.text import clip, squash, undash_english # noqa: E402 + +import claude_cli # noqa: E402 +import transcript # noqa: E402 +import zendesk # noqa: E402 DEFAULT_MODEL = "claude-sonnet-5" COMPOSE_TIMEOUT_SECONDS = 240 @@ -145,12 +147,12 @@ def done_marker(comment_id): emailed twice. Keyed on the commanding comment because that is what is unique per instruction; the ticket id is not. """ - return triage.marker("claude:done", comment_id) + return zendesk.marker("claude:done", comment_id) def draft_marker(comment_id): """Marks a note as carrying a sendable draft, and says which brief produced it.""" - return triage.marker("claude:draft", comment_id) + return zendesk.marker("claude:draft", comment_id) def english_marker(latest_public_id): @@ -160,7 +162,7 @@ def english_marker(latest_public_id): twice with nothing said in between should cost nothing, and asking again after the customer writes back should produce a fresh transcript. """ - return triage.marker("claude:english", latest_public_id) + return zendesk.marker("claude:english", latest_public_id) # Anchored to the start of a line so that prose mentioning the command in passing — @@ -201,20 +203,6 @@ def comment_text(comment): # ---- Zendesk ---------------------------------------------------------------- -def api_user_id(session, subdomain): - """The user the API token authenticates as. - - Its own notes are skipped when looking for a command, which is the in-code half - of the loop guard. The other half is the Zendesk trigger, which should exclude - this same user so a draft never fires the webhook at all — see the README. - """ - url = f"https://{subdomain}.zendesk.com/api/v2/users/me.json" - resp = request_with_retry(session, "GET", url) - if resp.status_code >= 400: - sys.exit(f"Zendesk refused to identify the API user ({resp.status_code}).") - return ((resp.json() or {}).get("user") or {}).get("id") - - def may_command(user): """Whether this Zendesk user may drive the command. @@ -230,29 +218,6 @@ def may_command(user): return not allowed or str(user.get("id")) in allowed -def change_tags(session, subdomain, ticket_id, add=(), drop=()): - """Add and remove tags, through the tags sub-resource. - - NOT `additional_tags`/`remove_tags` on the ticket update: those are update_many - fields. A single-ticket update accepts them with a 200 and silently ignores them, - which is how every tag this tool set went missing while every call reported - success. Measured against the live API, not assumed. - - The sub-resource is also additive rather than read-modify-write, so two runs on - one ticket cannot clobber each other's tags. - """ - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket_id}/tags.json" - for method, names in (("PUT", [t for t in add if t]), - ("DELETE", [t for t in drop if t])): - if not names: - continue - resp = request_with_retry(session, method, url, json={"tags": names}) - if resp.status_code >= 400: - # Never worth failing a run over: tags are a dashboard light, not the work. - print(f"Note: could not {method.lower()} tags on #{ticket_id} " - f"({resp.status_code}).") - - def clear_queued(session, subdomain, ticket_id, dry_run=False): """Take the ticket out of the "waiting on Claude" queue, writing no comment. @@ -265,7 +230,7 @@ def clear_queued(session, subdomain, ticket_id, dry_run=False): """ if dry_run: return - change_tags(session, subdomain, ticket_id, drop=[TAG_QUEUED]) + zendesk.change_tags(session, subdomain, ticket_id, drop=[TAG_QUEUED]) def para(text): @@ -281,12 +246,12 @@ def para(text): whitespace has to survive intact. quoted_para() carries what the customer said and what the agent gave as a solve reason, which are theirs and may not be English. """ - return f"

{html.escape(triage.undash_english(text))}

" + return f"

{html.escape(undash_english(text))}

" def bold_para(text): """A paragraph that leads a section, such as a speaker line in a transcript.""" - return f"

{html.escape(triage.undash_english(text))}

" + return f"

{html.escape(undash_english(text))}

" def quoted_para(text): @@ -308,7 +273,7 @@ def quoted_para(text): def transcript_blocks(turns, translated): """One turn at a time, as readable paragraphs. - Deliberately not triage.render_transcript's output re-split on blank lines: a + 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 @@ -357,13 +322,12 @@ def write_to_ticket(session, subdomain, ticket_id, body, public, fields = {"comment": {("html_body" if as_html else "body"): body, "public": public}} if status: fields["status"] = status - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket_id}.json" - resp = request_with_retry(session, "PUT", url, json={"ticket": fields}) + resp = zendesk.update_ticket(session, subdomain, ticket_id, fields) if resp.status_code >= 400: sys.exit(f"Zendesk rejected the {'reply' if public else 'note'} on " f"#{ticket_id} ({resp.status_code}).") # After the comment, so a tag failure cannot lose the thing that mattered. - change_tags(session, subdomain, ticket_id, add_tags, drop_tags) + zendesk.change_tags(session, subdomain, ticket_id, add_tags, drop_tags) # ---- What we usually reply --------------------------------------------------- @@ -453,16 +417,15 @@ def place_ticket(model, book, ticket, sample): """Which group and platform this ticket belongs to. (None, None) if unplaceable.""" catalogue = "\n".join(f"- {g['key']}: {g['title']}" for g in book["groups"]) body = clip(sample, CUSTOMER_SAMPLE_CHARS) - try: - found = triage.claude_cli_json( - model, "medium", PLACEMENT_SYSTEM, PLACEMENT_SCHEMA, - f"GROUP CATALOGUE:\n{catalogue}\n\nTHE TICKET:\n" - f"{(ticket.get('subject') or '')[:200]}\n\n{body}", - PLACEMENT_TIMEOUT_SECONDS, f"the placement of #{ticket['id']}") - except SystemExit as exc: - # Grounding is an enrichment. A failed classification costs a thinner draft, - # not the draft — the same call triage.py makes about its transcripts. - print(f"Note: could not place #{ticket['id']} ({exc}); drafting without " + found, failure = claude_cli.try_run_json( + model, "medium", PLACEMENT_SYSTEM, PLACEMENT_SCHEMA, + f"GROUP CATALOGUE:\n{catalogue}\n\nTHE TICKET:\n" + f"{(ticket.get('subject') or '')[:200]}\n\n{body}", + PLACEMENT_TIMEOUT_SECONDS, f"the placement of #{ticket['id']}") + if failure: + # Grounding is an enrichment: a failed placement costs a thinner draft, not + # the draft. + print(f"Note: could not place #{ticket['id']} ({failure}); drafting without " f"the house answer.") return None, None group = found.get("group") @@ -684,19 +647,19 @@ def validate_composition(result): sys.exit("Claude returned no usable reply option.") # `reply_en` and `back_translation` are English by construction, so the failsafe # always applies. `translated` is the customer's language, where a long dash may be - # grammar rather than decoration — see triage.undash_english — so it is cleaned only + # grammar rather than decoration — see undash_english — so it is cleaned only # when that language is English, and left alone otherwise. for option in options: for field in ("reply_en", "back_translation"): - option[field] = triage.undash_english(option.get(field)) + option[field] = undash_english(option.get(field)) if result["is_english"]: - option["translated"] = triage.undash_english(option.get("translated")) + option["translated"] = undash_english(option.get("translated")) result["options"] = options[:MAX_OPTIONS] return result def compose(model, sample, brief, previous=None, precedent=None): - return validate_composition(triage.claude_cli_json( + return validate_composition(claude_cli.run_json( model, "medium", COMPOSE_SYSTEM, COMPOSE_SCHEMA, build_compose_prompt(sample, brief, previous, precedent), COMPOSE_TIMEOUT_SECONDS, "the reply draft")) @@ -756,7 +719,7 @@ def find_draft(comments, api_user): so a human pasting the delimiters into a note of their own cannot smuggle text past the review. - triage.fetch_comments returns newest first, so this walks the list as it comes. + zendesk.fetch_comments returns newest first, so this walks the list as it comes. """ for comment in comments: if comment.get("public") or comment.get("author_id") != api_user: @@ -836,7 +799,7 @@ def run_draft(session, subdomain, model, ticket, comments, command, api_user, dr shown = find_draft(comments, api_user) previous = "\n\n".join(f"Option {n}:\n{shown[n]}" for n in sorted(shown)) or None - sample = triage.customer_text(session, subdomain, ticket, comments, + sample = zendesk.customer_text(session, subdomain, ticket, comments, CUSTOMER_SAMPLE_CHARS) book = load_house() group, platform = tagged_placement(ticket) @@ -893,7 +856,7 @@ def run_solve(session, subdomain, ticket, command, dry_run): say(session, subdomain, ticket_id, command["id"], "This ticket is already solved.", dry_run, error=False) return - author = triage.fetch_user(session, subdomain, command["author"]) + author = zendesk.fetch_user(session, subdomain, command["author"]) who = author.get("name") or f"user {command['author']}" if dry_run: print(f"#{ticket_id}: dry run, would solve on behalf of {who}.") @@ -932,7 +895,7 @@ def run_explain(session, subdomain, model, ticket, comments, command, dry_run): new_tags = [] if not group: group, platform = place_ticket( - model, book, ticket, triage.customer_text(session, subdomain, ticket, comments, + model, book, ticket, zendesk.customer_text(session, subdomain, ticket, comments, CUSTOMER_SAMPLE_CHARS)) new_tags = ([f"{TAG_GROUP_PREFIX}{group}"] if group else []) + \ ([f"{TAG_PLATFORM_PREFIX}{platform}"] if platform else []) @@ -985,7 +948,7 @@ 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 - author = triage.fetch_user(session, subdomain, command["author"]) + author = zendesk.fetch_user(session, subdomain, command["author"]) who = author.get("name") or f"user {command['author']}" if dry_run: print(f"#{ticket_id}: dry run, would send an option on behalf of {who}.") @@ -1034,38 +997,30 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): 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 triage.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; see + transcript.translate for why. """ ticket_id = ticket["id"] latest = next((c.get("id") for c in comments if c.get("public")), None) - if latest is not None and triage.has_marker(comments, english_marker(latest)): + if latest is not None and zendesk.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 - turns = triage.conversation_turns(session, subdomain, ticket) + turns = zendesk.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 - payload = json.dumps([{"index": t["index"], "speaker": t["who"], "text": t["body"]} - for t in turns], ensure_ascii=False) - try: - rendered = triage.claude_cli_json( - model, "medium", triage.TRANSCRIPT_SYSTEM_PROMPT, triage.TRANSCRIPT_SCHEMA, - clip(payload, triage.TRANSCRIPT_INPUT_CHARS), - triage.ENGLISH_TIMEOUT_SECONDS, f"the English transcript of #{ticket_id}") - except SystemExit as exc: + rendered, failure = transcript.translate(model, turns, ticket_id) + if failure: say(session, subdomain, ticket_id, command["id"], - f"The English transcript could not be produced: {exc} To try again, add " + f"The English transcript could not be produced: {failure} To try again, add " "a new private note reading claude: english.", dry_run) return - if already_english(turns, rendered.get("turns")): + if already_english(turns, rendered): # A transcript of English text repeats what is already a few comments above # it. Say so rather than posting the same words back. say(session, subdomain, ticket_id, command["id"], @@ -1079,7 +1034,7 @@ def run_english(session, subdomain, model, ticket, comments, command, dry_run): 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")) + + transcript_blocks(turns, rendered) + [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]) @@ -1111,7 +1066,7 @@ def say(session, subdomain, ticket_id, comment_id, text, dry_run, error=True): def latest_command(comments, api_user, session, subdomain): """The newest private note that is a command from someone allowed to give one. - triage.fetch_comments returns newest first, so the first match is the newest + zendesk.fetch_comments returns newest first, so the first match is the newest command: older ones have already been handled and carry their own done markers. An unauthorised author stops the search rather than falling through to an older @@ -1124,7 +1079,7 @@ def latest_command(comments, api_user, session, subdomain): parsed = parse_command(comment_text(comment)) if not parsed: continue - user = triage.fetch_user(session, subdomain, comment.get("author_id")) + user = zendesk.fetch_user(session, subdomain, comment.get("author_id")) if not may_command(user): print(f"Ignoring a command from {user.get('role') or 'an unknown user'}.") return None @@ -1140,40 +1095,41 @@ def main(): parser.add_argument("--dry-run", action="store_true", help="do everything except write to Zendesk") args = parser.parse_args() + model = claude_cli.resolve_api_model(args.model) subdomain = get_env("ZENDESK_SUBDOMAIN") - session = triage.zendesk_session(get_env("ZENDESK_EMAIL"), + session = zendesk.api_session(get_env("ZENDESK_EMAIL"), get_env("ZENDESK_API_TOKEN")) - api_user = api_user_id(session, subdomain) + api_user = zendesk.api_user_id(session, subdomain) - ticket = triage.fetch_ticket(session, subdomain, args.ticket) + ticket = zendesk.fetch_ticket(session, subdomain, args.ticket) if ticket.get("status") == "closed": # Closed is irreversible and takes no comments at all, so there is nowhere to # even report the refusal. Say it to the journal and stop. print(f"#{args.ticket}: closed, so Zendesk takes no comments. Nothing done.") return - comments = triage.fetch_comments(session, subdomain, args.ticket) + comments = zendesk.fetch_comments(session, subdomain, args.ticket) command = latest_command(comments, api_user, session, subdomain) if not command: print(f"#{args.ticket}: no command note to act on.") clear_queued(session, subdomain, args.ticket, args.dry_run) return - if triage.has_marker(comments, done_marker(command["id"])): + if zendesk.has_marker(comments, done_marker(command["id"])): print(f"#{args.ticket}: this command was already handled; nothing written.") clear_queued(session, subdomain, args.ticket, args.dry_run) return if command["action"] == "draft": - run_draft(session, subdomain, args.model, ticket, comments, command, + run_draft(session, subdomain, model, ticket, comments, command, api_user, args.dry_run) elif command["action"] == "solve": run_solve(session, subdomain, ticket, command, args.dry_run) elif command["action"] == "explain": - run_explain(session, subdomain, args.model, ticket, comments, command, + run_explain(session, subdomain, model, ticket, comments, command, args.dry_run) elif command["action"] == "english": - run_english(session, subdomain, args.model, ticket, comments, command, args.dry_run) + run_english(session, subdomain, model, ticket, comments, command, args.dry_run) else: run_reply(session, subdomain, ticket, comments, command, api_user, args.dry_run) diff --git a/zendesk_triage/resolve_reviews.py b/zendesk_triage/resolve_reviews.py index ba63391..b33b09e 100644 --- a/zendesk_triage/resolve_reviews.py +++ b/zendesk_triage/resolve_reviews.py @@ -81,11 +81,12 @@ sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -import triage # noqa: E402 (needs the path inserts above) from shared.discord import post_to_discord # noqa: E402 from shared.env import get_env # noqa: E402 from shared.retry import request_with_retry # noqa: E402 +import zendesk # noqa: E402 + # Reviews at or above this rating carry nothing to act on. Fixed rather than a flag: # 3★ and below are what the triage treats as bug reports in disguise, so lowering the # floor would have this job close the reviews most worth reading. Changing it is a @@ -136,10 +137,10 @@ def select_resolvable(tickets, min_stars): """ resolvable, skipped = [], [] for ticket in tickets: - if not triage.is_store_review(ticket): + if not zendesk.is_store_review(ticket): skipped.append((ticket, "not an app-store review")) continue - stars = triage.review_stars(ticket) + stars = zendesk.review_stars(ticket) if stars is None: skipped.append((ticket, "no star rating in the subject")) continue @@ -252,7 +253,7 @@ def tally_by_stars(tickets): counts = {} for ticket in tickets: # Never None here: select_resolvable drops anything without a parsed rating. - stars = triage.review_stars(ticket) + stars = zendesk.review_stars(ticket) counts[stars] = counts.get(stars, 0) + 1 return counts @@ -362,12 +363,12 @@ def main(): needs_webhook = args.apply and not args.no_discord webhook = get_env("ZENDESK_DISCORD_WEBHOOK_URL", args.webhook, required=needs_webhook) - session = triage.zendesk_session(email, api_token) + session = zendesk.api_session(email, api_token) query = build_query() # Every match, not the newest 1000: the tail of this query is held open by # low-star reviews the job never solves, so a plain fetch hides the solvable - # ones behind them for good. See triage.fetch_every_ticket. - tickets, total_matched = triage.fetch_every_ticket( + # ones behind them for good. See zendesk.fetch_every_ticket. + tickets, total_matched = zendesk.fetch_every_ticket( session, subdomain, query, args.max_tickets) matched = "?" if total_matched is None else total_matched print(f"Fetched {len(tickets)} of {matched} matching tickets (query: {query!r}).") diff --git a/zendesk_triage/test_note_reply.py b/zendesk_triage/test_note_reply.py index 26a72bb..991b7ab 100644 --- a/zendesk_triage/test_note_reply.py +++ b/zendesk_triage/test_note_reply.py @@ -23,9 +23,11 @@ sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import claude_cli # noqa: E402 import note_reply # noqa: E402 -import triage # noqa: E402 +import zendesk # noqa: E402 from shared.testing import Env, FakeResponse, FakeSession, Patched # noqa: E402 +from shared.text import undash_english # noqa: E402 API_USER = 901790886886 AGENT = 555 @@ -141,7 +143,7 @@ def test_an_up_to_date_transcript_is_not_re_rendered(self): author=API_USER, cid=8) comments = [comment("claude: english", cid=9), dict(comment("hallo", author=42, cid=7), public=True), prior] - with Patched(triage, conversation_turns=lambda *a: called.append(a)): + with Patched(zendesk, conversation_turns=lambda *a: called.append(a)): session = fake_session(*[FakeResponse({"ticket": {}})]) note_reply.run_english(session, "sub", "model", {"id": 7}, comments, {"id": 9, "author": AGENT, "action": "english", @@ -156,7 +158,7 @@ def test_nothing_new_is_not_an_error(self): comments = [comment("claude: english", cid=9), dict(comment("hallo", author=42, cid=7), public=True), prior] session = fake_session(*[FakeResponse({"ticket": {}})]) - with Patched(triage, conversation_turns=lambda *a: None): + with Patched(zendesk, conversation_turns=lambda *a: None): note_reply.run_english(session, "sub", "model", {"id": 7}, comments, {"id": 9, "author": AGENT, "action": "english", "brief": ""}, dry_run=False) @@ -173,9 +175,9 @@ def test_an_english_ticket_gets_no_transcript(self): """A transcript of English text repeats what is already on the ticket.""" turns = [{"index": 0, "who": "Customer", "when": "t", "body": "My app crashes"}] session = fake_session(*[FakeResponse({"ticket": {}})]) - with Patched(triage, conversation_turns=lambda *a: turns, - claude_cli_json=lambda *a, **k: { - "turns": [{"index": 0, "english": "My app crashes"}]}): + with Patched(zendesk, conversation_turns=lambda *a: turns), \ + Patched(claude_cli, run_json=lambda *a, **k: { + "turns": [{"index": 0, "english": "My app crashes"}]}): note_reply.run_english(session, "sub", "model", {"id": 7}, [comment("claude: english", cid=9)], {"id": 9, "author": AGENT, "action": "english", @@ -190,7 +192,8 @@ def fail(*a, **k): raise SystemExit("claude exited 1 on the English transcript of #7: 529.") session = fake_session(*[FakeResponse({"ticket": {}}), FakeResponse({}), FakeResponse({})]) - with Patched(triage, conversation_turns=lambda *a: turns, claude_cli_json=fail): + with Patched(zendesk, conversation_turns=lambda *a: turns), \ + Patched(claude_cli, run_json=fail): note_reply.run_english(session, "sub", "model", {"id": 7}, [comment("claude: english", cid=9)], {"id": 9, "author": AGENT, "action": "english", @@ -261,9 +264,9 @@ def test_the_transcript_note_is_never_public(self): turns = [{"index": 0, "who": "Customer", "when": "2026-09-03 10:00 UTC", "body": "Hallo"}] session = fake_session(*[FakeResponse({"ticket": {}})]) - with Patched(triage, conversation_turns=lambda *a: turns, - claude_cli_json=lambda *a, **k: {"turns": [{"index": 0, - "english": "Hello"}]}): + with Patched(zendesk, conversation_turns=lambda *a: turns), \ + Patched(claude_cli, run_json=lambda *a, **k: {"turns": [{"index": 0, + "english": "Hello"}]}): note_reply.run_english(session, "sub", "model", {"id": 7}, [dict(comment("Hallo", author=42, cid=3), public=True)], {"id": 9, "author": AGENT, "action": "english", @@ -684,8 +687,8 @@ def test_the_done_marker_is_keyed_on_the_command(self): def test_a_handled_command_is_recognised(self): note = note_reply.build_draft_note(GERMAN, "x", 42) - self.assertTrue(triage.has_marker([comment(note)], note_reply.done_marker(42))) - self.assertFalse(triage.has_marker([comment(note)], note_reply.done_marker(43))) + self.assertTrue(zendesk.has_marker([comment(note)], note_reply.done_marker(42))) + self.assertFalse(zendesk.has_marker([comment(note)], note_reply.done_marker(43))) class Writes(unittest.TestCase): @@ -943,21 +946,21 @@ def test_the_english_fields_are_cleaned_on_a_foreign_ticket_too(self): def test_a_range_becomes_a_plain_hyphen(self): """Not left alone: an unspaced long dash is still a long dash, and "14-21 days" is what a person would have typed.""" - self.assertEqual(triage.undash_english("kept 14—21 days"), "kept 14-21 days") - self.assertEqual(triage.undash_english("versions 2.14–2.15"), "versions 2.14-2.15") + self.assertEqual(undash_english("kept 14—21 days"), "kept 14-21 days") + self.assertEqual(undash_english("versions 2.14–2.15"), "versions 2.14-2.15") def test_no_long_dash_survives_anywhere(self): for text in ("a — b", "a—b", "a – b", "a–b", "— leading", "trailing —"): with self.subTest(text=text): - self.assertNotIn("—", triage.undash_english(text)) - self.assertNotIn("–", triage.undash_english(text)) + self.assertNotIn("—", undash_english(text)) + self.assertNotIn("–", undash_english(text)) def test_hyphens_in_words_survive(self): - self.assertEqual(triage.undash_english("end-to-end encrypted"), + self.assertEqual(undash_english("end-to-end encrypted"), "end-to-end encrypted") def test_en_dashes_go_too(self): - self.assertEqual(triage.undash_english("gone – sorry"), "gone, sorry") + self.assertEqual(undash_english("gone – sorry"), "gone, sorry") def test_every_option_is_cleaned_not_just_the_first(self): result = note_reply.validate_composition(dict(ENGLISH, options=[ diff --git a/zendesk_triage/test_resolve_reviews.py b/zendesk_triage/test_resolve_reviews.py index 7c82b65..cca2c55 100644 --- a/zendesk_triage/test_resolve_reviews.py +++ b/zendesk_triage/test_resolve_reviews.py @@ -22,6 +22,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import resolve_reviews # noqa: E402 import triage # noqa: E402 +import zendesk # noqa: E402 from shared.testing import FakeResponse, FakeSession, NoSleep, Patched # noqa: E402 from test_triage import ROOT, unit_commands # noqa: E402 @@ -103,7 +104,7 @@ def test_the_floor_is_fixed_at_four_stars(self): self.assertEqual([t["id"] for t in resolvable], [1]) def test_a_star_subject_counts_even_off_channel(self): - """triage.is_store_review accepts either signal; the rating still decides.""" + """zendesk.is_store_review accepts either signal; the rating still decides.""" resolvable, _ = self.select([review(1, stars=5, channel="email")]) self.assertEqual([t["id"] for t in resolvable], [1]) @@ -364,8 +365,8 @@ class TestSharedDetectionIsNotReimplemented(unittest.TestCase): triage's, so both scripts must call the same functions.""" def test_detection_comes_from_triage(self): - self.assertIs(resolve_reviews.triage.is_store_review, triage.is_store_review) - self.assertIs(resolve_reviews.triage.review_stars, triage.review_stars) + self.assertIs(resolve_reviews.zendesk.is_store_review, zendesk.is_store_review) + self.assertIs(resolve_reviews.zendesk.review_stars, zendesk.review_stars) def test_the_star_floor_matches_what_the_triage_skips(self): """The triage counts reviews above its floor without classifying them; this diff --git a/zendesk_triage/test_triage.py b/zendesk_triage/test_triage.py index c2ac3db..b7ae719 100644 --- a/zendesk_triage/test_triage.py +++ b/zendesk_triage/test_triage.py @@ -23,7 +23,10 @@ sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -import triage # noqa: E402 (needs the path insert above) +import claude_cli # noqa: E402 (needs the path inserts above) +import transcript # noqa: E402 +import triage # noqa: E402 +import zendesk # noqa: E402 from shared import discord # noqa: E402 from shared.testing import ( # noqa: E402 FakeResponse, FakeSession, NoSleep, NonJsonResponse, Patched) @@ -139,7 +142,7 @@ def test_store_reviews_are_never_in_scope(self): cannot be asked a follow-up. It is not work a digest can queue up.""" for query in (triage.build_window_query(72), triage.DEFAULT_QUERY): with self.subTest(query=query): - self.assertIn(f"-via:{triage.REVIEW_CHANNEL}", query) + self.assertIn(f"-via:{zendesk.REVIEW_CHANNEL}", query) def test_pending_tickets_are_out_of_scope(self): """A pending ticket is one somebody already answered. It leaves the queue on @@ -277,12 +280,12 @@ def metrics(self, *pairs): def test_it_attaches_the_requester_timestamp(self): tickets = [ticket(1), ticket(2)] session = FakeSession([self.metrics((1, "A"), (2, "B"))]) - self.assertEqual(triage.hydrate_requester_activity(session, "acme", tickets), 2) + self.assertEqual(zendesk.hydrate_requester_activity(session, "acme", tickets), 2) self.assertEqual([t["requester_updated_at"] for t in tickets], ["A", "B"]) def test_it_sideloads_rather_than_fetching_each_ticket(self): session = FakeSession([self.metrics((1, "A"))]) - triage.hydrate_requester_activity(session, "acme", [ticket(1)]) + zendesk.hydrate_requester_activity(session, "acme", [ticket(1)]) method, url, kwargs = session.calls[0] self.assertEqual(method, "GET") self.assertIn("show_many.json", url) @@ -292,7 +295,7 @@ def test_it_batches_at_a_hundred_ids(self): """show_many caps at 100 ids, so 150 tickets must be two requests.""" tickets = [ticket(i) for i in range(150)] session = FakeSession([self.metrics(), self.metrics()]) - triage.hydrate_requester_activity(session, "acme", tickets) + zendesk.hydrate_requester_activity(session, "acme", tickets) self.assertEqual(len(session.calls), 2) self.assertEqual(len(session.calls[0][2]["params"]["ids"].split(",")), 100) self.assertEqual(len(session.calls[1][2]["params"]["ids"].split(",")), 50) @@ -304,7 +307,7 @@ def test_a_failed_sideload_leaves_the_ticket_alone(self): requests.ConnectionError("unreachable")): with self.subTest(response=type(response).__name__): with NoSleep(): - triage.hydrate_requester_activity( + zendesk.hydrate_requester_activity( FakeSession([response] * 2), "acme", tickets) self.assertNotIn("requester_updated_at", tickets[0]) self.assertEqual(triage.activity_key(tickets[0]), "X") @@ -314,7 +317,7 @@ def test_a_ticket_the_requester_never_touched_falls_back(self): pin the ticket unchanged forever.""" tickets = [ticket(1, updated_at="X")] session = FakeSession([self.metrics((1, None))]) - triage.hydrate_requester_activity(session, "acme", tickets) + zendesk.hydrate_requester_activity(session, "acme", tickets) self.assertEqual(triage.activity_key(tickets[0]), "X") @@ -804,20 +807,20 @@ def review(self, ticket_id, stars, channel="any_channel"): via={"channel": channel}) def test_star_count_is_read_from_the_subject(self): - self.assertEqual(triage.review_stars(self.review(1, 5)), 5) - self.assertEqual(triage.review_stars(self.review(2, 1)), 1) + self.assertEqual(zendesk.review_stars(self.review(1, 5)), 5) + self.assertEqual(zendesk.review_stars(self.review(2, 1)), 1) def test_non_review_subject_has_no_stars(self): - self.assertIsNone(triage.review_stars(ticket(1, subject="Notifications broken"))) + self.assertIsNone(zendesk.review_stars(ticket(1, subject="Notifications broken"))) def test_channel_identifies_a_review_without_stars_in_the_subject(self): """The channel is the reliable signal: only 287 of 2,656 sampled reviews carried the app-store tag, so tag-based filtering would miss most.""" - self.assertTrue(triage.is_store_review( + self.assertTrue(zendesk.is_store_review( ticket(1, subject="no stars here", via={"channel": "any_channel"}))) def test_web_tickets_are_not_reviews(self): - self.assertFalse(triage.is_store_review(ticket(1, via={"channel": "web"}))) + self.assertFalse(zendesk.is_store_review(ticket(1, via={"channel": "web"}))) def test_positive_reviews_are_skipped(self): keep, skipped = triage.partition_reviews( @@ -879,23 +882,23 @@ def app_store(self, ticket_id): }) def test_google_play_is_android(self): - self.assertEqual(triage.review_platform(self.google_play(1)), "android") + self.assertEqual(zendesk.review_platform(self.google_play(1)), "android") def test_the_app_store_is_ios(self): """Its registered name is the generic 'AppFollow: Review Monitor'; only the instance name says which store, so both names have to be searched.""" - self.assertEqual(triage.review_platform(self.app_store(1)), "ios") + self.assertEqual(zendesk.review_platform(self.app_store(1)), "ios") def test_a_review_naming_no_store_stays_unresolved(self): - self.assertIsNone(triage.review_platform(self.review(1))) - self.assertIsNone(triage.review_platform( + self.assertIsNone(zendesk.review_platform(self.review(1))) + self.assertIsNone(zendesk.review_platform( self.review(2, {"registered_integration_service_name": "Some Other Importer"}))) def test_non_review_tickets_are_not_a_source_of_platform(self): email = ticket(1, via={"channel": "email", "source": {"from": {"address": "a@b.c", "name": "A"}}}) - self.assertIsNone(triage.review_platform(email)) - self.assertIsNone(triage.review_platform(ticket(2, via={"channel": "web"}))) + self.assertIsNone(zendesk.review_platform(email)) + self.assertIsNone(zendesk.review_platform(ticket(2, via={"channel": "web"}))) def test_the_ticket_overrides_the_models_guess(self): findings = [finding(1, platform="ios"), finding(2, platform="unknown")] @@ -930,7 +933,7 @@ def test_the_digest_line_shows_the_store_the_review_came_from(self): self.assertNotIn(triage.PLATFORM_EMOJI["unknown"], line) def test_every_resolvable_source_maps_to_a_known_platform(self): - for _, platform in triage.REVIEW_SOURCE_PLATFORMS: + for _, platform in zendesk.REVIEW_SOURCE_PLATFORMS: self.assertIn(platform, triage.PLATFORMS) @@ -1017,7 +1020,7 @@ def test_hydration_requests_a_bounded_page_of_comments(self): session = FakeSession([FakeResponse({"comments": [{"body": "detail"}]})]) triage.hydrate_descriptions(session, "acme", [row]) _, _, kwargs = session.calls[0] - self.assertEqual(kwargs["params"], {"per_page": 10}) + self.assertEqual(kwargs["params"], {"per_page": 10, "sort_order": "asc"}) def test_hydration_leaves_the_ticket_alone_when_no_comment_adds_anything(self): row = ticket(1, subject="Conversation with x", description="Conversation with x") @@ -1246,23 +1249,23 @@ class TestResolveApiModel(unittest.TestCase): """The CLI resolves aliases itself; the API takes ids, so only that path maps.""" def test_every_alias_maps_to_an_id(self): - for alias, model_id in triage.API_MODEL_ALIASES.items(): - self.assertEqual(triage.resolve_api_model(alias), model_id) + for alias, model_id in claude_cli.API_MODEL_ALIASES.items(): + self.assertEqual(claude_cli.resolve_api_model(alias), model_id) self.assertTrue(model_id.startswith("claude-"), model_id) def test_the_default_model_resolves_to_an_api_id(self): """The API 404s on a bare shorthand, so whatever DEFAULT_MODEL is — a pinned id today, an alias if that ever changes — it has to resolve to one.""" - resolved = triage.resolve_api_model(triage.DEFAULT_MODEL) - self.assertNotIn(resolved, triage.API_MODEL_ALIASES) + resolved = claude_cli.resolve_api_model(triage.DEFAULT_MODEL) + self.assertNotIn(resolved, claude_cli.API_MODEL_ALIASES) self.assertTrue(resolved.startswith("claude-"), resolved) def test_a_full_id_passes_through(self): - self.assertEqual(triage.resolve_api_model("claude-opus-4-8"), "claude-opus-4-8") + self.assertEqual(claude_cli.resolve_api_model("claude-opus-4-8"), "claude-opus-4-8") def test_an_unknown_value_passes_through(self): """A model newer than this table should reach the API rather than be rewritten.""" - self.assertEqual(triage.resolve_api_model("claude-future-9"), "claude-future-9") + self.assertEqual(claude_cli.resolve_api_model("claude-future-9"), "claude-future-9") class TestAnalyzeInChunks(unittest.TestCase): @@ -1386,7 +1389,7 @@ def test_updated_at_is_not_sent_to_the_model(self): class TestFetchTickets(unittest.TestCase): def test_returns_the_total_match_count_alongside_the_batch(self): session = FakeSession([FakeResponse({"count": 47, "results": [ticket(1), ticket(2)]})]) - tickets, total = triage.fetch_tickets(session, "acme", "q", 100) + tickets, total = zendesk.fetch_tickets(session, "acme", "q", 100) self.assertEqual(len(tickets), 2) self.assertEqual(total, 47) @@ -1395,7 +1398,7 @@ def test_follows_pagination(self): FakeResponse({"count": 3, "results": [ticket(1)], "next_page": "https://n/2"}), FakeResponse({"count": 3, "results": [ticket(2), ticket(3)]}), ]) - tickets, total = triage.fetch_tickets(session, "acme", "q", 100) + tickets, total = zendesk.fetch_tickets(session, "acme", "q", 100) self.assertEqual([t["id"] for t in tickets], [1, 2, 3]) self.assertEqual(total, 3) @@ -1403,7 +1406,7 @@ def test_max_tickets_caps_the_batch_but_not_the_reported_total(self): session = FakeSession([ FakeResponse({"count": 500, "results": [ticket(i) for i in range(10)]}), ]) - tickets, total = triage.fetch_tickets(session, "acme", "q", 4) + tickets, total = zendesk.fetch_tickets(session, "acme", "q", 4) self.assertEqual(len(tickets), 4) self.assertEqual(total, 500) # the gap is what the digest surfaces @@ -1411,18 +1414,18 @@ def test_non_ticket_search_results_are_ignored(self): session = FakeSession([ FakeResponse({"count": 2, "results": [ticket(1), {"result_type": "user", "id": 9}]}), ]) - tickets, _ = triage.fetch_tickets(session, "acme", "q", 100) + tickets, _ = zendesk.fetch_tickets(session, "acme", "q", 100) self.assertEqual([t["id"] for t in tickets], [1]) def test_total_is_none_when_zendesk_omits_the_count(self): session = FakeSession([FakeResponse({"results": [ticket(1)]})]) - _, total = triage.fetch_tickets(session, "acme", "q", 100) + _, total = zendesk.fetch_tickets(session, "acme", "q", 100) self.assertIsNone(total) def test_forbidden_response_exits_with_a_hint(self): session = FakeSession([FakeResponse({}, status_code=403)]) with self.assertRaises(SystemExit): - triage.fetch_tickets(session, "acme", "q", 100) + zendesk.fetch_tickets(session, "acme", "q", 100) def test_pagination_stops_at_the_zendesk_result_limit(self): """Past 1000 results the search API 422s, so we never ask for that page. @@ -1430,7 +1433,7 @@ def test_pagination_stops_at_the_zendesk_result_limit(self): A caller asking for more gets the limit, not an error: one page beyond the cap is queued here and must go unrequested. """ - limit = triage.SEARCH_RESULT_LIMIT + limit = zendesk.SEARCH_RESULT_LIMIT pages = [ FakeResponse({ "count": 5000, @@ -1440,7 +1443,7 @@ def test_pagination_stops_at_the_zendesk_result_limit(self): for offset in range(0, limit + 100, 100) ] session = FakeSession(pages) - tickets, total = triage.fetch_tickets(session, "acme", "q", 5000) + tickets, total = zendesk.fetch_tickets(session, "acme", "q", 5000) self.assertEqual(len(tickets), limit) self.assertEqual(total, 5000) # the digest still reports the real backlog self.assertEqual(len(session.calls), limit // 100) @@ -1449,7 +1452,7 @@ def test_max_tickets_below_the_limit_still_wins(self): session = FakeSession([ FakeResponse({"count": 500, "results": [ticket(i) for i in range(100)]}), ]) - tickets, _ = triage.fetch_tickets(session, "acme", "q", 7) + tickets, _ = zendesk.fetch_tickets(session, "acme", "q", 7) self.assertEqual(len(tickets), 7) def test_an_unexpected_422_keeps_the_tickets_already_fetched(self): @@ -1458,7 +1461,7 @@ def test_an_unexpected_422_keeps_the_tickets_already_fetched(self): FakeResponse({"count": 900, "results": [ticket(1)], "next_page": "https://n/2"}), FakeResponse({"error": "invalid"}, status_code=422), ]) - tickets, total = triage.fetch_tickets(session, "acme", "q", 900) + tickets, total = zendesk.fetch_tickets(session, "acme", "q", 900) self.assertEqual([t["id"] for t in tickets], [1]) self.assertEqual(total, 900) @@ -1466,7 +1469,7 @@ def test_a_422_on_the_first_page_still_exits(self): """Nothing fetched means nothing to salvage — that's a real failure.""" session = FakeSession([FakeResponse({"error": "invalid"}, status_code=422)]) with self.assertRaises(SystemExit): - triage.fetch_tickets(session, "acme", "q", 100) + zendesk.fetch_tickets(session, "acme", "q", 100) class TestFetchEveryTicket(unittest.TestCase): @@ -1478,57 +1481,57 @@ def page(self, ids, count, stamp="2026-08-01T00:00:00Z"): def test_one_short_slice_is_a_single_query(self): session = FakeSession([self.page([1, 2, 3], 3)]) - tickets, total = triage.fetch_every_ticket(session, "acme", "q", 100) + tickets, total = zendesk.fetch_every_ticket(session, "acme", "q", 100) self.assertEqual([t["id"] for t in tickets], [1, 2, 3]) self.assertEqual((total, len(session.calls)), (3, 1)) def test_a_full_slice_is_followed_by_another(self): """A full 1000 means there may be more behind it, so the walk continues from the oldest created_at rather than stopping at Zendesk's ceiling.""" - first = list(range(triage.SEARCH_RESULT_LIMIT)) + first = list(range(zendesk.SEARCH_RESULT_LIMIT)) session = FakeSession([ self.page(first, 1036, "2026-08-02T00:00:00Z"), self.page(range(9000, 9036), 36, "2025-08-02T00:00:00Z"), ]) - tickets, total = triage.fetch_every_ticket(session, "acme", "q", 5000) - self.assertEqual(len(tickets), triage.SEARCH_RESULT_LIMIT + 36) + tickets, total = zendesk.fetch_every_ticket(session, "acme", "q", 5000) + self.assertEqual(len(tickets), zendesk.SEARCH_RESULT_LIMIT + 36) self.assertEqual(total, 1036, "total comes from the unsliced query") self.assertIn("created<=2026-08-02T00:00:00Z", session.calls[1][2]["params"]["query"]) def test_the_overlapping_second_is_not_counted_twice(self): """created<= re-fetches everything sharing the oldest second; ids dedupe it.""" - first = list(range(triage.SEARCH_RESULT_LIMIT)) + first = list(range(zendesk.SEARCH_RESULT_LIMIT)) session = FakeSession([ self.page(first, 1002), self.page([998, 999, 1000, 1001], 4), self.page([], 0), ]) - tickets, _ = triage.fetch_every_ticket(session, "acme", "q", 5000) + tickets, _ = zendesk.fetch_every_ticket(session, "acme", "q", 5000) self.assertEqual(len(tickets), len({t["id"] for t in tickets})) - self.assertEqual(len(tickets), triage.SEARCH_RESULT_LIMIT + 2) + self.assertEqual(len(tickets), zendesk.SEARCH_RESULT_LIMIT + 2) def test_a_slice_that_adds_nothing_new_ends_the_walk(self): """Otherwise a tie group larger than a slice would loop forever.""" - first = list(range(triage.SEARCH_RESULT_LIMIT)) + first = list(range(zendesk.SEARCH_RESULT_LIMIT)) session = FakeSession([self.page(first, 99999), self.page(first, 99999)]) - tickets, _ = triage.fetch_every_ticket(session, "acme", "q", 99999) - self.assertEqual(len(tickets), triage.SEARCH_RESULT_LIMIT) + tickets, _ = zendesk.fetch_every_ticket(session, "acme", "q", 99999) + self.assertEqual(len(tickets), zendesk.SEARCH_RESULT_LIMIT) self.assertEqual(len(session.calls), 2) def test_it_never_returns_more_than_asked_for(self): session = FakeSession([self.page(range(10), 10)]) - tickets, _ = triage.fetch_every_ticket(session, "acme", "q", 4) + tickets, _ = zendesk.fetch_every_ticket(session, "acme", "q", 4) self.assertEqual(len(tickets), 4) class TestBacklogQueries(unittest.TestCase): def test_the_non_review_query_is_the_backlog_minus_the_review_channel(self): self.assertTrue(triage.BACKLOG_NON_REVIEW_QUERY.startswith(triage.BACKLOG_QUERY)) - self.assertIn(f"-via:{triage.REVIEW_CHANNEL}", triage.BACKLOG_NON_REVIEW_QUERY) + self.assertIn(f"-via:{zendesk.REVIEW_CHANNEL}", triage.BACKLOG_NON_REVIEW_QUERY) def test_the_counter_honours_the_query_it_is_given(self): session = FakeSession([FakeResponse({"count": 428})]) - count = triage.fetch_total_unsolved(session, "acme", triage.BACKLOG_NON_REVIEW_QUERY) + count = zendesk.count_tickets(session, "acme", triage.BACKLOG_NON_REVIEW_QUERY) self.assertEqual(count, 428) self.assertEqual(session.calls[0][2]["params"]["query"], triage.BACKLOG_NON_REVIEW_QUERY) @@ -1537,29 +1540,29 @@ def test_the_counter_honours_the_query_it_is_given(self): class TestFetchTotalUnsolved(unittest.TestCase): def test_returns_the_count(self): session = FakeSession([FakeResponse({"count": 5609})]) - self.assertEqual(triage.fetch_total_unsolved(session, "acme"), 5609) + self.assertEqual(zendesk.count_tickets(session, "acme", triage.BACKLOG_QUERY), 5609) def test_failure_is_non_fatal(self): """The backlog number is context, not a reason to abort the digest.""" session = FakeSession([FakeResponse({}, status_code=500)] * 2) - self.assertIsNone(triage.fetch_total_unsolved(session, "acme")) + self.assertIsNone(zendesk.count_tickets(session, "acme", triage.BACKLOG_QUERY)) def test_uses_a_short_retry_budget(self): """A full 6-attempt backoff would stall the digest ~60s for optional data.""" session = FakeSession([FakeResponse({}, status_code=500)] * 6) - triage.fetch_total_unsolved(session, "acme") + zendesk.count_tickets(session, "acme", triage.BACKLOG_QUERY) self.assertEqual(len(session.calls), 2) def test_a_transport_failure_is_non_fatal_too(self): """request_with_retry re-raises once its budget is spent; None is documented.""" session = FakeSession([requests.ConnectionError("no route")] * 2) with NoSleep(): - self.assertIsNone(triage.fetch_total_unsolved(session, "acme")) + self.assertIsNone(zendesk.count_tickets(session, "acme", triage.BACKLOG_QUERY)) def test_a_non_json_body_is_non_fatal(self): """A 200 with an HTML error page (proxy, maintenance) must not abort the run.""" session = FakeSession([NonJsonResponse()]) - self.assertIsNone(triage.fetch_total_unsolved(session, "acme")) + self.assertIsNone(zendesk.count_tickets(session, "acme", triage.BACKLOG_QUERY)) # ---- The Claude Code CLI --------------------------------------------------- @@ -1589,8 +1592,8 @@ def fake_run(command, **kwargs): "stdout": written if stdout is None else stdout, "stderr": stderr})() - with Patched(triage.subprocess, run=fake_run): - return triage.claude_cli_json("claude-opus-5", "medium", "be terse", + with Patched(claude_cli.subprocess, run=fake_run): + return claude_cli.run_json("claude-opus-5", "medium", "be terse", self.SCHEMA, prompt, 60, "a batch of 3") def command(self, **kwargs): @@ -1618,7 +1621,7 @@ def test_an_anthropic_key_in_the_environment_cannot_outrank_the_login(self): "PATH": "/usr/bin", "HOME": "/home/zendesk"}): self.run_cli() child_env = self.calls[0][1]["env"] - for name in triage.CLAUDE_AUTH_OVERRIDES: + for name in claude_cli.CLAUDE_AUTH_OVERRIDES: self.assertNotIn(name, child_env) # Everything else still reaches it: HOME is where the login lives, and PATH is # how a per-user install is found at all. @@ -1799,7 +1802,7 @@ def test_a_missing_cli_says_what_to_install(self): def test_a_wedged_cli_does_not_hang_the_run(self): with self.assertRaises(SystemExit) as caught: - self.run_cli(raises=triage.subprocess.TimeoutExpired("claude", 60)) + self.run_cli(raises=claude_cli.subprocess.TimeoutExpired("claude", 60)) self.assertIn("60s", str(caught.exception)) @@ -1896,18 +1899,18 @@ 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(triage.is_english({"language": language}), language) + 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(triage.is_english({"language": language}), repr(language)) + 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(triage.is_english({"language": language}), language) + self.assertFalse(transcript.is_english({"language": language}), language) # ---- turns ------------------------------------------------------------- @@ -1918,8 +1921,8 @@ class Resp: def json(): return {"comments": comments} - with Patched(triage, request_with_retry=lambda *a, **k: Resp()): - return triage.conversation_turns(object(), "acme", + with Patched(zendesk, request_with_retry=lambda *a, **k: Resp()): + return zendesk.conversation_turns(object(), "acme", {"id": 1, "requester_id": requester_id}) def comment(self, body, author_id=5, public=True, created_at="2026-08-28T00:22:38Z"): @@ -1971,7 +1974,7 @@ def test_python_owns_the_timestamps_and_the_speaker_labels(self): "body": "Es geht nicht."}, {"index": 1, "who": "Support", "when": "2026-08-28 01:31 UTC", "body": "Have you tried?"}] - got = triage.render_transcript(turns, [ + got = transcript.render_transcript(turns, [ {"index": 0, "english": "It does not work."}, {"index": 1, "english": "Have you tried?"}, ]) @@ -1984,7 +1987,7 @@ def test_a_turn_the_model_skipped_keeps_its_original_text(self): 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 = triage.render_transcript(turns, [{"index": 0, "english": "Broken."}]) + got = transcript.render_transcript(turns, [{"index": 0, "english": "Broken."}]) self.assertIn("Broken.", got) self.assertIn("Immer noch.", got) @@ -1993,21 +1996,21 @@ def test_a_scrambled_index_does_not_shift_every_later_turn(self): 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 = triage.render_transcript(turns, [{"index": 1, "english": "two"}, + 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): - self.assertEqual(triage.stamp_minutes("not a date"), "") - self.assertEqual(triage.stamp_minutes(None), "") - got = triage.render_transcript( + self.assertEqual(zendesk.stamp_minutes("not a date"), "") + self.assertEqual(zendesk.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 front of every paragraph.""" - self.assertEqual(triage.stamp_minutes("2026-08-28T01:31:09Z"), + self.assertEqual(zendesk.stamp_minutes("2026-08-28T01:31:09Z"), "2026-08-28 01:31 UTC") # ---- the run ----------------------------------------------------------- @@ -2017,9 +2020,9 @@ def test_nothing_happens_until_the_field_exists(self): to behave exactly as it did before — no comment fetches, no Claude calls, no writes.""" calls = [] - with Patched(triage, conversation_turns=lambda *a: calls.append(a), - claude_cli_json=lambda *a, **k: calls.append(a)): - written = triage.attach_english(object(), "acme", [{"id": 1}], + with Patched(zendesk, conversation_turns=lambda *a: calls.append(a)), \ + Patched(claude_cli, run_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) @@ -2029,7 +2032,7 @@ 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( - triage.attach_english(None, "acme", [{"id": 1}], + transcript.attach_english(None, "acme", [{"id": 1}], [{"id": 1, "language": "German"}], "claude-sonnet-5", field_id=42), 0) @@ -2039,7 +2042,7 @@ def test_a_run_that_posts_no_card_writes_nothing(self): 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(") + call = source.index("transcript.attach_english(") guard = source.rindex("if needs_discord:", 0, call) self.assertNotIn("\n ", source[guard:call].rstrip()) @@ -2050,21 +2053,21 @@ def said(self, **kwargs): model="claude-sonnet-5", field_id=42) defaults.update(kwargs) with contextlib.redirect_stdout(out): - triage.attach_english(**defaults) + 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(triage, conversation_turns=lambda *a: None): + with Patched(zendesk, 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(triage.ENGLISH_FIELD_ENV, said) + 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"}, @@ -2078,7 +2081,7 @@ def test_a_ticket_that_was_classified_but_never_fetched_is_named(self): self.assertIn("never fetched", said) def test_a_ticket_with_no_public_comments_is_named(self): - with Patched(triage, conversation_turns=lambda *a: None): + with Patched(zendesk, conversation_turns=lambda *a: None): said = self.said(tickets=[{"id": 7}], findings=[{"id": 7, "language": "German"}]) self.assertIn("7", said) @@ -2092,12 +2095,11 @@ def explode(*args, **kwargs): raise SystemExit("claude exited 1") written = [] - with Patched(triage, - 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 = triage.attach_english( + with Patched(zendesk, conversation_turns=lambda *a: [ + {"index": 0, "who": "Customer", "when": "", "body": "x"}]), \ + Patched(claude_cli, run_json=explode), \ + Patched(transcript, 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) @@ -2106,14 +2108,13 @@ def explode(*args, **kwargs): def test_the_transcript_is_written_to_the_configured_field(self): puts = [] - with Patched(triage, - 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 = triage.attach_english( + with Patched(zendesk, conversation_turns=lambda *a: [ + {"index": 0, "who": "Customer", "when": "2026-08-28 00:22 UTC", + "body": "Es geht nicht."}]), \ + Patched(claude_cli, run_json=lambda *a, **k: { + "turns": [{"index": 0, "english": "It does not work."}]}), \ + Patched(transcript, 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) @@ -2126,13 +2127,12 @@ def test_the_transcript_is_written_to_the_configured_field(self): 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(triage, - conversation_turns=lambda *a: [ - {"index": 0, "who": "Customer", "when": "", "body": "x"}], - claude_cli_json=lambda *a, **k: {"turns": []}, - write_english_field=lambda *a: True): + with Patched(zendesk, conversation_turns=lambda *a: [ + {"index": 0, "who": "Customer", "when": "", "body": "x"}]), \ + Patched(claude_cli, run_json=lambda *a, **k: {"turns": []}), \ + Patched(transcript, write_english_field=lambda *a: True): self.assertEqual( - triage.attach_english(object(), "acme", [], + transcript.attach_english(object(), "acme", [], [{"id": 99, "language": "German"}], "claude-sonnet-5", field_id=42), 0) @@ -2155,7 +2155,7 @@ def said(body, author, public=True, cid=1): def test_an_ordinary_ticket_costs_no_lookups(self): session = FakeSession([]) - authors = triage.customer_authors( + authors = zendesk.customer_authors( session, "acme", self.EMAIL, [self.said("Hallo", 999)]) self.assertEqual(authors, {999}) self.assertEqual(session.calls, [], "the requester wrote, so no role lookup") @@ -2163,7 +2163,7 @@ def test_an_ordinary_ticket_costs_no_lookups(self): def test_a_dm_falls_back_to_whoever_is_not_an_agent(self): session = FakeSession([FakeResponse({"user": {}}), FakeResponse({"user": {"id": 7, "role": "admin"}})]) - authors = triage.customer_authors(session, "acme", self.TWEET, [ + authors = zendesk.customer_authors(session, "acme", self.TWEET, [ self.said("中国大陆可以使用吗？", -1), self.said("Thanks for getting in touch.", 7)]) self.assertEqual(authors, {-1}) @@ -2171,13 +2171,13 @@ def test_a_dm_falls_back_to_whoever_is_not_an_agent(self): def test_an_unresolvable_author_counts_as_the_customer(self): session = FakeSession([FakeResponse({}, status_code=404)]) self.assertEqual( - triage.customer_authors(session, "acme", self.TWEET, + zendesk.customer_authors(session, "acme", self.TWEET, [self.said("中国大陆", -1)]), {-1}) def test_the_sample_keeps_their_words_and_drops_the_agent_s(self): session = FakeSession([FakeResponse({"user": {}}), FakeResponse({"user": {"id": 7, "role": "admin"}})]) - sample = triage.customer_text(session, "acme", self.TWEET, [ + sample = zendesk.customer_text(session, "acme", self.TWEET, [ self.said("中国大陆可以使用吗？", -1), self.said("Thanks for getting in touch.", 7)], 2000) self.assertIn("中国大陆可以使用吗", sample) @@ -2187,19 +2187,19 @@ def test_channel_boilerplate_is_not_the_customer_writing(self): """"Conversation with " is the ticket's own description, not words anybody typed, and it was what the language detector saw.""" session = FakeSession([FakeResponse({"user": {}})]) - sample = triage.customer_text(session, "acme", self.TWEET, + sample = zendesk.customer_text(session, "acme", self.TWEET, [self.said("中国大陆", -1)], 2000) self.assertNotIn("Conversation with", sample) def test_private_notes_never_reach_the_sample(self): session = FakeSession([]) - sample = triage.customer_text(session, "acme", self.EMAIL, + sample = zendesk.customer_text(session, "acme", self.EMAIL, [self.said("claude: draft - x", 999, public=False)], 2000) self.assertNotIn("claude: draft", sample) def test_a_ticket_with_no_text_still_yields_a_sample(self): session = FakeSession([]) - self.assertTrue(triage.customer_text( + self.assertTrue(zendesk.customer_text( session, "acme", dict(self.EMAIL, description=""), [], 2000).strip()) diff --git a/zendesk_triage/transcript.py b/zendesk_triage/transcript.py new file mode 100644 index 0000000..070a734 --- /dev/null +++ b/zendesk_triage/transcript.py @@ -0,0 +1,198 @@ +"""A ticket's conversation in English, for the agent who cannot read the original. + +Written to a custom field by the digest before it posts, so every ticket that can +reach the reply dialog already carries one, and on demand by the `claude: english` +note. Both go through translate(): the model translates, and nothing else. +""" +import json +import os +import sys + +import requests + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from shared.text import clip # noqa: E402 + +import claude_cli # noqa: E402 +import zendesk # noqa: E402 + +# 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" +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. +TRANSCRIPT_INPUT_CHARS = 8000 +TRANSCRIPT_CHARS = 12000 + +TRANSCRIPT_SCHEMA = { + "type": "object", + "additionalProperties": False, + "required": ["turns"], + "properties": { + "turns": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": False, + "required": ["index", "english"], + "properties": { + "index": { + "type": "integer", + "description": "The turn's index, echoed back unchanged.", + }, + "english": { + "type": "string", + "description": "That turn in English, or the original text unchanged if it was already English.", + }, + }, + }, + } + }, +} + + +TRANSCRIPT_SYSTEM_PROMPT = ( + "You translate support conversations into English for an agent who does not read " + "the original language.\n\n" + "You are given the turns of one ticket as JSON, each with an index. Return one " + "object per input turn, echoing its index back unchanged.\n\n" + "Translate faithfully and completely. Keep the speaker's meaning, their order of " + "events and their tone — an angry turn must still read as angry. Do not " + "summarise, do not answer, do not merge turns, do not add notes of your own.\n\n" + "A turn already in English is returned unchanged, word for word. Do not " + "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 field is only worth writing 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. + """ + language = (finding.get("language") or "").strip().lower() + return not language or language.startswith(("english", "en")) + + +def translate(model, turns, ticket_id): + """The turns in English: (turns, None), or (None, why) when the model call failed. + + Python owns the speaker labels and timestamps and the model only translates. 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. + """ + payload = json.dumps([{"index": t["index"], "speaker": t["who"], "text": t["body"]} + for t in turns], ensure_ascii=False) + rendered, failure = claude_cli.try_run_json( + model, "medium", TRANSCRIPT_SYSTEM_PROMPT, TRANSCRIPT_SCHEMA, + clip(payload, TRANSCRIPT_INPUT_CHARS), ENGLISH_TIMEOUT_SECONDS, + f"the English transcript of #{ticket_id}") + if failure: + return None, failure + return rendered.get("turns"), None + + +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. + """ + fields = {"custom_fields": [{"id": field_id, "value": english}]} + try: + resp = zendesk.update_ticket(session, subdomain, ticket_id, fields, attempts=2) + 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 about to be posted 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. + + 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)} being posted; " + 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 = zendesk.conversation_turns(session, subdomain, ticket) + if not turns: + print(f"Note: #{ticket['id']} has no public comments to render.") + continue + rendered, failure = translate(model, turns, ticket["id"]) + if failure: + print(f"Note: could not render #{ticket['id']} in English ({failure}).") + continue + english = clip(render_transcript(turns, rendered), 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/zendesk_triage/triage.py b/zendesk_triage/triage.py index 4e44201..93fcfb2 100644 --- a/zendesk_triage/triage.py +++ b/zendesk_triage/triage.py @@ -72,8 +72,6 @@ import argparse import json import os -import re -import subprocess import sys import textwrap from datetime import datetime, timedelta, timezone @@ -85,17 +83,17 @@ from shared import discord, state as dedup # noqa: E402 from shared.discord import post_to_discord # noqa: E402 from shared.env import get_env # noqa: E402 -from shared.retry import request_with_retry # noqa: E402 from shared.text import clip, squash, window_label # noqa: E402 -# The channel AppFollow imports app-store reviews on. Identified reviews with no -# false positives in a 3,662-ticket sample; tags did not (only 287 carried one). -REVIEW_CHANNEL = "any_channel" +import claude_cli # noqa: E402 +import transcript # noqa: E402 +import zendesk # noqa: E402 + # 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 # question — so it is not work a digest can queue up for someone. The volume stays # visible in the header's review count. -NO_REVIEWS = f"-via:{REVIEW_CHANNEL}" +NO_REVIEWS = f"-via:{zendesk.REVIEW_CHANNEL}" # New and open tickets, newest first. Broad on purpose within that: we want bug # reports AND low-star reviews, legal requests, security/legislation questions, and @@ -110,12 +108,6 @@ # The queue awaiting a human, for context in the digest. Not analyzed — just counted, # and scoped the same way as the analysis so the header and the body agree. BACKLOG_QUERY = "type:ticket status= 400: - sys.exit(f"Zendesk search failed ({resp.status_code}): {resp.text[:300]}") - payload = resp.json() - if total_matched is None: - total_matched = payload.get("count") - for row in payload.get("results", []): - if row.get("result_type") != "ticket": - continue - tickets.append(row) - if len(tickets) >= cap: - break - url = payload.get("next_page") - return tickets, total_matched - - -def fetch_ticket(session, subdomain, ticket_id): - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket_id}.json" - resp = request_with_retry(session, "GET", url) - if resp.status_code == 404: - sys.exit(f"Ticket #{ticket_id} does not exist.") - if resp.status_code >= 400: - # A read error's body describes the error, not the ticket, so it is safe to - # print here. The write path deliberately prints no body at all. - sys.exit(f"Could not read ticket #{ticket_id} ({resp.status_code}): " - f"{resp.text[:200]}") - return (resp.json() or {}).get("ticket") or {} - - -def fetch_comments(session, subdomain, ticket_id): - """The ticket's comments, newest first. - - Newest first because the marker that stops a re-run from writing twice will be on - the most recent comment, and one page of a busy ticket would otherwise be all - opening back-and-forth. - """ - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket_id}/comments.json" - resp = request_with_retry( - session, "GET", url, params={"per_page": 100, "sort_order": "desc"}) - if resp.status_code >= 400: - sys.exit(f"Could not read the comments on #{ticket_id} ({resp.status_code}).") - return (resp.json() or {}).get("comments") or [] - - -def fetch_total_unsolved(session, subdomain, query=BACKLOG_QUERY): - """Count an unsolved backlog. Best effort: returns None on failure. - - Context for the digest, not something to hold the run up for — hence the - short retry budget. - """ - url = f"https://{subdomain}.zendesk.com/api/v2/search/count.json" - try: - resp = request_with_retry( - session, "GET", url, attempts=2, params={"query": query} - ) - if resp.status_code >= 400: - print(f"Note: could not count the unsolved backlog ({resp.status_code}).") - return None - return resp.json().get("count") - except (requests.RequestException, ValueError) as exc: - # request_with_retry re-raises the transport error once its (short) budget is - # spent, and .json() raises on a non-JSON body — neither is a reason to lose - # the digest over one context number, so both land on the documented None. - print(f"Note: could not count the unsolved backlog ({exc}).") - return None - - # ---- Dedup state ----------------------------------------------------------- # # Maps ticket id -> {requester_updated_at, last_reported}. A ticket is re-reported @@ -550,42 +370,6 @@ def activity_key(ticket): return ticket.get("requester_updated_at") or ticket.get("updated_at") -def hydrate_requester_activity(session, subdomain, tickets): - """Fill `requester_updated_at` from each ticket's metric set. Returns the count. - - Sideloaded through show_many, so this is one request per 100 tickets rather than - one per ticket. - """ - hydrated = 0 - ids = [t["id"] for t in tickets if t.get("id") is not None] - for start in range(0, len(ids), 100): - chunk = ids[start : start + 100] - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/show_many.json" - try: - resp = request_with_retry(session, "GET", url, attempts=2, params={ - "ids": ",".join(str(i) for i in chunk), "include": "metric_sets"}) - except requests.RequestException as exc: - print(f"Note: could not fetch ticket metrics ({exc}); " - f"falling back to updated_at for {len(chunk)} ticket(s).") - continue - if resp.status_code >= 400: - print(f"Note: ticket metrics returned {resp.status_code}; " - f"falling back to updated_at for {len(chunk)} ticket(s).") - continue - try: - metric_sets = resp.json().get("metric_sets", []) - except ValueError as exc: - print(f"Note: unreadable ticket metrics ({exc}); falling back to updated_at.") - continue - by_id = {m.get("ticket_id"): m.get("requester_updated_at") for m in metric_sets} - for ticket in tickets: - stamp = by_id.get(ticket.get("id")) - if stamp: - ticket["requester_updated_at"] = stamp - hydrated += 1 - return hydrated - - STATE = dedup.Tracker(STATE_VERSION, "ticket", lambda t: str(t.get("id")), activity_key, "requester_updated_at") @@ -599,177 +383,10 @@ def hydrate_requester_activity(session, subdomain, tickets): # actionable, so counting them beats paying tokens to classify them. # The same backlog minus store reviews. 92% of unsolved tickets are AppFollow # reviews, so the unqualified number reads as ~13x the queue that needs a human. -BACKLOG_NON_REVIEW_QUERY = f"{BACKLOG_QUERY} -via:{REVIEW_CHANNEL}" -STAR_SUBJECT = re.compile(r"^\s*([★☆]{1,10})") +BACKLOG_NON_REVIEW_QUERY = f"{BACKLOG_QUERY} -via:{zendesk.REVIEW_CHANNEL}" DEFAULT_REVIEW_STAR_FLOOR = 3 -# A long dash is the clearest tell that text was machine-written, and no reply this -# team has sent uses one. The prompts that write for customers forbid it; this is the -# failsafe, because a prompt rule is advisory and the text reaches a real person. -# The spaced form is punctuation and becomes a comma; anything left is joining two -# things, like a range, and becomes the hyphen a person would have typed. -PUNCTUATING_DASH = re.compile(r"(?:\s+[—–]\s*|\s*[—–]\s+)") -ANY_LONG_DASH = re.compile(r"[—–]") - - -def undash_english(text): - """Replace every em and en dash: punctuation with a comma, the rest with a hyphen. - - ENGLISH ONLY, and the name says so because passing anything else corrupts it. In - Russian and the other East Slavic languages the long dash carries the present-tense - copula that the grammar omits: "Москва — столица России" IS the verb, and the comma - this produces leaves a subject with no predicate. Spanish, French, Polish and - Chinese give it dialogue and parenthetical duty that a comma does not carry either. - - Only for text a model wrote. Rewriting punctuation somebody typed themselves would - be wrong even in English. - - A failsafe, not a style pass: the substitution is blunt enough to turn a legitimate - strong break into a comma splice ("I checked the logs — nothing was uploaded"), so - the prompt is what should keep dashes out and this is what catches the misses. - """ - return ANY_LONG_DASH.sub("-", PUNCTUATING_DASH.sub(", ", text or "")) - - -def marker(kind, value): - """A machine-readable marker for a ticket comment. - - One shape for all of them: `[kind:value]`, matched by has_marker. What it is for - is idempotency — a marker on the ticket says the work behind it is already done, - so a replayed webhook or a re-run writes nothing a second time. - - The kind carries its own namespace (`discord`, `claude:done`) so the strings are - byte-identical to the four hand-rolled versions this replaces. That matters: - markers are already written into real tickets, and a changed format would stop - matching them and let a replay send twice. - """ - return f"[{kind}:{value}]" - - -def has_marker(comments, wanted): - """Whether any comment already carries this marker.""" - return any(wanted in (comment.get("body") or "") for comment in comments) - - -AGENT_ROLES = ("agent", "admin") - - -def fetch_user(session, subdomain, user_id): - """One Zendesk user, or {} when it cannot be read. - - An author we cannot resolve is treated as a customer by customer_authors, so a - failed lookup widens the sample rather than silencing it. - """ - url = f"https://{subdomain}.zendesk.com/api/v2/users/{user_id}.json" - resp = request_with_retry(session, "GET", url, attempts=2) - if resp.status_code >= 400: - return {} - return (resp.json() or {}).get("user") or {} - - -def customer_authors(session, subdomain, ticket, comments): - """The author ids on the customer's side of this ticket. - - Deciding by `requester_id` alone is right for email and web tickets and wrong for - every channel integration: on a Twitter or Sunshine DM the integration authors the - customer's own message under its id, so the requester appears to have written - nothing. That dropped every word a Chinese reviewer wrote and had them answered in - English, and it labelled their message "Support" in the English transcript. - - So: the requester when they wrote anything, and otherwise everyone who is not an - agent here. Roles are looked up rather than inferred from the id, because the - integration's id is an account detail and an author we cannot resolve is a - customer, not an agent. - - Ordinary tickets cost no extra API calls at all — the requester wrote something, - and the lookup never happens. - """ - requester = ticket.get("requester_id") - if any(c.get("author_id") == requester for c in comments): - return {requester} - roles, customers = {}, set() - for comment in comments: - author = comment.get("author_id") - if author not in roles: - roles[author] = (fetch_user(session, subdomain, author) or {}).get("role") - if roles[author] not in AGENT_ROLES: - customers.add(author) - return customers or {requester} - - -def customer_text(session, subdomain, ticket, comments, limit): - """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, - and including it would drag detection towards English on exactly the tickets this - exists for. - """ - # 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) - subject = squash(ticket.get("subject")) - parts = [] - description = (ticket.get("description") or "").strip() - # A channel integration puts "Conversation with " here, which is the - # ticket's own boilerplate rather than anything the customer typed. - if description and squash(description) != subject: - parts.append(description) - for comment in reversed(comments): # oldest first, so it reads in order - if comment.get("author_id") not in authors: - continue - body = (comment.get("body") or "").strip() - if body and squash(body) != subject and body not in parts: - parts.append(body) - # A ticket can carry no text at all — an attachment, or an import that lost its - # body. Say so rather than sending an empty sample, which reads as a blank - # question the model has to answer anyway. - return clip("\n\n".join(parts), limit) or "(no text)" - - -def review_stars(ticket): - """Star count from an AppFollow review subject, or None if not a review subject.""" - match = STAR_SUBJECT.match(ticket.get("subject") or "") - return match.group(1).count("★") if match else None - - -def is_store_review(ticket): - return (((ticket.get("via") or {}).get("channel") == REVIEW_CHANNEL) - or STAR_SUBJECT.match(ticket.get("subject") or "") is not None) - - -# Zendesk names the integration that imported a review under `via.source.from`, and -# that name is the store it came from. Both names are searched because the two -# integrations put the store in different ones: Google Play is the registered -# service name itself, while the App Store's registered name is the generic -# "AppFollow: Review Monitor" and only the instance name — "AppFollow (Session - -# Private Messenger, App Store)" — says which store. Across 5,113 sampled reviews -# spanning 2022-2026 these were the only two integrations, and both named the store -# on every ticket. -REVIEW_SOURCE_PLATFORMS = (("google play", "android"), ("app store", "ios")) -REVIEW_SOURCE_NAME_FIELDS = ("registered_integration_service_name", - "integration_service_instance_name") - - -def review_platform(ticket): - """Store an app-store review was imported from, as a PLATFORMS value. - - None when the ticket is not a review or its source names no store we know, which - leaves the model's guess in place rather than replacing it with 'unknown'. - """ - if not is_store_review(ticket): - return None - source = ((ticket.get("via") or {}).get("source") or {}).get("from") or {} - service = source.get("service_info") or {} - names = " ".join(str(service.get(field) or "") - for field in REVIEW_SOURCE_NAME_FIELDS).lower() - for needle, platform in REVIEW_SOURCE_PLATFORMS: - if needle in names: - return platform - return None - - def apply_review_platform(findings, tickets): """Replace the model's guessed platform wherever the ticket states the store. @@ -778,7 +395,7 @@ def apply_review_platform(findings, tickets): often a few words in another language — only invents a disagreement. Returns how many findings this corrected. """ - platforms = {ticket.get("id"): review_platform(ticket) for ticket in tickets} + platforms = {ticket.get("id"): zendesk.review_platform(ticket) for ticket in tickets} corrected = 0 for finding in findings: platform = platforms.get(finding.get("id")) @@ -798,8 +415,8 @@ def partition_reviews(tickets, star_floor): """ keep, skipped = [], [] for ticket in tickets: - stars = review_stars(ticket) - if is_store_review(ticket) and stars is not None and stars > star_floor: + stars = zendesk.review_stars(ticket) + if zendesk.is_store_review(ticket) and stars is not None and stars > star_floor: skipped.append(ticket) else: keep.append(ticket) @@ -825,22 +442,11 @@ def hydrate_descriptions(session, subdomain, tickets): for ticket in tickets: if not is_content_free(ticket): continue - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket['id']}/comments.json" - try: - resp = request_with_retry(session, "GET", url, attempts=2, params={"per_page": 10}) - except requests.RequestException as exc: - # Hydration is an enrichment, never a reason to abort the digest: an - # unreachable comments endpoint just leaves the description as-is. - print(f"Note: could not fetch comments for #{ticket['id']} ({exc}).") - continue - if resp.status_code >= 400: - continue - try: - comments = resp.json().get("comments", []) - except ValueError as exc: - # A 200 carrying an HTML error page (proxy, maintenance) is the same kind - # of non-event as an HTTP error here — enrich what we can, skip the rest. - print(f"Note: unreadable comments payload for #{ticket['id']} ({exc}).") + # An enrichment, never a reason to abort the digest: a ticket whose comments + # cannot be read keeps the description it has. + comments = zendesk.fetch_comments(session, subdomain, ticket["id"], newest_first=False, + per_page=10, attempts=2, required=False) + if comments is None: continue subject = squash(ticket.get("subject")) bodies = [squash(c.get("body")) for c in comments] @@ -853,250 +459,6 @@ def hydrate_descriptions(session, subdomain, tickets): return hydrated -# ---- English transcript, for the reply dialog ------------------------------- - -# 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" -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. -TRANSCRIPT_INPUT_CHARS = 8000 -TRANSCRIPT_CHARS = 12000 -# Private notes are left out. They are internal annotation rather than conversation, -# they are already English — note_reply.py's own attribution notes among them — and -# translating its `[discord:…]` markers back would put bookkeeping in front of an -# agent as if the customer had said it. -CUSTOMER_TURN = "Customer" -SUPPORT_TURN = "Support" - -TRANSCRIPT_SCHEMA = { - "type": "object", - "additionalProperties": False, - "required": ["turns"], - "properties": { - "turns": { - "type": "array", - "items": { - "type": "object", - "additionalProperties": False, - "required": ["index", "english"], - "properties": { - "index": { - "type": "integer", - "description": "The turn's index, echoed back unchanged.", - }, - "english": { - "type": "string", - "description": "That turn in English, or the original text unchanged if it was already English.", - }, - }, - }, - } - }, -} - -TRANSCRIPT_SYSTEM_PROMPT = ( - "You translate support conversations into English for an agent who does not read " - "the original language.\n\n" - "You are given the turns of one ticket as JSON, each with an index. Return one " - "object per input turn, echoing its index back unchanged.\n\n" - "Translate faithfully and completely. Keep the speaker's meaning, their order of " - "events and their tone — an angry turn must still read as angry. Do not " - "summarise, do not answer, do not merge turns, do not add notes of your own.\n\n" - "A turn already in English is returned unchanged, word for word. Do not " - "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 field is only worth writing 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. - """ - language = (finding.get("language") or "").strip().lower() - return not language or language.startswith(("english", "en")) - - -def conversation_turns(session, subdomain, ticket): - """One ticket's public comments as turns, oldest first. None on any failure. - - Both sides, not just the requester's. A customer's second message is usually an - answer to a reply, and dropping the reply leaves "still broken" sitting under the - original complaint with nothing visible for it to be answering. - - Who spoke is decided by customer_authors, not by `requester_id` alone: on a - Twitter or Sunshine DM the integration authors the customer's message under its - own id, and comparing against the requester labelled their words "Support" in the - transcript an agent then read. A transcript that mislabels who spoke is worse than - none. - """ - requester = ticket.get("requester_id") - if requester is None: - print(f"Note: #{ticket['id']} has no requester_id; skipping its transcript.") - return None - url = f"https://{subdomain}.zendesk.com/api/v2/tickets/{ticket['id']}/comments.json" - try: - resp = request_with_retry(session, "GET", url, attempts=2, - params={"per_page": 100, "sort_order": "asc"}) - except requests.RequestException as exc: - print(f"Note: could not fetch comments for #{ticket['id']} ({exc}).") - return None - if resp.status_code >= 400: - print(f"Note: comments for #{ticket['id']} returned {resp.status_code}.") - return None - try: - comments = resp.json().get("comments", []) - except ValueError as exc: - print(f"Note: unreadable comments payload for #{ticket['id']} ({exc}).") - return None - public = [c for c in comments if c.get("public")] - authors = customer_authors(session, subdomain, ticket, public) - turns = [] - for comment in public: - body = (comment.get("body") or "").strip() - if not body: - continue - turns.append({ - "index": len(turns), - "who": (CUSTOMER_TURN if comment.get("author_id") in authors - else SUPPORT_TURN), - "when": stamp_minutes(comment.get("created_at")), - "body": body, - }) - return turns or None - - -def stamp_minutes(created_at): - """Zendesk's ISO timestamp as `2026-08-28 01:31 UTC`, or '' if unparseable. - - Minutes, not seconds: this dates a turn for somebody reading a conversation, and - the extra precision is noise in front of every paragraph. - """ - try: - when = datetime.strptime(created_at, "%Y-%m-%dT%H:%M:%SZ") - except (TypeError, ValueError): - return "" - return when.strftime("%Y-%m-%d %H:%M UTC") - - -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 = request_with_retry(session, "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 about to be posted 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. - - 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)} being posted; " - 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 - - def compact_ticket(ticket): """Reduce a Zendesk ticket to the fields Claude needs for triage.""" description = (ticket.get("description") or "").strip() @@ -1115,15 +477,6 @@ def compact_ticket(ticket): } -def resolve_api_model(model): - """Map a shorthand model name onto the id `claude --model` expects. - - Anything that isn't a known shorthand passes through untouched, so a pinned id - (`claude-opus-4-8`) or a model newer than this table still works. - """ - return API_MODEL_ALIASES.get(model, model) - - def build_analysis_prompt(compact_tickets): return ( "Classify every ticket in this batch and return one object per ticket.\n\n" @@ -1212,143 +565,9 @@ def analyze_in_chunks(analyzer, compact_tickets, batch_size): return findings -def cli_failure_detail(stdout, stderr, limit=CLI_FAILURE_CHARS): - """The readable half of a failed `claude --print` run, clipped for the log. - - stderr wins, but the failures that matter most — a refused login, an exhausted - limit — leave it empty and put their message in the `--output-format json` - envelope on stdout, where it sits behind enough usage boilerplate to survive no - clip at all. Hence parsing the envelope rather than clipping it. `terminal_reason` - rides along when it fits: it is what separates an auth failure from a limit. - - Output that is not that envelope is reported raw: a CLI that dies before emitting - one has still said the only thing anybody will get. - """ - detail = (stderr or "").strip() - if detail: - return detail[:limit] - raw = (stdout or "").strip() - try: - envelope = json.loads(raw) - except ValueError: - envelope = None - if isinstance(envelope, dict): - message = "" - for name in ("result", "error"): - value = envelope.get(name) - if isinstance(value, dict): - value = value.get("message") - if isinstance(value, str) and value.strip(): - message = value.strip() - break - reason = envelope.get("terminal_reason") or envelope.get("subtype") - reason = reason.strip() if isinstance(reason, str) else "" - if message and reason and len(message) + len(reason) + 3 <= limit: - return f"{message} ({reason})" - if message: - return message[:limit] - if reason: - return f"it reported {reason!r} and no message." - return raw[:limit] - - -def claude_cli_json(model, effort, system_prompt, schema, prompt, timeout, label): - """Run one schema-enforced Claude Code request. Returns the parsed payload. - - Shared by the digest's classification and note_reply.py's composing: same flags, - same error semantics, one place to keep them right. - - `--json-schema` enforces the schema the way the API's structured outputs did. - Authentication is whatever `claude` is already logged in as, so neither caller - holds a Claude key. - - The prompt goes over **stdin**, not argv. Linux caps one argument at 128KB - (MAX_ARG_STRLEN) and a full --batch-size 400 chunk is around 685KB, so passing it - as an argument would work on a normal day and die with "Argument list too long" - on a backfill. It is also the more private channel: argv is world-readable - through /proc, and these prompts carry ticket text. - """ - command = [ - CLAUDE_CLI, "--print", - "--model", model, - "--effort", effort, - "--system-prompt", system_prompt, - "--json-schema", json.dumps(schema), - "--output-format", "json", - "--no-session-persistence", - # Nothing outside this call may change what the model is told. The two flags - # cover different halves of that and neither implies the other: - # --setting-sources "" drops the user and project settings — and the hooks - # inside them — while --tools "" removes the tools. Without the first, a - # .claude/settings.json next to this file, or one in the service account's - # home, silently joins every classification and every translation. - # - # --tools stays last: it is variadic, so it swallows any following argument - # that does not begin with a dash. - "--setting-sources", "", - "--tools", "", - ] - child_env = {name: value for name, value in os.environ.items() - if name not in CLAUDE_AUTH_OVERRIDES} - try: - done = subprocess.run(command, input=prompt, capture_output=True, text=True, - check=False, timeout=timeout, env=child_env) - except FileNotFoundError: - sys.exit(f"{CLAUDE_CLI} is not on PATH. {label} runs through the Claude Code " - f"CLI, so it has to be installed and logged in.") - except subprocess.TimeoutExpired: - sys.exit(f"{CLAUDE_CLI} did not finish {label} within {timeout}s.") - - if done.returncode != 0: - detail = cli_failure_detail(done.stdout, done.stderr) - if not detail: - detail = ("it printed nothing, which is what a login it can no longer " - "use looks like; check that `claude` is still signed in.") - sys.exit(f"{CLAUDE_CLI} exited {done.returncode} on {label}: {detail}") - try: - response = json.loads(done.stdout) - except ValueError as exc: - sys.exit(f"{CLAUDE_CLI} returned output that is not JSON on {label} ({exc}).") - if not isinstance(response, dict): - sys.exit(f"{CLAUDE_CLI} returned {type(response).__name__} on {label}, " - f"expected an object.") - # is_error and subtype are the CLI's signals for success; stop_reason deliberately - # is not — a successful structured-output run reports "tool_use", because that is - # how the schema is enforced underneath. - if response.get("is_error") or response.get("subtype") != "success": - detail = cli_failure_detail(done.stdout, done.stderr) - sys.exit(f"{CLAUDE_CLI} reported failure on {label} " - f"(subtype={response.get('subtype')!r}, " - f"api_error_status={response.get('api_error_status')!r})" - f"{': ' + detail if detail else '.'}") - # stop_reason is worth reading for this one value. There is no --max-tokens to - # raise, so an answer too long to finish comes back as JSON that stops mid-object, - # and the parse below would report a baffling syntax error for something whose - # only fix is a smaller batch. - if response.get("stop_reason") == "max_tokens": - sys.exit(f"{CLAUDE_CLI} ran out of output tokens on {label}, so the JSON is " - f"incomplete. Lower --batch-size (currently splitting at " - f"{DEFAULT_BATCH_SIZE}).") - - # structured_output is the object --json-schema produced, so it beats re-parsing - # the `result` string: one less decode, and immune to prose alongside the JSON. - payload = response.get("structured_output") - if payload is not None: - return payload - raw = response.get("result") - if not raw: - sys.exit(f"{CLAUDE_CLI} returned neither structured_output nor a result " - f"on {label}.") - try: - return json.loads(raw) - except ValueError as exc: - sys.exit(f"{CLAUDE_CLI} result on {label} is not the JSON the schema asked " - f"for ({exc}).") - - def analyze(model, effort, compact_tickets): """Classify a batch through the Claude Code CLI. Returns findings.""" - payload = claude_cli_json( + payload = claude_cli.run_json( model, effort, SYSTEM_PROMPT, SCHEMA, build_analysis_prompt(compact_tickets), CLAUDE_TIMEOUT_SECONDS, f"a batch of {len(compact_tickets)} tickets") @@ -1417,10 +636,6 @@ def analyze(model, effort, compact_tickets): MAX_COMPONENT_CHARS = discord.MAX_MESSAGE_TEXT_CHARS -def ticket_url(subdomain, ticket_id): - return f"https://{subdomain}.zendesk.com/agent/tickets/{ticket_id}" - - def is_urgent(finding): return finding.get("category") in URGENT_CATEGORIES @@ -1452,7 +667,7 @@ def build_ticket_line(finding, subdomain, is_update=False): severity_marker(finding), CATEGORY_EMOJI.get(finding.get("category"), "•"), PLATFORM_EMOJI.get(finding.get("platform"), PLATFORM_EMOJI["unknown"]), - f"{marker}[#{tid}]({ticket_url(subdomain, tid)}) · " + f"{marker}[#{tid}]({zendesk.ticket_url(subdomain, tid)}) · " f"{clip(finding.get('summary'), SUMMARY_CHARS) or '(no summary)'}", ] root = clip(finding.get("likely_root_cause"), ROOT_CAUSE_CHARS) @@ -1478,7 +693,7 @@ def build_collapsed_line(collapsed, subdomain): """ links, used = [], 0 for f in collapsed: - link = f"[#{f.get('id')}]({ticket_url(subdomain, f.get('id'))})" + link = f"[#{f.get('id')}]({zendesk.ticket_url(subdomain, f.get('id'))})" used += len(link) + 2 # ", " if used > COLLAPSED_LINKS_CHARS: break @@ -1714,15 +929,15 @@ def main(): else: query = DEFAULT_QUERY - zd = zendesk_session(email, api_token) - tickets, total_matched = fetch_tickets(zd, subdomain, query, args.max_tickets) + zd = zendesk.api_session(email, api_token) + tickets, total_matched = zendesk.fetch_tickets(zd, subdomain, query, args.max_tickets) matched = "?" if total_matched is None else total_matched print(f"Fetched {len(tickets)} of {matched} matching tickets (query: {query!r}).") if total_matched is not None and total_matched > len(tickets): # Name whichever cap actually bound, so a truncated digest doesn't send # someone raising --max-tickets against a limit that isn't ours. - reason = (f"Zendesk's search API returns at most {SEARCH_RESULT_LIMIT} results" - if args.max_tickets >= SEARCH_RESULT_LIMIT + reason = (f"Zendesk's search API returns at most {zendesk.SEARCH_RESULT_LIMIT} results" + if args.max_tickets >= zendesk.SEARCH_RESULT_LIMIT else f"--max-tickets is {args.max_tickets}") print(f"Note: {total_matched - len(tickets)} matching tickets were not analyzed " f"({reason}).") @@ -1731,8 +946,8 @@ def main(): return stats["matched"] = total_matched - stats["total_unsolved"] = fetch_total_unsolved(zd, subdomain) - stats["total_unsolved_non_review"] = fetch_total_unsolved( + stats["total_unsolved"] = zendesk.count_tickets(zd, subdomain, BACKLOG_QUERY) + stats["total_unsolved_non_review"] = zendesk.count_tickets( zd, subdomain, BACKLOG_NON_REVIEW_QUERY) # Drop positive store reviews before anything expensive: they were 59% of all @@ -1751,7 +966,7 @@ def main(): # whenever either needs it. After the review filter, so it only covers # tickets that can still be reported. One request per 100 tickets. if window_start or args.state: - hydrate_requester_activity(zd, subdomain, tickets) + zendesk.hydrate_requester_activity(zd, subdomain, tickets) if window_start: tickets, quiet = drop_quiet_tickets(tickets, window_start) @@ -1792,7 +1007,7 @@ def main(): print("Classify it, then: --findings --dry-run") return - analyzer = partial(analyze, resolve_api_model(model), args.effort) + analyzer = partial(analyze, claude_cli.resolve_api_model(model), args.effort) findings = analyze_in_chunks(analyzer, compact, args.batch_size) # Keep only findings whose id maps to a fetched ticket, in case of drift. @@ -1829,8 +1044,8 @@ def main(): # rendering written there would be a write to a production ticket for a dialog # that can never be opened. if needs_discord: - attach_english(zd, subdomain, classified, shown, model, - get_env(ENGLISH_FIELD_ENV, required=False)) + transcript.attach_english(zd, subdomain, classified, shown, model, + get_env(transcript.ENGLISH_FIELD_ENV, required=False)) messages, coverage = build_messages(findings, subdomain, stats, updated_ids) if args.dry_run: diff --git a/zendesk_triage/zendesk.py b/zendesk_triage/zendesk.py new file mode 100644 index 0000000..114cccc --- /dev/null +++ b/zendesk_triage/zendesk.py @@ -0,0 +1,493 @@ +"""The Zendesk account as the scripts here see it. + +The API, through one authenticated session: search, one ticket, its comments and +users, the one PUT every write is. And the shapes in it that more than one script +reads: the markers the tools leave on comments, who the customer is on a channel +integration, what an imported app-store review looks like. + +Every read names its own failure policy, because the callers differ: a digest +enriching many tickets skips the one it cannot read, while a command acting on one +ticket has nothing to do without it and exits. +""" +import os +import re +import sys +from datetime import datetime + +import requests + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from shared.retry import request_with_retry # noqa: E402 +from shared.text import clip, squash # noqa: E402 + + +def api_url(subdomain, path): + return f"https://{subdomain}.zendesk.com/api/v2/{path}" + + +def ticket_url(subdomain, ticket_id): + return f"https://{subdomain}.zendesk.com/agent/tickets/{ticket_id}" + + +# The channel AppFollow imports app-store reviews on. Identified reviews with no +# false positives in a 3,662-ticket sample; tags did not (only 287 carried one). +REVIEW_CHANNEL = "any_channel" +# The Search API hard-caps a query at 1000 results and returns 422 for any page past +# it (at per_page=100 that is page 11), so pagination stops here rather than walking +# into that error. Above the cap the digest reports truncation — which it already does +# for --max-tickets — instead of failing the run. +# https://developer.zendesk.com/api-reference/ticketing/ticket-management/search/#results-limit +SEARCH_RESULT_LIMIT = 1000 + + +def api_session(email, token): + session = requests.Session() + # Zendesk API-token auth: username is "{email}/token", password is the token. + session.auth = (f"{email}/token", token) + session.headers["Accept"] = "application/json" + return session + + +def fetch_tickets(session, subdomain, query, max_tickets): + """Fetch tickets via the Zendesk Search API, following pagination. + + Returns (tickets, total_matched). total_matched is the full result count + reported by Zendesk, which can exceed len(tickets) when max_tickets — or + SEARCH_RESULT_LIMIT — caps the batch; the caller surfaces that gap so the + truncation isn't silent. + """ + base = api_url(subdomain, "search.json") + url = base + params = {"query": query, "per_page": 100} + tickets = [] + total_matched = None + # Whichever bites first: our own runaway guard or Zendesk's hard result limit. + cap = min(max_tickets, SEARCH_RESULT_LIMIT) + while url and len(tickets) < cap: + resp = request_with_retry(session, "GET", url, params=params) + params = None # next_page already carries the query + if resp.status_code == 403: + sys.exit("Zendesk returned 403 — the API token/email may lack search access.") + # 422 past the result limit: `cap` should have stopped us first, so this only + # fires if the account's effective limit is lower than documented. Keep the + # tickets already in hand — a partial digest beats no digest — and let the + # caller report the gap. With nothing in hand there is nothing to salvage. + if resp.status_code == 422 and tickets: + print(f"Note: Zendesk stopped paginating at {len(tickets)} results " + f"(search result limit); analyzing what was fetched.") + break + if resp.status_code >= 400: + sys.exit(f"Zendesk search failed ({resp.status_code}): {resp.text[:300]}") + payload = resp.json() + if total_matched is None: + total_matched = payload.get("count") + for row in payload.get("results", []): + if row.get("result_type") != "ticket": + continue + tickets.append(row) + if len(tickets) >= cap: + break + url = payload.get("next_page") + return tickets, total_matched + + +def fetch_every_ticket(session, subdomain, query, max_tickets): + """Fetch past the Search API's 1000-result ceiling, in created_at slices. + + The ceiling is per query, not per account: `created<=` the oldest result so far + is a different query with a fresh 1000 of its own. `query` must order by + created_at descending for that to hold. + + Without this, a query matching more than 1000 truncates at the newest 1000 and + the tail is unreachable at any --max-tickets — permanently, when the surplus is + tickets the caller never removes. The positive-review job hit exactly that: 1,036 + matches held open by 1,030 low-star reviews it will never solve, hiding six 4-5★ + ones from August 2025 that it would have. + + `created<=`, not `<`: created_at has second granularity, so `<` would skip every + ticket sharing the oldest second. The overlap is re-fetched and dropped by id + instead, and a slice that adds nothing new ends the walk — which is also what + stops a tie group larger than a whole slice from looping forever. + + Returns (tickets, total_matched) like fetch_tickets, with total_matched from the + unsliced query so the caller still reports the true gap. + """ + tickets, seen, total_matched, cutoff = [], set(), None, None + while len(tickets) < max_tickets: + sliced = query if cutoff is None else f"{query} created<={cutoff}" + batch, matched = fetch_tickets(session, subdomain, sliced, + max_tickets - len(tickets)) + if total_matched is None: + total_matched = matched + fresh = [t for t in batch if t.get("id") not in seen] + if not fresh: + break + seen.update(t.get("id") for t in fresh) + tickets.extend(fresh) + stamps = [t.get("created_at") for t in batch if t.get("created_at")] + if not stamps or len(batch) < SEARCH_RESULT_LIMIT: + break + cutoff = min(stamps) + return tickets[:max_tickets], total_matched + + +def fetch_ticket(session, subdomain, ticket_id): + url = api_url(subdomain, f"tickets/{ticket_id}.json") + resp = request_with_retry(session, "GET", url) + if resp.status_code == 404: + sys.exit(f"Ticket #{ticket_id} does not exist.") + if resp.status_code >= 400: + # A read error's body describes the error, not the ticket, so it is safe to + # print here. The write path deliberately prints no body at all. + sys.exit(f"Could not read ticket #{ticket_id} ({resp.status_code}): " + f"{resp.text[:200]}") + return (resp.json() or {}).get("ticket") or {} + + +def fetch_comments(session, subdomain, ticket_id, newest_first=True, per_page=100, + attempts=6, required=True): + """The ticket's comments; None when they cannot be read and `required` is False. + + Newest first by default: the marker that stops a re-run from writing twice is on + the most recent comment, and one page of a busy ticket would otherwise be all + opening back-and-forth. + + `required` exits on any failure: a command acting on one ticket has nothing to do + without them. A digest enriching many tickets passes False and skips the one it + cannot read. + """ + url = api_url(subdomain, f"tickets/{ticket_id}/comments.json") + params = {"per_page": per_page, "sort_order": "desc" if newest_first else "asc"} + if required: + resp = request_with_retry(session, "GET", url, attempts=attempts, params=params) + if resp.status_code >= 400: + sys.exit(f"Could not read the comments on #{ticket_id} ({resp.status_code}).") + return (resp.json() or {}).get("comments") or [] + try: + resp = request_with_retry(session, "GET", url, attempts=attempts, params=params) + except requests.RequestException as exc: + print(f"Note: could not fetch comments for #{ticket_id} ({exc}).") + return None + if resp.status_code >= 400: + print(f"Note: comments for #{ticket_id} returned {resp.status_code}.") + return None + try: + return resp.json().get("comments", []) + except ValueError as exc: + print(f"Note: unreadable comments payload for #{ticket_id} ({exc}).") + return None + + +def count_tickets(session, subdomain, query): + """How many tickets match `query`. Best effort: None on failure. + + Context for a digest header, not something to hold the run up for, hence the + short retry budget. + """ + url = api_url(subdomain, "search/count.json") + try: + resp = request_with_retry( + session, "GET", url, attempts=2, params={"query": query} + ) + if resp.status_code >= 400: + print(f"Note: could not count {query!r} ({resp.status_code}).") + return None + return resp.json().get("count") + except (requests.RequestException, ValueError) as exc: + # request_with_retry re-raises the transport error once its (short) budget is + # spent, and .json() raises on a non-JSON body — neither is a reason to lose + # the digest over one context number, so both land on the documented None. + print(f"Note: could not count {query!r} ({exc}).") + return None + + +def fetch_user(session, subdomain, user_id): + """One Zendesk user, or {} when it cannot be read. + + An author we cannot resolve is treated as a customer by customer_authors, so a + failed lookup widens the sample rather than silencing it. + """ + url = api_url(subdomain, f"users/{user_id}.json") + resp = request_with_retry(session, "GET", url, attempts=2) + if resp.status_code >= 400: + return {} + return (resp.json() or {}).get("user") or {} + + +def hydrate_requester_activity(session, subdomain, tickets): + """Fill `requester_updated_at` from each ticket's metric set. Returns the count. + + Sideloaded through show_many, so this is one request per 100 tickets rather than + one per ticket. + """ + hydrated = 0 + ids = [t["id"] for t in tickets if t.get("id") is not None] + for start in range(0, len(ids), 100): + chunk = ids[start : start + 100] + url = api_url(subdomain, "tickets/show_many.json") + try: + resp = request_with_retry(session, "GET", url, attempts=2, params={ + "ids": ",".join(str(i) for i in chunk), "include": "metric_sets"}) + except requests.RequestException as exc: + print(f"Note: could not fetch ticket metrics ({exc}); " + f"falling back to updated_at for {len(chunk)} ticket(s).") + continue + if resp.status_code >= 400: + print(f"Note: ticket metrics returned {resp.status_code}; " + f"falling back to updated_at for {len(chunk)} ticket(s).") + continue + try: + metric_sets = resp.json().get("metric_sets", []) + except ValueError as exc: + print(f"Note: unreadable ticket metrics ({exc}); falling back to updated_at.") + continue + by_id = {m.get("ticket_id"): m.get("requester_updated_at") for m in metric_sets} + for ticket in tickets: + stamp = by_id.get(ticket.get("id")) + if stamp: + ticket["requester_updated_at"] = stamp + hydrated += 1 + return hydrated + + +def update_ticket(session, subdomain, ticket_id, fields, attempts=6): + """PUT one ticket update; returns the response. + + Callers never print its body: Zendesk echoes a submitted comment back in a 422, + and a comment can be a reply to a customer. + """ + return request_with_retry(session, "PUT", api_url(subdomain, f"tickets/{ticket_id}.json"), + attempts=attempts, json={"ticket": fields}) + + +def api_user_id(session, subdomain): + """The user the API token authenticates as. + + Its own notes are skipped when looking for a command, which is the in-code half + of the loop guard. The other half is the Zendesk trigger, which should exclude + this same user so a draft never fires the webhook at all — see the README. + """ + url = api_url(subdomain, f"users/me.json") + resp = request_with_retry(session, "GET", url) + if resp.status_code >= 400: + sys.exit(f"Zendesk refused to identify the API user ({resp.status_code}).") + return ((resp.json() or {}).get("user") or {}).get("id") + + +def change_tags(session, subdomain, ticket_id, add=(), drop=()): + """Add and remove tags, through the tags sub-resource. + + NOT `additional_tags`/`remove_tags` on the ticket update: those are update_many + fields. A single-ticket update accepts them with a 200 and silently ignores them, + which is how every tag this tool set went missing while every call reported + success. Measured against the live API, not assumed. + + The sub-resource is also additive rather than read-modify-write, so two runs on + one ticket cannot clobber each other's tags. + """ + url = api_url(subdomain, f"tickets/{ticket_id}/tags.json") + for method, names in (("PUT", [t for t in add if t]), + ("DELETE", [t for t in drop if t])): + if not names: + continue + resp = request_with_retry(session, method, url, json={"tags": names}) + if resp.status_code >= 400: + # Never worth failing a run over: tags are a dashboard light, not the work. + print(f"Note: could not {method.lower()} tags on #{ticket_id} " + f"({resp.status_code}).") + + +# ---- Who said what -------------------------------------------------------- + + +def marker(kind, value): + """A machine-readable marker for a ticket comment. + + One shape for all of them: `[kind:value]`, matched by has_marker. What it is for + is idempotency — a marker on the ticket says the work behind it is already done, + so a replayed webhook or a re-run writes nothing a second time. + + The kind carries its own namespace (`discord`, `claude:done`) so the strings are + byte-identical to the four hand-rolled versions this replaces. That matters: + markers are already written into real tickets, and a changed format would stop + matching them and let a replay send twice. + """ + return f"[{kind}:{value}]" + + +def has_marker(comments, wanted): + """Whether any comment already carries this marker.""" + return any(wanted in (comment.get("body") or "") for comment in comments) + + +AGENT_ROLES = ("agent", "admin") + + +def customer_authors(session, subdomain, ticket, comments): + """The author ids on the customer's side of this ticket. + + Deciding by `requester_id` alone is right for email and web tickets and wrong for + every channel integration: on a Twitter or Sunshine DM the integration authors the + customer's own message under its id, so the requester appears to have written + nothing. That dropped every word a Chinese reviewer wrote and had them answered in + English, and it labelled their message "Support" in the English transcript. + + So: the requester when they wrote anything, and otherwise everyone who is not an + agent here. Roles are looked up rather than inferred from the id, because the + integration's id is an account detail and an author we cannot resolve is a + customer, not an agent. + + Ordinary tickets cost no extra API calls at all — the requester wrote something, + and the lookup never happens. + """ + requester = ticket.get("requester_id") + if any(c.get("author_id") == requester for c in comments): + return {requester} + roles, customers = {}, set() + for comment in comments: + author = comment.get("author_id") + if author not in roles: + roles[author] = (fetch_user(session, subdomain, author) or {}).get("role") + if roles[author] not in AGENT_ROLES: + customers.add(author) + return customers or {requester} + + +def customer_text(session, subdomain, ticket, comments, limit): + """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, + and including it would drag detection towards English on exactly the tickets this + exists for. + """ + # 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) + subject = squash(ticket.get("subject")) + parts = [] + description = (ticket.get("description") or "").strip() + # A channel integration puts "Conversation with " here, which is the + # ticket's own boilerplate rather than anything the customer typed. + if description and squash(description) != subject: + parts.append(description) + for comment in reversed(comments): # oldest first, so it reads in order + if comment.get("author_id") not in authors: + continue + body = (comment.get("body") or "").strip() + if body and squash(body) != subject and body not in parts: + parts.append(body) + # A ticket can carry no text at all — an attachment, or an import that lost its + # body. Say so rather than sending an empty sample, which reads as a blank + # question the model has to answer anyway. + return clip("\n\n".join(parts), limit) or "(no text)" + + +# Private notes are left out. They are internal annotation rather than conversation, +# they are already English — note_reply.py's own attribution notes among them — and +# translating its `[discord:…]` markers back would put bookkeeping in front of an +# agent as if the customer had said it. +CUSTOMER_TURN = "Customer" +SUPPORT_TURN = "Support" + + +def conversation_turns(session, subdomain, ticket): + """One ticket's public comments as turns, oldest first. None on any failure. + + Both sides, not just the requester's. A customer's second message is usually an + answer to a reply, and dropping the reply leaves "still broken" sitting under the + original complaint with nothing visible for it to be answering. + + Who spoke is decided by customer_authors, not by `requester_id` alone: on a + Twitter or Sunshine DM the integration authors the customer's message under its + own id, and comparing against the requester labelled their words "Support" in the + transcript an agent then read. A transcript that mislabels who spoke is worse than + none. + """ + requester = ticket.get("requester_id") + if requester is None: + print(f"Note: #{ticket['id']} has no requester_id; skipping its transcript.") + return None + comments = fetch_comments(session, subdomain, ticket["id"], newest_first=False, + attempts=2, required=False) + if comments is None: + return None + public = [c for c in comments if c.get("public")] + authors = customer_authors(session, subdomain, ticket, public) + turns = [] + for comment in public: + body = (comment.get("body") or "").strip() + if not body: + continue + turns.append({ + "index": len(turns), + "who": (CUSTOMER_TURN if comment.get("author_id") in authors + else SUPPORT_TURN), + "when": stamp_minutes(comment.get("created_at")), + "body": body, + }) + return turns or None + + +def stamp_minutes(created_at): + """Zendesk's ISO timestamp as `2026-08-28 01:31 UTC`, or '' if unparseable. + + Minutes, not seconds: this dates a turn for somebody reading a conversation, and + the extra precision is noise in front of every paragraph. + """ + try: + when = datetime.strptime(created_at, "%Y-%m-%dT%H:%M:%SZ") + except (TypeError, ValueError): + return "" + return when.strftime("%Y-%m-%d %H:%M UTC") + + +# ---- App-store reviews ------------------------------------------------------ + + +STAR_SUBJECT = re.compile(r"^\s*([★☆]{1,10})") + + +def review_stars(ticket): + """Star count from an AppFollow review subject, or None if not a review subject.""" + match = STAR_SUBJECT.match(ticket.get("subject") or "") + return match.group(1).count("★") if match else None + + +def is_store_review(ticket): + return (((ticket.get("via") or {}).get("channel") == REVIEW_CHANNEL) + or STAR_SUBJECT.match(ticket.get("subject") or "") is not None) + + +# Zendesk names the integration that imported a review under `via.source.from`, and +# that name is the store it came from. Both names are searched because the two +# integrations put the store in different ones: Google Play is the registered +# service name itself, while the App Store's registered name is the generic +# "AppFollow: Review Monitor" and only the instance name — "AppFollow (Session - +# Private Messenger, App Store)" — says which store. Across 5,113 sampled reviews +# spanning 2022-2026 these were the only two integrations, and both named the store +# on every ticket. +REVIEW_SOURCE_PLATFORMS = (("google play", "android"), ("app store", "ios")) + + +REVIEW_SOURCE_NAME_FIELDS = ("registered_integration_service_name", + "integration_service_instance_name") + + +def review_platform(ticket): + """Store an app-store review was imported from, as a PLATFORMS value. + + None when the ticket is not a review or its source names no store we know, which + leaves the model's guess in place rather than replacing it with 'unknown'. + """ + if not is_store_review(ticket): + return None + source = ((ticket.get("via") or {}).get("source") or {}).get("from") or {} + service = source.get("service_info") or {} + names = " ".join(str(service.get(field) or "") + for field in REVIEW_SOURCE_NAME_FIELDS).lower() + for needle, platform in REVIEW_SOURCE_PLATFORMS: + if needle in names: + return platform + return None + From 2fd6d903bfa4b7827dd07ea6edf44fd1da93afb2 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 24 Sep 2026 16:52:01 +1000 Subject: [PATCH 4/5] refactor: download Crowdin exports through the shared retry, with tests The script's own retry loop retried 429 only, crashed on a Retry-After given as a date, and re-sent the bearer token per call with no session. It now goes through shared.retry on one session, with the export downloads on a second, unauthenticated one: they are served from a signed URL on another host. Arguments were parsed at import time and the functions read module globals, which is why the script had no tests. The client is an object now, the arguments parse in main, and the printers come from generate_shared next door instead of being spelled out again. Every argument the workflow passes is unchanged. --- crowdin/download_translations_from_crowdin.py | 360 ++++++++---------- crowdin/test_download_translations.py | 111 ++++++ 2 files changed, 263 insertions(+), 208 deletions(-) create mode 100644 crowdin/test_download_translations.py diff --git a/crowdin/download_translations_from_crowdin.py b/crowdin/download_translations_from_crowdin.py index 033607b..f39ce23 100644 --- a/crowdin/download_translations_from_crowdin.py +++ b/crowdin/download_translations_from_crowdin.py @@ -1,229 +1,173 @@ -import os +#!/usr/bin/env python3 +"""Download every language of a Crowdin project as XLIFF, plus its non-translatable terms. + +Usage: + download_translations_from_crowdin.py + [--glossary_id ID --concept_id ID] [--skip-untranslated-strings] + [--force-allow-unapproved] [--max-workers N] [-v] +""" +import argparse import json +import os import sys -import argparse -import requests -import time from concurrent.futures import ThreadPoolExecutor, as_completed from threading import Lock, Semaphore -from colorama import Fore, Style, init - -# Initialize colorama -init(autoreset=True) - -# Parse command-line arguments -parser = argparse.ArgumentParser(description='Download translations from Crowdin.') -parser.add_argument('api_token', help='Crowdin API token') -parser.add_argument('project_id', help='Crowdin project ID') -parser.add_argument('download_directory', help='Directory to save the initial downloaded files') -parser.add_argument('--glossary_id', help='Crowdin glossary ID (optional)', default=None) -parser.add_argument('--concept_id', help='Crowdin non-translatable terms concept ID (optional)', default=None) -parser.add_argument('--skip-untranslated-strings', action='store_true', help='Exclude strings which have not been translated from the translation files') -parser.add_argument('--force-allow-unapproved', action='store_true', help='Include unapproved translations in the translation files') -parser.add_argument('--max-workers', type=int, default=10, help='Maximum number of parallel downloads (default: 10, max: 20 due to Crowdin API limits)') -parser.add_argument('-v', '--verbose', action='store_true', help='Enable verbose output') -args = parser.parse_args() - -CROWDIN_API_BASE_URL = "https://api.crowdin.com/api/v2" -CROWDIN_API_TOKEN = args.api_token -CROWDIN_PROJECT_ID = args.project_id -CROWDIN_GLOSSARY_ID = args.glossary_id -CROWDIN_CONCEPT_ID = args.concept_id -DOWNLOAD_DIRECTORY = args.download_directory -SKIP_UNTRANSLATED_STRINGS = args.skip_untranslated_strings -FORCE_ALLOW_UNAPPROVED = args.force_allow_unapproved -VERBOSE = args.verbose -# Crowdin API limit is 20 simultaneous requests per account -MAX_WORKERS = min(args.max_workers, 20) -# Semaphore ensures we don't exceed the concurrent requests limit -api_semaphore = Semaphore(MAX_WORKERS) +import requests + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from generate_shared import print_error, print_progress, print_success, run_main # noqa: E402 +from shared.retry import request_with_retry # noqa: E402 +API = "https://api.crowdin.com/api/v2" +# Crowdin allows 20 simultaneous requests per account. +MAX_CONCURRENT_REQUESTS = 20 REQUEST_TIMEOUT_S = 30 -MAX_RETRIES = 5 -INITIAL_RETRY_DELAY_S = 0.5 - -progress_lock = Lock() -completed_count = 0 -total_count = 0 - - -def make_request_with_retry(method: str, url: str, **kwargs) -> requests.Response: - last_exception = None - - for attempt in range(MAX_RETRIES): - try: - with api_semaphore: - if method.upper() == 'GET': - response = requests.get( - url, timeout=REQUEST_TIMEOUT_S, **kwargs) - elif method.upper() == 'POST': - response = requests.post( - url, timeout=REQUEST_TIMEOUT_S, **kwargs) - else: - raise ValueError(f"Unsupported HTTP method: {method}") - - # Handle rate limiting - if response.status_code == 429: - retry_after = int(response.headers.get( - 'Retry-After', INITIAL_RETRY_DELAY_S * (2 ** attempt))) - if VERBOSE: - print(f"\n{Fore.YELLOW}⚠️ Rate limited, waiting { - retry_after}s before retry...{Style.RESET_ALL}") - time.sleep(retry_after) - continue - - return response - - except requests.exceptions.RequestException as e: - last_exception = e - delay = INITIAL_RETRY_DELAY_S * (2 ** attempt) - if VERBOSE: - print(f"\n{Fore.YELLOW}⚠️ Request failed, retrying in { - delay}s... ({e}){Style.RESET_ALL}") - time.sleep(delay) - - raise last_exception or Exception( - f"Request failed after {MAX_RETRIES} retries") - - -def check_error(response, context=""): - if response.status_code != 200: - error_msg = response.json().get('error', {}).get('message', 'Unknown error') - raise Exception( - f"{context}: {error_msg} (Code: {response.status_code})") - - -def download_file(url: str, output_path: str): - response = requests.get(url, stream=True, timeout=REQUEST_TIMEOUT_S) - response.raise_for_status() - - with open(output_path, 'wb') as f: - for chunk in response.iter_content(chunk_size=8192): - f.write(chunk) - - -def export_and_download_language(language: dict, is_source: bool = False) -> str: - global completed_count - - lang_id = language['id'] - lang_locale = language['locale'] - export_payload = { - "targetLanguageId": lang_id, - "format": "xliff", - "skipUntranslatedStrings": False if is_source else SKIP_UNTRANSLATED_STRINGS, - "exportApprovedOnly": False if is_source else (not FORCE_ALLOW_UNAPPROVED) - } +MAX_ATTEMPTS = 5 - export_response = make_request_with_retry( - 'POST', - f"{CROWDIN_API_BASE_URL}/projects/{CROWDIN_PROJECT_ID}/translations/exports", - headers={"Authorization": f"Bearer {CROWDIN_API_TOKEN}", - "Content-Type": "application/json"}, - data=json.dumps(export_payload) - ) - check_error(export_response, f"Export failed for {lang_locale}") - download_url = export_response.json()['data']['url'] - download_path = os.path.join(DOWNLOAD_DIRECTORY, f"{lang_locale}.xliff") - download_file(download_url, download_path) +class CrowdinError(Exception): + pass - with progress_lock: - completed_count += 1 - print(f"\033[2K{Fore.WHITE}⏳ Downloaded { - completed_count}/{total_count} translations...{Style.RESET_ALL}", end='\r') - return lang_locale +def error_message(response): + """Crowdin's error message, or the start of the body when it is not that envelope.""" + try: + return response.json().get("error", {}).get("message", "Unknown error") + except ValueError: + return response.text[:200] or "Unknown error" + + +class Crowdin: + """One project's API, gated to Crowdin's concurrency limit across the worker threads.""" + + def __init__(self, token, project_id, max_workers): + self.project_id = project_id + self.session = requests.Session() + self.session.headers["Authorization"] = f"Bearer {token}" + # Exports are served from a signed URL on another host, which must not see the token. + self.downloads = requests.Session() + self.gate = Semaphore(min(max_workers, MAX_CONCURRENT_REQUESTS)) + + def request(self, method, path, context, **kwargs): + """One API response's JSON, or CrowdinError naming `context`.""" + with self.gate: + response = request_with_retry(self.session, method, f"{API}/{path}", + attempts=MAX_ATTEMPTS, timeout=REQUEST_TIMEOUT_S, + **kwargs) + if response.status_code != 200: + raise CrowdinError(f"{context}: {error_message(response)} " + f"(Code: {response.status_code})") + return response.json() + + def download(self, url, output_path): + response = request_with_retry(self.downloads, "GET", url, attempts=MAX_ATTEMPTS, + timeout=REQUEST_TIMEOUT_S, stream=True) + response.raise_for_status() + with open(output_path, "wb") as handle: + for chunk in response.iter_content(chunk_size=8192): + handle.write(chunk) + + +class Progress: + def __init__(self, total): + self.total, self.done, self.lock = total, 0, Lock() + + def tick(self): + with self.lock: + self.done += 1 + print_progress(f"Downloaded {self.done}/{self.total} translations...") + + +def export_and_download_language(client, language, directory, is_source, skip_untranslated, + allow_unapproved): + """Export one language and save it as .xliff. Returns the locale.""" + locale = language["locale"] + payload = { + "targetLanguageId": language["id"], + "format": "xliff", + "skipUntranslatedStrings": False if is_source else skip_untranslated, + "exportApprovedOnly": False if is_source else not allow_unapproved, + } + export = client.request("POST", f"projects/{client.project_id}/translations/exports", + f"Export failed for {locale}", json=payload) + client.download(export["data"]["url"], os.path.join(directory, f"{locale}.xliff")) + return locale + + +def parse_args(argv=None): + parser = argparse.ArgumentParser(description="Download translations from Crowdin.") + parser.add_argument("api_token", help="Crowdin API token") + parser.add_argument("project_id", help="Crowdin project ID") + parser.add_argument("download_directory", help="Directory to save the downloaded files") + parser.add_argument("--glossary_id", help="Crowdin glossary ID (optional)") + parser.add_argument("--concept_id", help="Crowdin non-translatable terms concept ID (optional)") + parser.add_argument("--skip-untranslated-strings", action="store_true", + help="Exclude strings which have not been translated") + parser.add_argument("--force-allow-unapproved", action="store_true", + help="Include unapproved translations") + parser.add_argument("--max-workers", type=int, default=10, + help=f"Parallel downloads (default: 10, at most {MAX_CONCURRENT_REQUESTS})") + parser.add_argument("-v", "--verbose", action="store_true", help="Print the API responses") + return parser.parse_args(argv) def main(): - global total_count, completed_count - # Retrieve the list of languages - print(f"{Fore.WHITE}⏳ Retrieving project details...{Style.RESET_ALL}", end='\r') - project_response = make_request_with_retry( - 'GET', - f"{CROWDIN_API_BASE_URL}/projects/{CROWDIN_PROJECT_ID}", - headers={"Authorization": f"Bearer {CROWDIN_API_TOKEN}"} - ) - check_error(project_response, "Failed to retrieve project details") - project_details = project_response.json()['data'] - source_language = project_details['sourceLanguage'] - target_languages = project_details['targetLanguages'] - num_languages = len(target_languages) - print(f"\033[2K{Fore.GREEN}✅ Project details retrieved, found {num_languages} translations{Style.RESET_ALL}") - - if VERBOSE: - print(f"{Fore.BLUE}Response: {json.dumps(project_response.json(), indent=2)}{Style.RESET_ALL}") - - if not os.path.exists(DOWNLOAD_DIRECTORY): - os.makedirs(DOWNLOAD_DIRECTORY) - - project_info_file = os.path.join(DOWNLOAD_DIRECTORY, "_project_info.json") - with open(project_info_file, 'w', encoding='utf-8') as file: - json.dump(project_response.json(), file, indent=2) - - all_languages = [{'language': source_language, 'is_source': True}] - for lang in sorted(target_languages, key=lambda x: x['locale']): - all_languages.append({'language': lang, 'is_source': False}) - - total_count = len(all_languages) - completed_count = 0 - - print(f"{Fore.WHITE}⏳ Downloading {total_count} translations (using {MAX_WORKERS} parallel workers)...{Style.RESET_ALL}") - failed_languages = [] - with ThreadPoolExecutor(max_workers=MAX_WORKERS) as executor: - future_to_lang = { - executor.submit( - export_and_download_language, - item['language'], - item['is_source'] - ): item['language']['locale'] - for item in all_languages - } - - for future in as_completed(future_to_lang): - lang_locale = future_to_lang[future] + args = parse_args() + client = Crowdin(args.api_token, args.project_id, args.max_workers) + + print_progress("Retrieving project details...") + project = client.request("GET", f"projects/{args.project_id}", + "Failed to retrieve project details") + if args.verbose: + print(json.dumps(project, indent=2)) + source_language = project["data"]["sourceLanguage"] + target_languages = sorted(project["data"]["targetLanguages"], key=lambda x: x["locale"]) + print_success(f"Project details retrieved, found {len(target_languages)} translations") + + os.makedirs(args.download_directory, exist_ok=True) + with open(os.path.join(args.download_directory, "_project_info.json"), "w", + encoding="utf-8") as handle: + json.dump(project, handle, indent=2) + + languages = [(source_language, True)] + [(lang, False) for lang in target_languages] + workers = min(args.max_workers, MAX_CONCURRENT_REQUESTS) + print(f"⏳ Downloading {len(languages)} translations (using {workers} parallel workers)...") + progress = Progress(len(languages)) + failed = [] + with ThreadPoolExecutor(max_workers=workers) as executor: + futures = { + executor.submit(export_and_download_language, client, language, + args.download_directory, is_source, args.skip_untranslated_strings, + args.force_allow_unapproved): language["locale"] + for language, is_source in languages} + for future in as_completed(futures): try: future.result() - except Exception as e: - failed_languages.append((lang_locale, str(e))) - if VERBOSE: - print(f"\n{Fore.RED}❌ Failed: {lang_locale} - {e}{Style.RESET_ALL}") - - if failed_languages: - print(f"\033[2K{Fore.RED}❌ {len(failed_languages)} downloads failed:{Style.RESET_ALL}") - for locale, error in failed_languages: + progress.tick() + except Exception as exc: # one language's failure must not hide the others' + failed.append((futures[future], str(exc))) + if failed: + print_error(f"{len(failed)} downloads failed:") + for locale, error in sorted(failed): print(f" - {locale}: {error}") sys.exit(1) - else: - print(f"\033[2K{Fore.GREEN}✅ Downloaded {total_count} translations complete{Style.RESET_ALL}") - - # Download non-translatable terms (if requested) - if CROWDIN_GLOSSARY_ID is not None and CROWDIN_CONCEPT_ID is not None: - print(f"{Fore.WHITE}⏳ Retrieving non-translatable strings...{Style.RESET_ALL}", end='\r') - static_string_response = make_request_with_retry( - 'GET', - f"{CROWDIN_API_BASE_URL}/glossaries/{CROWDIN_GLOSSARY_ID}/terms?conceptId={CROWDIN_CONCEPT_ID}&limit=500", - headers={"Authorization": f"Bearer {CROWDIN_API_TOKEN}"} - ) - check_error(static_string_response, "Failed to retrieve non-translatable strings") - - if VERBOSE: - print(f"{Fore.BLUE}Response: {json.dumps(static_string_response.json(), indent=2)}{Style.RESET_ALL}") - - non_translatable_strings_file = os.path.join(DOWNLOAD_DIRECTORY, "_non_translatable_strings.json") - with open(non_translatable_strings_file, 'w', encoding='utf-8') as file: - json.dump(static_string_response.json(), file, indent=2) - - print(f"\033[2K{Fore.GREEN}✅ Downloading non-translatable complete{Style.RESET_ALL}") + print_success(f"Downloaded {len(languages)} translations complete") + + if args.glossary_id is not None and args.concept_id is not None: + print_progress("Retrieving non-translatable strings...") + terms = client.request( + "GET", f"glossaries/{args.glossary_id}/terms", + "Failed to retrieve non-translatable strings", + params={"conceptId": args.concept_id, "limit": 500}) + if args.verbose: + print(json.dumps(terms, indent=2)) + with open(os.path.join(args.download_directory, "_non_translatable_strings.json"), "w", + encoding="utf-8") as handle: + json.dump(terms, handle, indent=2) + print_success("Downloading non-translatable complete") if __name__ == "__main__": - try: - main() - except KeyboardInterrupt: - print(f"\n{Fore.RED}Process interrupted by user{Style.RESET_ALL}") - sys.exit(0) - except Exception as e: - print(f"\033[2K{Fore.RED}❌ An error occurred: {e}{Style.RESET_ALL}") - sys.exit(1) + run_main(main) diff --git a/crowdin/test_download_translations.py b/crowdin/test_download_translations.py new file mode 100644 index 0000000..3a703c0 --- /dev/null +++ b/crowdin/test_download_translations.py @@ -0,0 +1,111 @@ +"""Run with: cd crowdin && python -m unittest discover""" +import os +import sys +import tempfile +import unittest + +import requests + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import download_translations_from_crowdin as download # noqa: E402 +from shared.testing import FakeResponse, FakeSession, NoSleep, NonJsonResponse # noqa: E402 + + +class StreamResponse(FakeResponse): + def __init__(self, body): + super().__init__({}) + self.body = body + + def iter_content(self, chunk_size): + yield self.body + + +def client(api_responses=(), download_responses=()): + crowdin = download.Crowdin("secret", "618696", max_workers=4) + crowdin.session = FakeSession(api_responses) + crowdin.downloads = FakeSession(download_responses) + return crowdin + + +class TestRequests(unittest.TestCase): + def test_the_token_goes_to_the_api_and_never_to_the_export_host(self): + crowdin = download.Crowdin("secret", "618696", max_workers=4) + self.assertEqual(crowdin.session.headers["Authorization"], "Bearer secret") + self.assertNotIn("Authorization", crowdin.downloads.headers) + + def test_a_server_error_is_retried(self): + crowdin = client([FakeResponse({}, status_code=503), FakeResponse({"data": {"id": 1}})]) + with NoSleep(): + self.assertEqual(crowdin.request("GET", "projects/618696", "project")["data"], {"id": 1}) + self.assertEqual(len(crowdin.session.calls), 2) + self.assertEqual(crowdin.session.calls[0][1], f"{download.API}/projects/618696") + + def test_a_client_error_names_the_step_and_crowdins_message(self): + crowdin = client([FakeResponse({"error": {"message": "Token invalid"}}, status_code=401)]) + with self.assertRaises(download.CrowdinError) as caught: + crowdin.request("GET", "projects/618696", "Failed to retrieve project details") + self.assertEqual(str(caught.exception), + "Failed to retrieve project details: Token invalid (Code: 401)") + + def test_a_non_json_error_page_still_reads_as_an_error(self): + page = NonJsonResponse() + page.status_code = 404 + with self.assertRaises(download.CrowdinError) as caught: + client([page]).request("GET", "projects/618696", "project") + self.assertIn("maintenance", str(caught.exception)) + self.assertIn("404", str(caught.exception)) + + +class TestExport(unittest.TestCase): + def test_a_language_is_exported_then_saved_under_its_locale(self): + crowdin = client([FakeResponse({"data": {"url": "https://storage.example/x.xliff"}})], + [StreamResponse(b"")]) + with tempfile.TemporaryDirectory() as directory: + locale = download.export_and_download_language( + crowdin, {"id": "de", "locale": "de-DE"}, directory, is_source=False, + skip_untranslated=True, allow_unapproved=False) + with open(os.path.join(directory, "de-DE.xliff"), "rb") as handle: + self.assertEqual(handle.read(), b"") + self.assertEqual(locale, "de-DE") + method, url, kwargs = crowdin.session.calls[0] + self.assertEqual((method, url), ("POST", f"{download.API}/projects/618696/translations/exports")) + self.assertEqual(kwargs["json"], {"targetLanguageId": "de", "format": "xliff", + "skipUntranslatedStrings": True, + "exportApprovedOnly": True}) + self.assertEqual(crowdin.downloads.calls[0][1], "https://storage.example/x.xliff") + + def test_the_source_language_is_always_exported_whole(self): + crowdin = client([FakeResponse({"data": {"url": "https://storage.example/en.xliff"}})], + [StreamResponse(b"")]) + with tempfile.TemporaryDirectory() as directory: + download.export_and_download_language( + crowdin, {"id": "en", "locale": "en"}, directory, is_source=True, + skip_untranslated=True, allow_unapproved=False) + payload = crowdin.session.calls[0][2]["json"] + self.assertFalse(payload["skipUntranslatedStrings"]) + self.assertFalse(payload["exportApprovedOnly"]) + + def test_a_rejected_download_raises(self): + crowdin = client([FakeResponse({"data": {"url": "https://storage.example/x.xliff"}})], + [FakeResponse({}, status_code=403)]) + with tempfile.TemporaryDirectory() as directory, self.assertRaises(requests.HTTPError): + download.export_and_download_language( + crowdin, {"id": "de", "locale": "de-DE"}, directory, is_source=False, + skip_untranslated=False, allow_unapproved=True) + + +class TestArguments(unittest.TestCase): + def test_the_workflow_invocation_still_parses(self): + args = download.parse_args(["tok", "618696", "raw", "--glossary_id", "407522", + "--concept_id", "36", "--skip-untranslated-strings"]) + self.assertEqual((args.api_token, args.project_id, args.download_directory), + ("tok", "618696", "raw")) + self.assertEqual((args.glossary_id, args.concept_id), ("407522", "36")) + self.assertTrue(args.skip_untranslated_strings) + self.assertFalse(args.force_allow_unapproved) + self.assertEqual(args.max_workers, 10) + + +if __name__ == "__main__": + unittest.main() From 475dac1e806584e5280737d065f90f44f69142ab Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 24 Sep 2026 16:52:01 +1000 Subject: [PATCH 5/5] ci: run every unittest suite on pull requests Nothing ran them before. One job per directory, because two of the requirements files pin requests differently. sogs_moderation runs on the system interpreter with a venv that can see it, since session_util ships as a deb built against that interpreter rather than as a wheel. --- .github/workflows/tests.yml | 71 +++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..af9570c --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,71 @@ +name: Tests + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + unittest: + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + include: + - suite: shared + requirements: github_prs/requirements.txt + - suite: github_prs + requirements: github_prs/requirements.txt + - suite: zendesk_triage + requirements: zendesk_triage/requirements.txt zendesk_triage/requirements-dev.txt + - suite: deploy + requirements: github_prs/requirements.txt + - suite: crowdin + requirements: crowdin/requirements.txt + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v6 + with: + python-version: "3.12" + cache: pip + cache-dependency-path: "**/requirements*.txt" + + - name: Install dependencies + run: | + for file in ${{ matrix.requirements }}; do + pip install -r "$file" + done + + - name: Run ${{ matrix.suite }} tests + working-directory: ${{ matrix.suite }} + run: python -m unittest discover -v + + sogs_moderation: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + + # session_util is published as a deb built against the system interpreter, so + # this suite runs on that interpreter with a venv that can see it. + - name: Install python3-session-util + run: | + sudo curl -so /usr/share/keyrings/session-foundation.gpg https://deb.session.foundation/pub.gpg + printf 'Types: deb\nURIs: https://deb.session.foundation\nSuites: %s\nComponents: main\nSigned-By: /usr/share/keyrings/session-foundation.gpg\n' \ + "$(lsb_release -sc)" | sudo tee /etc/apt/sources.list.d/session.sources + sudo apt-get update + sudo apt-get install -y python3-session-util + + - name: Install dependencies + run: | + python3 -m venv --system-site-packages .venv + .venv/bin/pip install -r sogs_moderation/requirements.txt + + - name: Run sogs_moderation tests + working-directory: sogs_moderation + run: ../.venv/bin/python -m unittest discover -v