refactor(filters): снизить сложность match_message_filter F(42)→A + safety-тесты (#922)

[axisrow]

Что и зачем

PR-5 из #922 (финальная функция раунда). match_message_filter (src/services/pipeline_filters.py) была radon F(42) — одна функция, оценивающая 9 независимых критериев фильтра линейной if-цепочкой с несколькими legacy text-fallback ветками. Покрытие было слабым (2 непрямых кейса через FilterHandler), поэтому сначала safety-тесты, потом рефактор.

1. Safety-тесты (написаны против неизменённой функции)

Новый tests/test_pipeline_filters.py — 40 кейсов, фиксирующих каждый критерий (message_kinds, service_actions, media_types, sender_kinds, forwarded, has_text, keywords, match_links, regex), legacy text-fallback, нормализацию None-config и комбинированную AND-семантику. Прогон против старого кода — зелёные (зафиксировали реальное поведение; в т.ч. выявили, что match_message_filter(msg, None) нормализуется в пустой keywords-фильтр и отклоняет всё).

2. Рефактор

_extract_message_attributes (+ _MessageAttrs) + по одному _check_*-предикату на критерий. match_message_filter стал all(check(...) for check in _MESSAGE_FILTER_CHECKS) — порядок предикатов = исходная precedence, поэтому short-circuit/AND поведение идентично.

radon cc: match_message_filter F(42) → A(3); самый крупный предикат B(9).

Проверка

• ruff check (src + tests) — clean
• pytest tests/test_pipeline_filters.py tests/test_pipeline_graph.py — 64 passed (40 новых safety + 24 существующих)

Refs #922. Внешнее поведение фильтров без изменений.

🤖 Generated with Claude Code

[axisrow]

@claude @codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 3m 29s —— View job

---

Code Review

• Read changed files (src/services/pipeline_filters.py, tests/test_pipeline_filters.py)
• Compared pre-refactor vs post-refactor logic against original commit
• Analyzed all 9 predicate functions for behavioral drift
• Reviewed test coverage

---

Bottom line: no critical issues. The refactoring is a faithful behavioral translation. I verified each _check_* function line-by-line against HEAD^'s original match_message_filter — semantics are identical.

Two findings worth knowing about, neither is a blocker:

---

1. Unreachable guard in match_message_filter (pre-existing dead code)

src/services/pipeline_filters.py line 236:

if config.get("type") != "message_filter":
    return True

Every code path through normalize_filter_config returns _empty_message_filter() as a base, which always sets "type": "message_filter". This guard was dead code in the original too — it's not a regression, just a misleading escape hatch that silently "allows" a case that can never actually occur. Not blocking, but could be removed in a follow-up.

---

2. Incomplete _MessageAttrs extraction — media_type and forward_from_channel_id bypass the cache

_MessageAttrs pre-extracts 7 attributes, but _check_media_types (line 154) and _check_forwarded (line 174) still call getattr(message, ...) directly:

# _check_media_types — media_type missing from _MessageAttrs
if config["media_types"] and getattr(message, "media_type", None) not in config["media_types"]:

# _check_forwarded — forward_from_channel_id missing from _MessageAttrs  
is_forwarded = getattr(message, "forward_from_channel_id", None) is not None

This is a design inconsistency: the message parameter must be threaded into all 9 check functions (and the uniform signature (message, attrs, config) is fine), but the pre-extraction goal of _MessageAttrs is only partially fulfilled. No behavior impact — the message object is stable across the all(...) evaluation — just means the message parameter in 7 of the 9 functions goes unused.

---

Tests

Coverage is solid — 40 cases lock down every criterion including legacy text-fallback paths, the None-config behavior, and combined AND-semantics. The test that pins match_message_filter(msg, None) is False and explains the filter_messages short-circuit is particularly good documentation.

No bugs, security issues, logical errors, or data loss risks found.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: b772b6b415

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[axisrow]

Триаж ревью (cycle 1/3)

Спасибо @claude @codex. Codex — без замечаний. Claude — критичных нет, 2 non-blocking находки. Вердикты (оба проверены по коду):

1. Недостижимый guard if config.get("type") != "message_filter": return True — SKIP (keep). Верно, что он мёртвый (normalize_filter_config всегда возвращает базу _empty_message_filter() с type="message_filter"), и это так же было в оригинале — не регрессия. Оставляю намеренно: это дешёвый защитный инвариант на случай, если normalize_filter_config когда-нибудь начнёт возвращать иной тип; удаление сделало бы поведение «тихо проходит всё» при будущем изменении нормализации. Удалять смысла нет, риск держать — нулевой.

2. Неполная экстракция _MessageAttrs (media_type, forward_from_channel_id читаются через getattr мимо кэша; message не используется в 7 из 9 чеков) — применяю в финальном cleanup-проходе. Доведу экстракцию до конца: добавлю оба поля в _MessageAttrs и уберу неиспользуемый параметр message из предикатов (сигнатура (attrs, config)). Поведение не меняется — 40 safety-тестов это подтвердят.

[claude]

Claude finished @axisrow's task in 2m 14s —— View job

---

Cleanup: complete _MessageAttrs extraction

• Read current pipeline_filters.py
• Add media_type + forward_from_channel_id to _MessageAttrs
• Update _extract_message_attributes to populate both fields
• Update _check_media_types and _check_forwarded to use attrs
• Remove unused message param from all 9 predicates → signature (attrs, config)
• Update match_message_filter call site → check(attrs, config)
• Committed and pushed (4589b32) — CI will run ruff + 40 safety tests

Summary

Commit 4589b32 completes finding #2 from the review:

• _MessageAttrs now carries all 9 extracted fields: added media_type and forward_from_channel_id
• _extract_message_attributes populates both via _safe_scalar(getattr(message, ...)), consistent with the other 7 fields
• _check_media_types and _check_forwarded now read from attrs instead of calling getattr(message, ...) directly
• All 9 predicate signatures trimmed from (message, attrs, config) → (attrs, config); message is no longer threaded through the predicate layer at all
• match_message_filter call site updated: check(attrs, config)

Behavior is unchanged — the 40 safety tests in test_pipeline_filters.py confirm this on CI.
| Branch

[chatgpt-codex-connector]

Summary

• Added media_type and forward_from_channel_id to _MessageAttrs, and now pre-extract both fields through _extract_message_attributes. src/services/pipeline_filters.pyL91-L117
• Updated all message-filter predicate signatures to use (attrs, config) and removed the unused message parameter from the check pipeline. src/services/pipeline_filters.pyL120-L220
• Updated media and forwarded checks to read from pre-extracted attrs, then adjusted match_message_filter to call each predicate with only (attrs, config). src/services/pipeline_filters.pyL157-L179 src/services/pipeline_filters.pyL238-L243
• Committed changes on the current branch: 4e65465.
• Created PR: refactor(filters): finish message attrs cleanup.

Testing

• ✅ ruff check src/services/pipeline_filters.py
• ✅ ruff check src/ tests/ conftest.py
• ⚠️ pytest tests/test_pipeline_filters.py tests/test_pipeline_graph.py -q — could not run because the installed pytest environment does not recognize the repo’s configured --dist=loadfile option without pytest-xdist.
• ⚠️ pytest tests/test_pipeline_filters.py tests/test_pipeline_graph.py -q -o addopts='' — could not run because required pytest plugins are unavailable; collection failed on the xdist hook and pytest config compatibility.
• ⚠️ pip install -e . — dependency installation was blocked by package index 403 Forbidden.
• ⚠️ pip install pytest-xdist pytest-asyncio — dependency installation was blocked by package index 403 Forbidden.

View task →

[axisrow]

Finding #2 уже применён в этой ветке коммитом 4589b32 (claude[bot] auto-fix): media_type и forward_from_channel_id добавлены в _MessageAttrs, неиспользуемый параметр message убран — все 9 предикатов теперь (attrs, config). Проверил: поведение сохранено (для реальных типов media_type: str|None / forward_from_channel_id: int|None _safe_scalar — no-op), match_message_filter остаётся A(3), 40 safety + 24 graph теста зелёные. Принимаю.

Finding #1 (мёртвый guard) оставлен намеренно как защитный инвариант — см. предыдущий комментарий.