fix(tracker): no duplicate Telegram messages on not-modified/transient edits

edit_telegram now returns a distinguishable outcome (ok|not_modified|gone| failed) instead of a bare bool. update_task_tracker only sends a NEW message when the original is truly gone; not_modified and transient failures no longer spawn duplicate trackers or orphan the live one. render_task_tracker shows "попытка N" on an actively re-run stage (>=2 agent runs) so the text changes between review<->development cycles. Finished (✅) lines are unchanged. Tests: edit_telegram classification (ok/not_modified/gone/failed via mocked httpx), update_task_tracker (not_modified/failed -> no send, gone -> send+id), render attempt marker.
2026-06-04 13:20:40 +03:00
parent 3e5c74ce4f
commit ec9aa74492
2 changed files with 285 additions and 22 deletions
--- a/tests/test_telegram_tracker.py
+++ b/tests/test_telegram_tracker.py
@@ -249,7 +249,7 @@ def test_second_call_edits_existing_message(monkeypatch):

    edited = {}
    monkeypatch.setattr(N, "edit_telegram",
-                        lambda mid, text: edited.update(mid=mid) or True)
+                        lambda mid, text: edited.update(mid=mid) or N.EDIT_OK)
    monkeypatch.setattr(N, "send_telegram",
                        lambda *a, **k: (_ for _ in ()).throw(AssertionError("should not send when edit succeeds")))

@@ -257,20 +257,196 @@ def test_second_call_edits_existing_message(monkeypatch):
    assert edited["mid"] == 777


-def test_fallback_to_new_message_when_edit_fails(monkeypatch):
+def test_fallback_to_new_message_when_edit_gone(monkeypatch):
+    """edit returns 'gone' (message deleted/too old) -> send NEW + update id."""
    tid = _mk_task(stage="development")
    _mk_run(tid, "analyst", "2026-06-04 09:00:00", "2026-06-04 09:10:00",
            in_tok=10, out_tok=5, cost=0.1)
    from src.db import set_tracker_message_id, get_tracker_message_id
    set_tracker_message_id(tid, 100)

-    monkeypatch.setattr(N, "edit_telegram", lambda mid, text: False)  # edit fails
+    monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_GONE)
    monkeypatch.setattr(N, "send_telegram", lambda text, disable_notification=False: 200)

    N.update_task_tracker(tid)
    assert get_tracker_message_id(tid) == 200  # id updated to the new message


