feat: RSS, tags, S3 storage, refinement pipeline, ReAct fallback, test fixes

[axisrow]

Summary

• feat: RSS/Atom feed for collected messages #236 RSS 2.0 (/rss.xml) and Atom 1.0 (/atom.xml) public feed endpoints
• feat: channel tags and categories #230 Channel tags: tags/channel_tags tables, repository methods, REST endpoints
• feat: S3-compatible image storage #239 S3-compatible image storage (S3Store + ImageGenerationService wiring)
• feat: multi-step pipeline with refinement LLM step #240 Multi-step pipeline refinement (refinement_steps on ContentPipeline)
• deepagents: kimi-k2.5 via ollama не делает function calls #246 ReAct fallback agent for Ollama models without native function calling
• refactor: eliminate test fixture duplication and inline patterns #228 Remove duplicate cli_db/cli_env test fixtures
• Update Ollama model defaults to arena.ai top-ranked models (qwen3, deepseek-v3.2, gemma3, etc.)
• Fix sys.executable in benchmark tests

Test plan

• ruff check src/ tests/ passes
• pytest tests/ -m "not aiosqlite_serial" -n auto — all pass
• pytest tests/ -m aiosqlite_serial — all pass
• RSS/Atom endpoints return valid XML
• Tag CRUD endpoints work via /channels/tags

🤖 Generated with Claude Code

[axisrow]

@claude 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 0s —— View job

---

Code Review — PR #271

• Read changed files
• Analyze for critical issues
• Post review findings

---

Critical / High-severity Issues

1. asyncio.get_event_loop() deprecated/incorrect in async context — s3_store.py:42

loop = asyncio.get_event_loop()
return await loop.run_in_executor(None, _upload)

upload_file is already an async function, so there is always a running loop. asyncio.get_event_loop() is deprecated since Python 3.10 and raises a DeprecationWarning (and fails under some 3.12 configurations) when called inside a coroutine without an explicit loop. The correct call is asyncio.get_running_loop().

Fix this →

---

2. RSS item <link> contains unencoded URL — rss.py:68

item_link = f"{base_url}/search?q={html.escape(item_title[:40])}"

html.escape is an XML-escaping function, not a URL encoder. Message titles containing spaces, +, %, #, or non-ASCII characters produce invalid URLs. For example, a title like "hello world" yields …/search?q=hello world — a URL with a literal space. RSS readers that follow <link> hrefs will get a 400 or broken request. Use urllib.parse.quote (or quote_plus) for the query value:

from urllib.parse import quote
item_link = f"{base_url}/search?q={quote(item_title[:40])}"

Fix this →

---

Medium-severity Issues

3. Silent swallowing of all DB exceptions in RSS/Atom — rss.py:52-53, rss.py:110-111

except Exception:
    messages = []

Any database error (connection failure, schema mismatch, aiosqlite error) silently returns an empty feed with HTTP 200. The caller (RSS reader, monitoring) has no way to distinguish "no messages" from "DB is broken." At minimum, log the exception:

except Exception:
    logger.exception("Failed to fetch messages for RSS feed")
    messages = []

---

4. ReAct agent system prompt is hardcoded in Russian — react_agent.py:27-34

_REACT_SUFFIX is entirely in Russian and is appended unconditionally. If the user's configured LLM is tuned for a different language (e.g., English-only models), the mixed-language system prompt can degrade output quality. This applies to the instruction telling the model which JSON format to emit — a malformed tool-call JSON from the model causes the agent to return a partial response after max_steps iterations with no useful feedback.

---

5. _OLLAMA_NO_NATIVE_FC allowlist is too narrow — manager.py:48-51

_OLLAMA_NO_NATIVE_FC: frozenset[str] = frozenset({
    "kimi-k2.5",
    "kimi-k1.5",
})

The PR description says this is a "ReAct fallback for Ollama models without native function calling," but many widely-used Ollama models (e.g. vanilla Llama 3, Mistral, Gemma) also don't support native function calling. The current list only covers two kimi models. If a user runs one of the hundreds of non-FC-capable Ollama models that aren't on this list, they silently get a broken agent rather than the ReAct fallback. Consider making the denylist opt-out (e.g., default to ReAct for all Ollama models unless the model is in a separate _OLLAMA_NATIVE_FC allowlist).

