From 6f4394113b728f556c358f1339656bf2cfbbe92a Mon Sep 17 00:00:00 2001 From: Hermes Team Date: Tue, 1 Sep 2026 14:32:35 +0700 Subject: [PATCH] =?UTF-8?q?review(a56):=20=D1=81=D0=B6=D0=B0=D1=82=D0=B8?= =?UTF-8?q?=D0=B5=20=D0=BA=D0=BE=D0=BD=D1=82=D0=B5=D0=BA=D1=81=D1=82=D0=B0?= =?UTF-8?q?=20=D0=BF=D1=80=D0=B8=D0=BD=D1=8F=D1=82=D0=BE=20=D1=81=20=D0=B8?= =?UTF-8?q?=D1=81=D0=BF=D1=80=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=D0=B8=D1=8F?= =?UTF-8?q?=D0=BC=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Проверено исполнением на сервере владельца, а не по отчёту. Работает. Сжатие вызывается в настоящем пути запроса (local_adapter.py:168) — разрыв, ради которого писалось задание, закрыт. Замер на живом компрессоре: 5667 токенов на входе, 624 на выходе, 0,11x, экономия 5043 токена за 127,6 с. Исправлено четыре дефекта. 1. /props и /tokenize запрашивались по адресу с суффиксом /v1. У llama.cpp они живут в корне: измерено, /props → 200, /v1/props → 404, то же с /tokenize. Адаптер передаёт супервизору именно адрес с /v1, поэтому счёт токенов молча падал на посимвольную оценку, и порог сжатия считался от выдуманного числа. Адрес нормализуется. 2. Заявленные «сто процентов сохранения фактов» модель не даёт. Замер: 37 из 38, 97,4%, потерян 001cd1f. Сто процентов получались дописыванием недостающих фактов списком — механизм верный, но измеренное число подменялось исправленным, а «(100%)» было вписано в сообщение текстом. Теперь полнота самой модели сохраняется отдельно и показывается владельцу: иначе ухудшение модели осталось бы незамеченным. 3. Итог сжатия считался посимвольно (длина / 3.5) и подавался рядом с настоящим числом токенов на входе. Теперь пересчитывается токенизатором сервера, а недоступность токенизатора помечается признаком оценки. 4. Порт 8082 был зашит запасным адресом в двух местах вопреки прямому запрету в задании. Профиль без адреса теперь даёт состояние «не настроен» с причиной, а не молчаливый стук в 8082. Не выполнено исполнителем: проверка на живом сервере (P0-6.1). Тест на неё пропускается как негерметичный, замеры выше сделал ревьюер. 649 passed, 2 skipped; ruff чисто; релизный гейт пройден. Co-Authored-By: Claude Opus 5 --- .../router/context_compressor.py | 46 +++- .../router/local_supervisor.py | 34 ++- tests/test_a56_review_findings.py | 233 ++++++++++++++++++ 3 files changed, 303 insertions(+), 10 deletions(-) create mode 100644 tests/test_a56_review_findings.py diff --git a/src/antigravity_provider/router/context_compressor.py b/src/antigravity_provider/router/context_compressor.py index 361d4af..83fa7a1 100644 --- a/src/antigravity_provider/router/context_compressor.py +++ b/src/antigravity_provider/router/context_compressor.py @@ -76,6 +76,11 @@ class CompressionOutcome: facts_total: int = 0 facts_retained: int = 0 retention_percent: float = 100.0 + # Итог считается посимвольно, если токенизатор сервера недоступен. + tokens_after_is_estimate: bool = True + # Полнота, которую дала сама модель, до дописывания недостающих фактов. + model_retention_percent: float = 100.0 + facts_added_by_safeguard: List[str] = field(default_factory=list) retained_facts: List[str] = field(default_factory=list) missing_facts: List[str] = field(default_factory=list) model_name: str = "" @@ -189,14 +194,16 @@ class ContextCompressor: getattr(profile_config, "custom_base_url", None) or (profile_config.auth_config.get("base_url") if hasattr(profile_config, "auth_config") and isinstance(profile_config.auth_config, dict) else None) or os.environ.get("LOCAL_COMPRESSOR_BASE_URL") - or "http://127.0.0.1:8082/v1" + or "" ) model_name = profile_config.preferred_models[0] if getattr(profile_config, "preferred_models", None) else "default" token = profile_config.auth_config.get("api_key") or profile_config.auth_config.get("token") if hasattr(profile_config, "auth_config") and isinstance(profile_config.auth_config, dict) else "" return str(base_url).rstrip("/"), str(model_name), str(token or "") - # Fallback to environment or standard compressor port - env_url = os.environ.get("LOCAL_COMPRESSOR_BASE_URL", "http://127.0.0.1:8082/v1") + # Порт 8082 сегодняшний, завтра другой: зашивать его нельзя. Нет + # адреса — значит компрессор не настроен, и это отдельное состояние, + # а не повод молча постучаться в 8082. + env_url = os.environ.get("LOCAL_COMPRESSOR_BASE_URL", "") return env_url.rstrip("/"), "default", "" def compress_messages_if_needed( @@ -216,10 +223,14 @@ class ContextCompressor: Guarantees 100% factual retention. """ # P0-1: If compressor profile is not configured - if not compressor_profile and not os.environ.get("LOCAL_COMPRESSOR_BASE_URL"): + if not self.resolve_compressor_endpoint(compressor_profile)[0]: outcome = CompressionOutcome( status="UNCONFIGURED", - status_message="Н/Д: модель для сжатия не выбрана", + status_message=( + "Н/Д: модель для сжатия не выбрана" + if not compressor_profile + else "Н/Д: у выбранного профиля не задан адрес сервера" + ), tokens_before=current_token_count, tokens_after=current_token_count, compression_ratio=1.0, @@ -362,12 +373,20 @@ class ContextCompressor: # P0-3: Verify facts retention retention_rate, preserved, missing = verify_facts_retention(raw_summary, all_expected_facts) + # Полнота, которую дала САМА модель. Её нельзя терять: ниже + # недостающие факты дописываются списком, и после этого проверка + # покажет сто процентов. Замер на сервере владельца дал 97,4% — + # один факт из тридцати восьми модель потеряла. Показывая только + # исправленное число, мы скрыли бы от владельца, что модель + # теряет факты, и он не заметил бы, когда станет хуже. + model_retention_rate = retention_rate + model_missing = list(missing) + # P0-3 Safeguard: If any critical technical entities were omitted by model, append explicit factual ledger if missing: logger.info("Context compressor missed %d facts. Appending verbatim factual safeguard ledger.", len(missing)) facts_ledger = "\n### Ключевые сохранённые факты:\n" + "\n".join(f"- `{f}`" for f in missing) final_summary = f"{raw_summary}\n{facts_ledger}" - # Re-verify -> guaranteed 100% retention retention_rate, preserved, missing = verify_facts_retention(final_summary, all_expected_facts) else: final_summary = raw_summary @@ -399,7 +418,18 @@ class ContextCompressor: outcome = CompressionOutcome( status="SUCCESS", - status_message=f"Контекст успешно сжат: {est_before} → {est_after} токенов ({ratio}x, экономия {saved} токенов) за {elapsed:.2f}с. Сохранено фактов: {len(preserved)}/{all_expected_facts.total_count} (100%).", + status_message=( + f"Контекст сжат: {est_before} → {est_after} токенов " + f"({ratio}x, экономия {saved}) за {elapsed:.2f}с. " + f"Фактов сохранено: {len(preserved)}/{all_expected_facts.total_count} " + f"({retention_rate:.1f}%)" + + ( + f"; {len(model_missing)} из них дописано списком, " + f"сама модель сохранила {model_retention_rate:.1f}%." + if model_missing + else ", все — самой моделью." + ) + ), tokens_before=est_before, tokens_after=est_after, compression_ratio=ratio, @@ -408,6 +438,8 @@ class ContextCompressor: facts_total=all_expected_facts.total_count, facts_retained=len(preserved), retention_percent=retention_rate, + model_retention_percent=model_retention_rate, + facts_added_by_safeguard=model_missing, retained_facts=preserved, missing_facts=missing, model_name=model_name, diff --git a/src/antigravity_provider/router/local_supervisor.py b/src/antigravity_provider/router/local_supervisor.py index a445adc..701ef7b 100644 --- a/src/antigravity_provider/router/local_supervisor.py +++ b/src/antigravity_provider/router/local_supervisor.py @@ -83,6 +83,10 @@ class ModelMemoryRecord: history: List[Dict[str, Any]] = field(default_factory=list) +# Разделитель между сообщениями при подсчёте объёма контекста. +MESSAGE_SEPARATOR = "\n\n" + + class LocalSupervisor: """Oversees and regulates work dispatch to local models.""" @@ -97,6 +101,12 @@ class LocalSupervisor: compressor: Optional[ContextCompressor] = None, ): self.base_url = base_url.rstrip("/") + # /props и /tokenize у llama.cpp живут в КОРНЕ, а не под /v1. Проверено + # на сервере владельца: /props → 200, /v1/props → 404; то же с + # /tokenize. Адаптер передаёт сюда адрес вида .../v1, поэтому без + # нормализации оба запроса получали 404, счёт токенов молча падал на + # посимвольную оценку, и порог сжатия считался от выдуманного числа. + self.root_url = self.base_url[:-3].rstrip("/") if self.base_url.endswith("/v1") else self.base_url self.memory_path = memory_path or LOCAL_MEMORY_FILE self.compressor = compressor or ContextCompressor() @@ -105,7 +115,7 @@ class LocalSupervisor: # ------------------------------------------------------------- def query_server_props(self, timeout_sec: float = 3.0) -> ServerPropsResult: """Query real model properties and context limits from live server.""" - props_url = f"{self.base_url}/props" + props_url = f"{self.root_url}/props" try: req = urllib.request.Request(props_url, headers={"User-Agent": "Hermes-LocalSupervisor/1.0"}) with urllib.request.urlopen(req, timeout=timeout_sec) as resp: @@ -139,7 +149,7 @@ class LocalSupervisor: if not text: return TokenCountResult(tokens_count=0, is_estimated=False, method="exact_empty") - tok_url = f"{self.base_url}/tokenize" + tok_url = f"{self.root_url}/tokenize" try: payload = json.dumps({"content": text}).encode("utf-8") req = urllib.request.Request( @@ -423,7 +433,7 @@ class LocalSupervisor: full_text = "\n\n".join(str(m.get("content", "")) for m in messages if isinstance(m, dict)) current_token_count = self.count_tokens(full_text).tokens_count - return self.compressor.compress_messages_if_needed( + new_messages, outcome = self.compressor.compress_messages_if_needed( messages=messages, target_context_limit=target_context_limit, current_token_count=current_token_count, @@ -433,6 +443,24 @@ class LocalSupervisor: timeout_sec=timeout_sec, ) + # Сжиматель считает итог посимвольно (длина / 3.5): доступа к серверу у + # него нет. Здесь он есть, и степень сжатия — величина, которую владелец + # читает как измеренную. Пересчитываем настоящим токенизатором, а если + # он недоступен, честно помечаем оценкой. + if outcome.status == "SUCCESS": + new_text = MESSAGE_SEPARATOR.join( + str(m.get("content", "")) for m in new_messages if isinstance(m, dict) + ) + counted = self.count_tokens(new_text) + outcome.tokens_after = counted.tokens_count + outcome.tokens_after_is_estimate = counted.is_estimated + outcome.saved_tokens = max(0, outcome.tokens_before - counted.tokens_count) + outcome.compression_ratio = round( + counted.tokens_count / max(1, outcome.tokens_before), 2 + ) + + return new_messages, outcome + def get_compression_status(self, compressor_profile: Optional[Any] = None) -> Dict[str, Any]: """Return real-time diagnostic status of context compressor.""" if not compressor_profile and not os.environ.get("LOCAL_COMPRESSOR_BASE_URL"): diff --git a/tests/test_a56_review_findings.py b/tests/test_a56_review_findings.py new file mode 100644 index 0000000..d675e50 --- /dev/null +++ b/tests/test_a56_review_findings.py @@ -0,0 +1,233 @@ +"""Проверки по итогам разбора A56 ревьюером. + +Три находки, каждая подтверждена измерением на сервере владельца: + +1. `/props` и `/tokenize` у llama.cpp живут в корне, а не под `/v1`. + Измерено: `/props` → 200, `/v1/props` → 404; то же с `/tokenize`. + Адаптер передаёт супервизору адрес вида `.../v1`, поэтому счёт токенов + молча падал на посимвольную оценку, и порог сжатия считался от выдуманного + числа. + +2. Заявленные «100% сохранения фактов» модель не даёт. Измерено на живом + компрессоре: 5667 токенов на входе, 624 на выходе, 37 фактов из 38 — + 97,4%, потерян `001cd1f`. Сто процентов получаются дописыванием + недостающих фактов списком. Механизм верный, но подменять им измеренное + число нельзя: иначе ухудшение модели останется незамеченным. + +3. Порт 8082 был зашит в коде как запасной адрес. Задание это прямо + запрещает: сегодня он такой, завтра другой. +""" +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from antigravity_provider.router.context_compressor import ( + CompressionOutcome, + ContextCompressor, +) +from antigravity_provider.router.local_supervisor import LocalSupervisor + + +# ── Находка 1: адрес /props и /tokenize ── + +@pytest.mark.unit +@pytest.mark.parametrize( + "given, expected_root", + [ + ("http://127.0.0.1:8081/v1", "http://127.0.0.1:8081"), + ("http://127.0.0.1:8081/v1/", "http://127.0.0.1:8081"), + ("http://127.0.0.1:8081", "http://127.0.0.1:8081"), + ("http://server.local:9000/v1", "http://server.local:9000"), + ], +) +def test_props_and_tokenize_go_to_the_root(given, expected_root): + assert LocalSupervisor(base_url=given).root_url == expected_root + + +@pytest.mark.unit +def test_tokenize_request_url_has_no_v1(): + supervisor = LocalSupervisor(base_url="http://127.0.0.1:8081/v1") + seen = {} + + def _fake_urlopen(req, timeout=None): + seen["url"] = req.full_url + raise RuntimeError("сеть в тесте недоступна") + + with patch("urllib.request.urlopen", _fake_urlopen): + supervisor.count_tokens("проверка") + + assert seen["url"] == "http://127.0.0.1:8081/tokenize" + + +@pytest.mark.unit +def test_props_request_url_has_no_v1(): + supervisor = LocalSupervisor(base_url="http://127.0.0.1:8081/v1") + seen = {} + + def _fake_urlopen(req, timeout=None): + seen["url"] = req.full_url + raise RuntimeError("сеть в тесте недоступна") + + with patch("urllib.request.urlopen", _fake_urlopen): + supervisor.query_server_props() + + assert seen["url"] == "http://127.0.0.1:8081/props" + + +@pytest.mark.unit +def test_unmeasured_limit_is_marked_as_such(): + """Недоступный /props даёт запасное значение — и оно обязано быть помечено.""" + supervisor = LocalSupervisor(base_url="http://127.0.0.1:8081/v1") + + def _fake_urlopen(req, timeout=None): + raise RuntimeError("сервер не отвечает") + + with patch("urllib.request.urlopen", _fake_urlopen): + result = supervisor.query_server_props() + + assert result.is_measured is False + + +# ── Находка 2: полнота фактов не подменяется исправленной ── + +class _Profile: + custom_base_url = "http://127.0.0.1:9999/v1" + preferred_models = ["compressor"] + auth_config: dict = {} + + +def _compress_with_summary(summary: str): + payload = {"choices": [{"message": {"content": summary}}], "model": "test-gguf"} + + class _Resp: + def read(self): + import json + + return json.dumps(payload).encode() + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + messages = [ + {"role": "system", "content": "система"}, + {"role": "user", "content": "Правка в коммите 001cd1f, порт 8082, файл /srv/app/main.py"}, + {"role": "assistant", "content": "Скорость 107.4 tok/s на версии v0.1.3"}, + {"role": "user", "content": "свежее 1"}, + {"role": "user", "content": "свежее 2"}, + {"role": "user", "content": "свежее 3"}, + ] + compressor = ContextCompressor() + with patch("urllib.request.urlopen", return_value=_Resp()), \ + patch.object(ContextCompressor, "_record_to_shared_memory", lambda self, o: None): + _, outcome = compressor.compress_messages_if_needed( + messages=messages, + target_context_limit=1000, + current_token_count=900, + compressor_profile=_Profile(), + ) + return outcome + + +@pytest.mark.unit +def test_model_retention_is_reported_separately_when_facts_were_added(): + # Сводка намеренно теряет часть фактов — их дописывает страховка. + outcome = _compress_with_summary("Кратко: правка внесена, порт 8082.") + + assert outcome.status == "SUCCESS" + assert outcome.facts_added_by_safeguard, "страховка обязана была сработать" + assert outcome.model_retention_percent < 100.0 + assert outcome.retention_percent == pytest.approx(100.0) + assert "дописано списком" in outcome.status_message + assert f"{outcome.model_retention_percent:.1f}%" in outcome.status_message + + +@pytest.mark.unit +def test_full_model_retention_is_stated_as_the_model_s_own(): + summary = "Коммит 001cd1f, порт 8082, файл /srv/app/main.py, 107.4 tok/s, v0.1.3" + outcome = _compress_with_summary(summary) + + assert not outcome.facts_added_by_safeguard + assert outcome.model_retention_percent == pytest.approx(100.0) + assert "самой моделью" in outcome.status_message + + +@pytest.mark.unit +def test_status_message_never_hardcodes_a_hundred_percent(): + import inspect + + from antigravity_provider.router import context_compressor + + source = inspect.getsource(context_compressor) + assert "(100%)." not in source, "полнота обязана браться из замера, а не из строки" + + +# ── Находка 3: порт компрессора не зашит ── + +@pytest.mark.unit +def test_compressor_port_is_not_hardcoded(): + import inspect + + from antigravity_provider.router import context_compressor + + code_lines = [ + line + for line in inspect.getsource(context_compressor).splitlines() + if not line.lstrip().startswith("#") + ] + assert not any("127.0.0.1:8082" in line for line in code_lines) + + +@pytest.mark.unit +def test_profile_without_address_is_unconfigured_not_port_8082(monkeypatch): + monkeypatch.delenv("LOCAL_COMPRESSOR_BASE_URL", raising=False) + + class _NoAddress: + custom_base_url = None + preferred_models: list = [] + auth_config: dict = {} + + _, outcome = ContextCompressor().compress_messages_if_needed( + messages=[{"role": "user", "content": "x"}], + target_context_limit=1000, + current_token_count=900, + compressor_profile=_NoAddress(), + ) + + assert outcome.status == "UNCONFIGURED" + assert "адрес" in outcome.status_message + + +# ── Итог сжатия пересчитывается токенизатором ── + +@pytest.mark.unit +def test_result_size_is_recounted_by_the_tokenizer(): + supervisor = LocalSupervisor(base_url="http://127.0.0.1:8081/v1") + outcome = CompressionOutcome( + status="SUCCESS", status_message="", tokens_before=900, tokens_after=1 + ) + + with patch.object( + ContextCompressor, + "compress_messages_if_needed", + return_value=([{"role": "user", "content": "сводка"}], outcome), + ), patch.object( + LocalSupervisor, + "count_tokens", + side_effect=[ + type("T", (), {"tokens_count": 900, "is_estimated": False})(), + type("T", (), {"tokens_count": 120, "is_estimated": False})(), + ], + ): + _, result = supervisor.compress_context_if_needed( + messages=[{"role": "user", "content": "длинная история"}], + target_context_limit=1000, + ) + + assert result.tokens_after == 120 + assert result.tokens_after_is_estimate is False + assert result.saved_tokens == 780