From b97edd5184aad53ba37af2f5ee50b720173874f9 Mon Sep 17 00:00:00 2001 From: Marius Mutu Date: Sun, 23 Aug 2026 14:52:44 +0000 Subject: [PATCH] fix(fallback): turnul nu se mai pierde cand Claude e la limita MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detectia de rate limit functiona, dar `_local_fallback_reply` putea intoarce None din patru locuri fara nicio linie de log — userul primea `Claude CLI error (exit 1): You've hit your session limit` pe Discord in loc de raspuns, si nu se putea afla ulterior din log care branch a picat. - daca runda de unelte iese goala, turnul se reface fara unelte in loc sa fie abandonat - fiecare return None ramas e logat - daca nici modelul local nu raspunde, mesajul e „Claude e la limita…" plus ora de reset, nu eroarea bruta a CLI-ului - cron: job-urile heartbeat* tac la limita (last_status: rate_limited); celelalte trimit o singura linie scurta, ca sa se vada rularea sarita - is_rate_limit_error / rate_limit_detail mutate in claude_session.py, folosite si de scheduler (care nu poate importa router-ul) Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DzZAMzbyQbzFdsrVoXijBg --- CLAUDE.md | 3 +++ src/claude_session.py | 22 ++++++++++++++++++++ src/router.py | 39 +++++++++++++++++++++++++---------- src/scheduler.py | 24 +++++++++++++++++++--- tests/test_local_fallback.py | 27 ++++++++++++++++++++++++ tests/test_router.py | 9 +++++--- tests/test_scheduler.py | 40 ++++++++++++++++++++++++++++++++++++ 7 files changed, 147 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 57b7c51..0329456 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -113,6 +113,9 @@ source .venv/bin/activate && pip install -r requirements.txt - **Web** (`src/web_search.py`): DuckDuckGo Lite, fără API key. Atenție la parser — DDG emite `class='...'` cu ghilimele simple. - **Două treceri.** Prima decide uneltele (cu `tools`, temperatură 0, fără few-shot). Dacă nu iese niciun tool call, turnul se reface **fără unelte** — prezența definițiilor degradează răspunsurile simple: cu ele „cat fac 128/4?” întoarce „Nu știu ce înseamnă 128/4”, fără ele „128 / 4 = 32”. Few-shot se adaugă la a doua trecere **doar pentru cereri creative** (`_is_creative_request` — glumă/banc/poveste/poezie), unde modelul altfel deflectează („O glumă bună!”); pe turnuri factuale strică aritmetica (17*23 → 471). În apelul de decizie few-shot scade selecția de la 17/17 la 15/17, iar temperatura 0.6 tot la 15/17 — de aceea rămân separate și la 0. - **Prefix:** banner-ul „Claude e la limită" apare doar pe calea de rate limit. Prin `/f` userul a ales modelul local deliberat (`manual=True`) și nu se anunță nicio limită. +- **Turnul nu se pierde niciodată.** Fiecare `return None` din `_local_fallback_reply` e logat — o întoarcere tăcută înseamnă că userul primește eroarea brută de rate limit de la Claude în loc de răspuns (exact ce s-a întâmplat pe 2026-08-23: detecția a mers, fallback-ul a întors None fără nicio linie de log, imposibil de diagnosticat post-factum). Dacă runda de unelte iese goală, turnul se reface **fără unelte** în loc să se abandoneze. Dacă nici modelul local nu răspunde, mesajul către user e „Claude e la limită…" + ora de reset, nu `Claude CLI error (exit 1): …`. +- **Detecția e partajată:** `is_rate_limit_error` / `rate_limit_detail` stau în `src/claude_session.py` (nu în router) pentru că le folosește și `src/scheduler.py`, care nu are voie să importe router-ul. +- **Cron la limită** (`src/scheduler.py`): job-urile `heartbeat*` tac complet (`last_status: rate_limited`, nimic pe canal) — o limită nu e acționabilă și s-ar repeta la fiecare rulare până la reset. Celelalte job-uri trimit o singură linie scurtă, ca să se vadă că rularea a fost sărită. - **Comenzi:** `/f ` conversează, `/f` arată starea, `/f reset` golește istoricul (`/testfallback` rămâne alias). `/masini [nume]` e disponibilă și ca fast command normală. **roa2web — re-autentificare 2FA din chat.** Când tokenul de dispozitiv expiră, orice comandă financiară (`/sold`, `/facturi`, `/trezorerie`) întoarce „Necesar cod OTP nou, trimis pe m***@…”. Codul se trimite înapoi cu **`/otp `** (Discord: slash command ephemeral, restul: text). Adresa de email e ținută în keyring ca `roa2web_email` și se reține la prima folosire — `/otp ` o suprascrie. Mesajul de eroare din `tools/roa2web_client.py` trimitea înainte la `verify_2fa(code, email)`, un apel Python imposibil de rulat din Discord, adică exact acolo unde apare eroarea. `otp` **nu** e în registrul de unelte al fallback-ului: acela e strict read-only, iar autentificarea schimbă stare. diff --git a/src/claude_session.py b/src/claude_session.py index 03f848a..5c0c22d 100644 --- a/src/claude_session.py +++ b/src/claude_session.py @@ -9,6 +9,7 @@ tracked in sessions/active.json. import json import logging import os +import re import shutil import subprocess import tempfile @@ -37,6 +38,27 @@ DEFAULT_TIMEOUT = 300 # seconds CLAUDE_BIN = os.environ.get("CLAUDE_BIN", "claude") +# Claude CLI rate-limit errors look like: +# "Claude CLI error (exit 1): You've hit your session limit · resets 10:50am (UTC)" +# (same phrasing for the total subscription limit, not just the per-session one). +# Lives here rather than in router.py because the scheduler needs it too and +# must not import the router. +RATE_LIMIT_RE = re.compile(r"hit your .*limit", re.IGNORECASE) + + +def is_rate_limit_error(err: Exception | str) -> bool: + """True if *err* is the Claude CLI refusing because a limit was reached.""" + return bool(RATE_LIMIT_RE.search(str(err))) + + +def rate_limit_detail(err: Exception | str) -> str: + """The bare limit notice, without the `Claude CLI error (exit 1):` wrapper.""" + text = str(err) + if text.startswith("Claude CLI error (") and "):" in text: + text = text.split("):", 1)[1] + return text.strip() + + # --------------------------------------------------------------------------- # Per-channel mutex for send_message # --------------------------------------------------------------------------- diff --git a/src/router.py b/src/router.py index 03f1522..88eeb1d 100644 --- a/src/router.py +++ b/src/router.py @@ -20,6 +20,9 @@ from src.claude_session import ( get_active_session, list_sessions, set_session_model, + is_rate_limit_error as _is_rate_limit_error, + rate_limit_detail as _rate_limit_detail, + RATE_LIMIT_RE as _RATE_LIMIT_RE, VALID_MODELS, ) from src.jsonlock import read_locked, write_locked @@ -89,11 +92,6 @@ def _get_config() -> Config: return _config -# Claude CLI rate-limit errors look like: -# "Claude CLI error (exit 1): You've hit your session limit · resets 10:50am (UTC)" -# (also seen for the total subscription limit, not just per-session — same phrasing). -_RATE_LIMIT_RE = re.compile(r"hit your .*limit", re.IGNORECASE) - _LOCAL_FALLBACK_SYSTEM_PROMPT = ( "Ești Echo, asistentul personal al lui Marius. Răspunde direct și la " "obiect, în limba în care a fost scris mesajul. Când ți se cere ceva — o " @@ -248,10 +246,6 @@ def _forced_tool(text: str) -> tuple[str, dict] | None: return None -def _is_rate_limit_error(err: Exception) -> bool: - return bool(_RATE_LIMIT_RE.search(str(err))) - - def _call_local_llm( url: str, messages: list[dict], @@ -301,9 +295,11 @@ def _local_fallback_reply( prefix = _LOCAL_MANUAL_PREFIX if manual else _LOCAL_FALLBACK_PREFIX cfg = _get_config().get("local_fallback", {}) or {} if not cfg.get("enabled"): + log.warning("Local fallback requested but local_fallback.enabled is false") return None url = cfg.get("url") if not url: + log.error("Local fallback enabled but local_fallback.url is missing") return None if channel_id: @@ -358,6 +354,7 @@ def _local_fallback_reply( log.error("Local fallback LLM failed: %s", e) return None if message is None: + log.error("Local fallback returned no message object (url=%s)", url) return None answer = (message.get("content") or "").strip() @@ -365,8 +362,19 @@ def _local_fallback_reply( if calls: answer = _run_fallback_tools(url, messages, message, calls, local_fallback_tools) - if answer is None: - return None + if not answer: + # The tool round yielded nothing usable (synthesis came back + # empty). Returning None here dropped the whole turn and the user + # saw the raw Claude rate-limit error instead of a reply — answer + # conversationally rather than giving up. + log.warning( + "Fallback tool round produced no answer for %r — retrying tool-free", + text[:60], + ) + answer = _conversational_reply( + url, system_prompt, history, text, + fewshot=_is_creative_request(text), + ) else: # No tool was called, so this is a plain answer — and carrying the tool # definitions degrades those: with them "cat fac 128/4?" comes back as @@ -380,6 +388,7 @@ def _local_fallback_reply( ) or answer if not answer: + log.error("Local fallback produced an empty answer for %r", text[:60]) return None if channel_id: fallback_history.append(channel_id, text, answer) @@ -605,6 +614,14 @@ def route_message( if fallback is not None: _set_last_response(channel_id, fallback) return fallback, False + log.error( + "Local fallback unavailable for channel %s — surfacing the limit notice", + channel_id, + ) + return ( + "⚠️ Claude e la limită, iar modelul local nu a răspuns.\n" + f"{_rate_limit_detail(e)}" + ), False return f"Error: {e}", False diff --git a/src/scheduler.py b/src/scheduler.py index ad62bfc..051f5e1 100644 --- a/src/scheduler.py +++ b/src/scheduler.py @@ -21,6 +21,8 @@ from apscheduler.schedulers.asyncio import AsyncIOScheduler from apscheduler.triggers.cron import CronTrigger from src.claude_session import ( + is_rate_limit_error, + rate_limit_detail as _rate_limit_detail, CLAUDE_BIN, PROJECT_ROOT, VALID_MODELS, @@ -440,9 +442,25 @@ class Scheduler: logger.error("Job '%s' timed out", name) except (RuntimeError, json.JSONDecodeError) as exc: - job["last_status"] = "error" - result_text = f"[cron:{name}] Error: {exc}" - logger.error("Job '%s' failed: %s", name, exc) + if is_rate_limit_error(exc): + # A rate limit is not a job failure to debug — dumping the raw + # `Claude CLI error (exit 1): ...` into the channel on every + # run until the window resets is pure noise. Heartbeats stay + # silent (they only speak when something is actionable); + # other jobs get one short line so a skipped run is visible. + job["last_status"] = "rate_limited" + logger.warning("Job '%s' skipped — Claude at limit: %s", name, exc) + if name.startswith("heartbeat"): + result_text = "" + else: + result_text = ( + f"[cron:{name}] Claude e la limită — job sărit. " + f"{_rate_limit_detail(exc)}" + ) + else: + job["last_status"] = "error" + result_text = f"[cron:{name}] Error: {exc}" + logger.error("Job '%s' failed: %s", name, exc) except Exception as exc: job["last_status"] = "error" diff --git a/tests/test_local_fallback.py b/tests/test_local_fallback.py index d3590c2..b662e2a 100644 --- a/tests/test_local_fallback.py +++ b/tests/test_local_fallback.py @@ -596,3 +596,30 @@ class TestOtpCommand: def test_otp_is_not_a_fallback_tool(self): """The fallback registry is read-only; auth is state-changing.""" assert "otp" not in lft.TOOLS + + +# --- The tool round must never swallow the turn --- + +class TestFallbackNeverDropsTurn: + """A rate-limit rescue that returns None puts the raw Claude error on + Discord — exactly the failure seen on 2026-08-23. If the tool round comes + back empty, retry tool-free instead of giving up.""" + + @patch("src.router._get_config") + def test_empty_tool_round_retries_tool_free(self, mock_get_config): + from src.router import _local_fallback_reply + + mock_cfg = MagicMock() + mock_cfg.get.return_value = {"enabled": True, "url": "http://x/v1/chat/completions"} + mock_get_config.return_value = mock_cfg + + with patch("src.router._call_local_llm", + return_value={"content": "", "tool_calls": [_call("cauta_web")]}), \ + patch("src.router._run_fallback_tools", return_value=None), \ + patch("src.router._conversational_reply", return_value="răspuns direct") as conv: + reply = _local_fallback_reply("ceva") + + assert reply is not None + assert "răspuns direct" in reply + conv.assert_called_once() + diff --git a/tests/test_router.py b/tests/test_router.py index 83b03cb..b2189b0 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -242,7 +242,7 @@ class TestRegularMessage: @patch("src.router._get_channel_config") @patch("src.router._get_config") @patch("src.router.send_message") - def test_rate_limit_fallback_unavailable_surfaces_original_error( + def test_rate_limit_fallback_unavailable_surfaces_limit_notice( self, mock_send, mock_get_config, mock_chan_cfg, mock_fallback, ): mock_send.side_effect = RuntimeError( @@ -255,8 +255,11 @@ class TestRegularMessage: mock_fallback.return_value = None # local fallback itself unreachable response, is_cmd = route_message("ch-1", "user-1", "hello") - assert "Error:" in response - assert "hit your session limit" in response + # The reset time still has to reach the user; the `Claude CLI error + # (exit 1):` wrapper is noise and must not. + assert "resets 10:50am (UTC)" in response + assert "Claude e la limită" in response + assert "Claude CLI error" not in response assert is_cmd is False @patch("src.router._get_channel_config") diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 3090633..540b73a 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -362,6 +362,46 @@ class TestRunJob: assert "Error" in result assert sched._jobs[0]["last_status"] == "error" + @pytest.mark.asyncio + async def test_rate_limited_heartbeat_stays_silent(self, sched, tmp_jobs, callback): + """A limit isn't actionable and recurs every run until it resets — + heartbeats must not narrate it into the channel.""" + sched.add_job("heartbeat-4h", "0 * * * *", "ch", "prompt") + + mock_proc = MagicMock() + mock_proc.returncode = 1 + mock_proc.stdout = "" + mock_proc.stderr = "You've hit your session limit \u00b7 resets 10:50am (UTC)" + + with patch("src.scheduler.build_system_prompt", return_value="sys"), \ + patch("subprocess.run", return_value=mock_proc): + result = await sched.run_job("heartbeat-4h") + + assert result == "" + assert sched._jobs[0]["last_status"] == "rate_limited" + callback.assert_not_awaited() + + @pytest.mark.asyncio + async def test_rate_limited_other_job_reports_one_short_line( + self, sched, tmp_jobs, callback + ): + sched.add_job("evening-report", "0 * * * *", "ch", "prompt") + + mock_proc = MagicMock() + mock_proc.returncode = 1 + mock_proc.stdout = "" + mock_proc.stderr = "You've hit your session limit \u00b7 resets 10:50am (UTC)" + + with patch("src.scheduler.build_system_prompt", return_value="sys"), \ + patch("subprocess.run", return_value=mock_proc): + result = await sched.run_job("evening-report") + + assert "Claude e la limită" in result + assert "resets 10:50am (UTC)" in result + assert "Claude CLI error" not in result + assert sched._jobs[0]["last_status"] == "rate_limited" + callback.assert_awaited_once() + @pytest.mark.asyncio async def test_execute_job_invalid_json(self, sched, tmp_jobs, callback): sched.add_job("json-err", "0 * * * *", "ch", "prompt")