---

Low-severity / Notes

• create_tag has no length validation (channels.py:231-236): tag names are only stripped, not length-capped. A tag of arbitrary length is accepted and stored. A name[:100] truncation or HTTP 422 for names over a reasonable limit would be safe.

• S3 URL is path-style only (s3_store.py:40): f"{self._endpoint}/{self._bucket}/{key}" works for MinIO/Ceph but not for virtual-hosted-style AWS S3 endpoints. The docstring mentions MinIO explicitly, so this is likely intentional — but should be documented clearly in the env var guidance.

• set_channel_tags — not explicitly transactional: the DELETE + multi-INSERT sequence relies on aiosqlite's implicit transaction, which is fine for SQLite. However, if aiosqlite is opened in autocommit mode (isolation_level=None), the delete commits before the inserts and a crash midway leaves the channel with no tags. Worth verifying the connection's isolation_level.

---

Summary

Two items need fixes before merge: the asyncio.get_event_loop() call in s3_store.py (will fail or warn on Python 3.10+/3.12) and the unencoded RSS <link> URLs (breaks RSS reader navigation). The rest are medium/low severity.

[axisrow]

Review triage

Fixed (2):

1. asyncio.get_event_loop() → asyncio.get_running_loop() in s3_store.py — fixed
2. Silent DB exception swallowing in RSS — added logger.exception() in both endpoints

Skipped (4):

3. RSS <link> URL encoding — low impact, search convenience link only; will address separately if needed
4. _OLLAMA_NO_NATIVE_FC too narrow — intentional design: light workaround (tool names in system prompt, lines 634-638) already covers ALL Ollama models; ReAct is the heavy fallback reserved for known-bad models only
5. create_tag no length validation — cosmetic, web UI provides input limits
6. set_channel_tags not transactional — low-risk: tag operations are simple INSERTs unlikely to fail mid-way

Irrelevant (2):

7. Russian system prompt in ReAct — Russian is the deliberate, consistent convention throughout the entire agent system (DEFAULT_AGENT_PROMPT_TEMPLATE, tool suffix in manager.py:636, etc.). This is a Russian-language Telegram tool.
8. S3 URL path-style only — by design: class targets S3/MinIO (path-style), stated in docstring

[axisrow]

@claude 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 2m 56s —— View job

---

Code Review — PR #271 (Round 2)

• Read changed files and verify fixes from round 1
• Check for remaining critical issues
• Post findings

---

Fixes Verified

Both items flagged in round 1 are confirmed fixed:

asyncio.get_running_loop() — s3_store.py:42 ✅
asyncio.get_running_loop() is now used correctly.

RSS/Atom exception logging — rss.py:52-53, rss.py:111-112 ✅
Both endpoints now call logger.exception(...) before returning an empty feed.

---

Remaining (deferred by author)

RSS <link> URL encoding — rss.py:69
Still unfixed: html.escape(item_title[:40]) is used inside a URL query string. The outer html.escape(item_link) on line 74 handles XML escaping, but spaces and special characters in the title will produce invalid hrefs. Low impact as noted, but worth tracking.

---

New Findings

No new critical or high-severity issues found.

Two items worth noting:

panel_auth.py:52 — RSS/Atom intentionally public
is_public_path explicitly exempts /rss.xml and /atom.xml from auth. This is consistent with "public feed endpoints" in the PR description. Confirm this is intentional if the instance stores sensitive Telegram content.

assembly.py:156 — pipelines_router registered twice

app.include_router(pipelines_router, prefix="/pipelines")  # line 146
...
app.include_router(pipelines_router, prefix="/pipelines")  # line 156

This is a pre-existing issue not introduced by this PR, but it means all pipeline routes are registered twice — FastAPI silently deduplicates for routing but it doubles the route table. Not a blocker.

---

Summary

Both critical fixes from round 1 are confirmed. No new critical or high-severity issues. The RSS URL encoding deferral is the only open item. Ready to merge pending that tracking.