+def test_not_modified_does_not_send_new_message(monkeypatch):
+    """edit returns 'not_modified' -> NO new message, id unchanged (no dupe)."""
+    tid = _mk_task(stage="development")
+    _mk_run(tid, "analyst", "2026-06-04 09:00:00", "2026-06-04 09:10:00",
+            in_tok=10, out_tok=5, cost=0.1)
+    from src.db import set_tracker_message_id, get_tracker_message_id
+    set_tracker_message_id(tid, 100)
+
+    monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_NOT_MODIFIED)
+    monkeypatch.setattr(N, "send_telegram",
+                        lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not send on not_modified")))
+
+    N.update_task_tracker(tid)
+    assert get_tracker_message_id(tid) == 100  # unchanged, no duplicate
+
+
+def test_transient_edit_failure_does_not_send_new_message(monkeypatch):
+    """edit returns 'failed' (network/timeout/5xx) -> NO new message (no dupe)."""
+    tid = _mk_task(stage="development")
+    _mk_run(tid, "analyst", "2026-06-04 09:00:00", "2026-06-04 09:10:00",
+            in_tok=10, out_tok=5, cost=0.1)
+    from src.db import set_tracker_message_id, get_tracker_message_id
+    set_tracker_message_id(tid, 100)
+
+    monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_FAILED)
+    monkeypatch.setattr(N, "send_telegram",
+                        lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not send on transient failure")))
+
+    N.update_task_tracker(tid)
+    assert get_tracker_message_id(tid) == 100  # unchanged, no duplicate
+
+
+# --------------------------------------------------------------------------- #
+# edit_telegram outcome classification (httpx mocked)
+# --------------------------------------------------------------------------- #
+def _edit_resp(ok, description=None):
+    resp = MagicMock()
+    body = {"ok": ok}
+    if description is not None:
+        body["description"] = description
+    resp.json.return_value = body
+    return resp
+
+
+def _patch_tg_creds(monkeypatch):
+    monkeypatch.setattr(N._get_settings(), "telegram_bot_token", "T", raising=False)
+    monkeypatch.setattr(N._get_settings(), "telegram_chat_id", "C", raising=False)
+
+
+def test_edit_telegram_ok(monkeypatch):
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(True)
+        assert N.edit_telegram(1, "x") == N.EDIT_OK
+
+
+def test_edit_telegram_not_modified_is_success(monkeypatch):
+    # 400 "message is not modified" -> success, not gone, no duplicate
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(
+            False, "Bad Request: message is not modified: ...")
+        assert N.edit_telegram(1, "x") == N.EDIT_NOT_MODIFIED
+
+
+def test_edit_telegram_exactly_the_same_is_not_modified(monkeypatch):
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(
+            False, "Bad Request: specified new message content and reply markup "
+                   "are exactly the same")
+        assert N.edit_telegram(1, "x") == N.EDIT_NOT_MODIFIED
+
+
+def test_edit_telegram_message_not_found_is_gone(monkeypatch):
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(
+            False, "Bad Request: message to edit not found")
+        assert N.edit_telegram(1, "x") == N.EDIT_GONE
+
+
+def test_edit_telegram_cant_be_edited_is_gone(monkeypatch):
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(
+            False, "Bad Request: message can't be edited")
+        assert N.edit_telegram(1, "x") == N.EDIT_GONE
+
+
+def test_edit_telegram_unknown_400_is_failed(monkeypatch):
+    # unknown 400 -> failed (NOT gone) -> caller won't duplicate
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(
+            False, "Bad Request: some other unexpected error")
+        assert N.edit_telegram(1, "x") == N.EDIT_FAILED
+
+
+def test_edit_telegram_timeout_is_failed(monkeypatch):
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.side_effect = Exception("read timeout")
+        assert N.edit_telegram(1, "x") == N.EDIT_FAILED
+
+
+def test_edit_telegram_5xx_is_failed(monkeypatch):
+    # Telegram 5xx still returns ok:false w/o gone/not_modified markers
+    _patch_tg_creds(monkeypatch)
+    with patch("src.notifications.httpx") as hx:
+        hx.post.return_value = _edit_resp(False, "Internal Server Error")
+        assert N.edit_telegram(1, "x") == N.EDIT_FAILED
+
+
+# --------------------------------------------------------------------------- #
+# render: repeated stage attempt shows "попытка N"
+# --------------------------------------------------------------------------- #
+_POPYTKA = "\u043f\u043e\u043f\u044b\u0442\u043a\u0430"  # popytka
+
+
+def test_render_active_stage_shows_attempt_on_second_run():
+    # Two reviewer runs while in review -> active line shows attempt 2.
+    tid = _mk_task(stage="review")
+    _mk_run(tid, "analyst", "2026-06-04 09:00:00", "2026-06-04 09:10:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    _mk_run(tid, "developer", "2026-06-04 09:10:00", "2026-06-04 09:20:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    # First review run finished (sent back to dev), second review run active.
+    _mk_run(tid, "reviewer", "2026-06-04 09:20:00", "2026-06-04 09:25:00",
+            in_tok=10, out_tok=5, cost=0.1, model="vibecode/claude-sonnet-4.6",
+            exit_code=0)
+    _mk_run(tid, "reviewer", "2026-06-04 09:30:00", None,
+            in_tok=0, out_tok=0, exit_code=None)
+
+    text = N.render_task_tracker(tid)
+    active = [l for l in text.splitlines()
+              if l.startswith("\U0001f504") and "Review" in l][0]
+    assert _POPYTKA in active
+    assert "2" in active
+    assert "\u0438\u0434\u0451\u0442" in active
+
+
+def test_render_active_stage_no_attempt_on_first_run():
+    # Single reviewer run -> active line has NO attempt marker.
+    tid = _mk_task(stage="review")
+    _mk_run(tid, "analyst", "2026-06-04 09:00:00", "2026-06-04 09:10:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    _mk_run(tid, "developer", "2026-06-04 09:10:00", "2026-06-04 09:20:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    _mk_run(tid, "reviewer", "2026-06-04 09:20:00", None,
+            in_tok=0, out_tok=0, exit_code=None)
+
+    text = N.render_task_tracker(tid)
+    active = [l for l in text.splitlines()
+              if l.startswith("\U0001f504") and "Review" in l][0]
+    assert _POPYTKA not in active
+    assert "\u0438\u0434\u0451\u0442" in active
+
+
+def test_render_finished_lines_unaffected_by_attempt_logic():
+    # Completed (checkmark) lines never carry an attempt marker.
+    tid = _mk_task(stage="review")
+    _mk_run(tid, "analyst", "2026-06-04 09:00:00", "2026-06-04 09:10:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    # developer ran twice (retry) but is a FINISHED stage now.
+    _mk_run(tid, "developer", "2026-06-04 09:10:00", "2026-06-04 09:15:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    _mk_run(tid, "developer", "2026-06-04 09:16:00", "2026-06-04 09:20:00",
+            in_tok=10, out_tok=5, cost=0.1, model="tokenator/claude-opus-4-8")
+    text = N.render_task_tracker(tid)
+    for l in text.splitlines():
+        if l.startswith("\u2705"):
+            assert _POPYTKA not in l
+
+
 # --------------------------------------------------------------------------- #
 # which alerts are SEPARATE vs tracker-only
 # --------------------------------------------------------------------------- #