From 9dd4178774e6a54720b29f094fc8120631559b49 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 5 Apr 2026 15:32:54 -0400 Subject: [PATCH] docs: add article reading implementation plan --- .../plans/2026-04-05-article-reading.md | 695 ++++++++++++++++++ 1 file changed, 695 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-05-article-reading.md diff --git a/docs/superpowers/plans/2026-04-05-article-reading.md b/docs/superpowers/plans/2026-04-05-article-reading.md new file mode 100644 index 0000000..3cc6ad3 --- /dev/null +++ b/docs/superpowers/plans/2026-04-05-article-reading.md @@ -0,0 +1,695 @@ +# Article Reading Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a `read_article` tool so the LLM can fetch any URL, fix the history builder so tool context survives follow-up turns, redesign the Discuss button to inject article content as a persisted tool exchange, and remove the RSS content character cap. + +**Architecture:** Four independent changes executed in dependency order: (1) content cap removal, (2) `read_article` tool, (3) history builder fix (prerequisite for everything persisting across follow-ups), (4) Discuss endpoint + frontend. Each task is independently committable. + +**Tech Stack:** Python/Quart, SQLAlchemy async, trafilatura (already installed), httpx (already installed), Vue 3 + TypeScript frontend. + +--- + +## File map + +| Action | Path | Responsibility | +|---|---|---| +| Modify | `src/fabledassistant/services/rss.py` | Remove `CONTENT_MAX_CHARS` truncation | +| Modify | `src/fabledassistant/services/tools.py` | Add `_URL_TOOLS` list, add `read_article` to `get_tools_for_user`, add handler in `execute_tool` | +| Modify | `src/fabledassistant/routes/chat.py` | Fix history builder to replay tool_calls | +| Modify | `src/fabledassistant/services/chat.py` | Add `tool_calls` parameter to `add_message` | +| Modify | `src/fabledassistant/routes/briefing.py` | Add `POST /api/briefing/articles//discuss` endpoint | +| Modify | `frontend/src/views/BriefingView.vue` | Replace `discussArticle()` to call new endpoint | +| Modify | `tests/test_rss_service.py` | Update truncation test, add no-truncation test | +| Create | `tests/test_article_reading.py` | Tests for `read_article` tool and history builder | + +--- + +## Task 1: Remove RSS content cap + +**Files:** +- Modify: `src/fabledassistant/services/rss.py:17-18,83,213` +- Modify: `tests/test_rss_service.py:19-26` + +The `CONTENT_MAX_CHARS = 50_000` constant and all uses of `[:CONTENT_MAX_CHARS]` are removed. +Trafilatura extracts only article body text, so content is naturally bounded. + +- [ ] **Step 1: Update the truncation test to assert no truncation** + +In `tests/test_rss_service.py`, replace the existing `test_extract_item_truncates_content` test: + +```python +def test_extract_item_does_not_truncate_content(): + """extract_item() should store content without truncation.""" + from fabledassistant.services.rss import extract_item + long_text = "x" * 100_000 + entry = MagicMock() + entry.get = lambda k, d="": {"summary": long_text, "title": "", "link": "", "id": "g"}.get(k, d) + entry.content = [] + entry.published_parsed = None + item = extract_item(entry) + assert len(item["content"]) == 100_000 +``` + +- [ ] **Step 2: Run the test to confirm it fails** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/fabledassistant +make test ARGS="tests/test_rss_service.py::test_extract_item_does_not_truncate_content -v" +``` + +Expected: FAIL (current code truncates to 50_000). + +- [ ] **Step 3: Remove CONTENT_MAX_CHARS from rss.py** + +In `src/fabledassistant/services/rss.py`: + +Remove lines 17–18: +```python +# Safety cap on stored content — effectively unlimited for typical articles +CONTENT_MAX_CHARS = 50_000 +``` + +Change line 83 from: +```python + content = _html_to_text(content)[:CONTENT_MAX_CHARS] +``` +to: +```python + content = _html_to_text(content) +``` + +Change line 213 from: +```python + item.content = full_text[:CONTENT_MAX_CHARS] +``` +to: +```python + item.content = full_text +``` + +- [ ] **Step 4: Run all rss tests** + +```bash +make test ARGS="tests/test_rss_service.py -v" +``` + +Expected: all pass. The `test_extract_item_truncates_content` test name no longer exists (replaced in Step 1). + +- [ ] **Step 5: Commit** + +```bash +git add src/fabledassistant/services/rss.py tests/test_rss_service.py +git commit -m "feat(rss): remove article content character cap" +``` + +--- + +## Task 2: Add `read_article` tool + +**Files:** +- Modify: `src/fabledassistant/services/tools.py` +- Create: `tests/test_article_reading.py` + +The tool uses `_fetch_full_article` from `rss.py` (lazy import inside `execute_tool` to avoid circular dependencies). Added unconditionally to all users via a new `_URL_TOOLS` list. + +- [ ] **Step 1: Write failing tests** + +Create `tests/test_article_reading.py`: + +```python +import json +import pytest +from unittest.mock import AsyncMock, patch + + +@pytest.mark.asyncio +async def test_read_article_success(): + """read_article tool returns article content on success.""" + from fabledassistant.services.tools import execute_tool + with patch( + "fabledassistant.services.rss._fetch_full_article", + new=AsyncMock(return_value="Article text here."), + ): + result = await execute_tool( + user_id=1, + tool_name="read_article", + arguments={"url": "https://example.com/article"}, + ) + assert result["success"] is True + assert result["type"] == "article_content" + assert result["url"] == "https://example.com/article" + assert result["content"] == "Article text here." + assert result["truncated"] is False + + +@pytest.mark.asyncio +async def test_read_article_fetch_failure(): + """read_article tool returns success=False when fetch returns None.""" + from fabledassistant.services.tools import execute_tool + with patch( + "fabledassistant.services.rss._fetch_full_article", + new=AsyncMock(return_value=None), + ): + result = await execute_tool( + user_id=1, + tool_name="read_article", + arguments={"url": "https://example.com/bad"}, + ) + assert result["success"] is False + assert "Could not fetch" in result["error"] + + +@pytest.mark.asyncio +async def test_read_article_truncates_at_40k(): + """read_article tool truncates content at 40_000 chars and sets truncated=True.""" + from fabledassistant.services.tools import execute_tool + long_content = "x" * 50_000 + with patch( + "fabledassistant.services.rss._fetch_full_article", + new=AsyncMock(return_value=long_content), + ): + result = await execute_tool( + user_id=1, + tool_name="read_article", + arguments={"url": "https://example.com/long"}, + ) + assert result["success"] is True + assert len(result["content"]) == 40_000 + assert result["truncated"] is True + + +@pytest.mark.asyncio +async def test_read_article_empty_url(): + """read_article tool returns success=False when url is empty.""" + from fabledassistant.services.tools import execute_tool + result = await execute_tool( + user_id=1, + tool_name="read_article", + arguments={"url": ""}, + ) + assert result["success"] is False +``` + +- [ ] **Step 2: Run tests to confirm they fail** + +```bash +make test ARGS="tests/test_article_reading.py -v" +``` + +Expected: all 4 fail with "read_article not handled" or AttributeError. + +- [ ] **Step 3: Add `_URL_TOOLS` list and register it in `get_tools_for_user`** + +In `src/fabledassistant/services/tools.py`, add the `_URL_TOOLS` list immediately after the `_SEARCH_TOOLS` block (around line 836): + +```python +_URL_TOOLS = [ + { + "type": "function", + "function": { + "name": "read_article", + "description": ( + "Fetch and read the full text of a web page or article from a URL. " + "Use when the user shares a URL and wants you to read it, or to get " + "the full content of a linked page. " + "Do NOT use search_web for URLs — use this tool instead." + ), + "parameters": { + "type": "object", + "properties": { + "url": {"type": "string", "description": "The URL to fetch and read"} + }, + "required": ["url"], + }, + }, + } +] +``` + +In `get_tools_for_user` (around line 1034), add `_URL_TOOLS` unconditionally after `_CORE_TOOLS`: + +```python +async def get_tools_for_user(user_id: int) -> list[dict]: + """Build the tool list for a user based on their configured integrations.""" + tools = list(_CORE_TOOLS) + tools.extend(_URL_TOOLS) + tools.extend(_RAG_TOOLS) + tools.extend(_ENTITY_TOOLS) + if await is_caldav_configured(user_id): + tools.extend(_CALDAV_TOOLS) + if Config.searxng_enabled(): + tools.extend(_SEARCH_TOOLS) + tools.extend(_RESEARCH_TOOLS) + tools.extend(_IMAGE_TOOLS) + logger.debug("User %d: %d tools available", user_id, len(tools)) + return tools +``` + +- [ ] **Step 4: Add `read_article` handler in `execute_tool`** + +In `src/fabledassistant/services/tools.py`, in the `execute_tool` function, find the `elif tool_name == "search_web":` block (around line 1771). Add the new handler immediately before it: + +```python + elif tool_name == "read_article": + from fabledassistant.services.rss import _fetch_full_article + url = arguments.get("url", "").strip() + if not url: + return {"success": False, "error": "No URL provided"} + content = await _fetch_full_article(url) + if not content: + return {"success": False, "error": f"Could not fetch article content from {url}"} + _TOOL_CONTENT_CAP = 40_000 + truncated = len(content) > _TOOL_CONTENT_CAP + return { + "success": True, + "type": "article_content", + "url": url, + "content": content[:_TOOL_CONTENT_CAP], + "truncated": truncated, + } + +``` + +- [ ] **Step 5: Run the tests** + +```bash +make test ARGS="tests/test_article_reading.py -v" +``` + +Expected: all 4 pass. + +- [ ] **Step 6: Run full test suite** + +```bash +make test +``` + +Expected: all pass. + +- [ ] **Step 7: Commit** + +```bash +git add src/fabledassistant/services/tools.py tests/test_article_reading.py +git commit -m "feat(tools): add read_article tool using trafilatura extraction" +``` + +--- + +## Task 3: Fix history builder + +**Files:** +- Modify: `src/fabledassistant/routes/chat.py:162-166` +- Modify: `tests/test_article_reading.py` (add history builder tests) + +The loop that builds `history` for `run_generation` currently drops `tool_calls`. This fix replays the full tool exchange so the LLM sees prior tool results on follow-up turns. + +- [ ] **Step 1: Add history builder tests** + +Append to `tests/test_article_reading.py`: + +```python +def test_history_builder_plain_messages(): + """Messages without tool_calls are added as {role, content} unchanged.""" + import json + messages = [ + type("M", (), {"role": "system", "content": "sys", "tool_calls": None})(), + type("M", (), {"role": "user", "content": "hello", "tool_calls": None})(), + type("M", (), {"role": "assistant", "content": "hi", "tool_calls": None})(), + ] + history = _build_history(messages) + assert history == [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi"}, + ] + + +def test_history_builder_with_tool_calls(): + """Messages with tool_calls emit an assistant entry + tool result entries.""" + import json + tool_calls_data = [ + { + "function": "read_article", + "arguments": {"url": "https://example.com"}, + "result": {"success": True, "content": "Article text"}, + } + ] + messages = [ + type("M", (), {"role": "user", "content": "read this", "tool_calls": None})(), + type("M", (), { + "role": "assistant", + "content": "", + "tool_calls": tool_calls_data, + })(), + type("M", (), {"role": "user", "content": "follow up", "tool_calls": None})(), + ] + history = _build_history(messages) + assert history[0] == {"role": "user", "content": "read this"} + assert history[1]["role"] == "assistant" + assert history[1]["tool_calls"] == [ + {"function": {"name": "read_article", "arguments": {"url": "https://example.com"}}} + ] + assert history[2] == {"role": "tool", "content": json.dumps({"success": True, "content": "Article text"})} + assert history[3] == {"role": "user", "content": "follow up"} + + +def _build_history(messages): + """Inline copy of the fixed history builder for testing.""" + import json + history = [] + for msg in messages: + if msg.role == "system": + continue + msg_dict = {"role": msg.role, "content": msg.content or ""} + if msg.tool_calls: + msg_dict["tool_calls"] = [ + {"function": {"name": tc["function"], "arguments": tc["arguments"]}} + for tc in msg.tool_calls + ] + history.append(msg_dict) + for tc in msg.tool_calls: + history.append({"role": "tool", "content": json.dumps(tc.get("result", {}))}) + else: + history.append(msg_dict) + return history +``` + +- [ ] **Step 2: Run the tests to confirm they pass** + +(These tests use `_build_history` defined inline — they test the logic directly, not the route. They should pass immediately.) + +```bash +make test ARGS="tests/test_article_reading.py::test_history_builder_plain_messages tests/test_article_reading.py::test_history_builder_with_tool_calls -v" +``` + +Expected: both pass. + +- [ ] **Step 3: Apply the fix to `chat.py`** + +In `src/fabledassistant/routes/chat.py`, replace lines 162–166: + +```python + # Build history from existing messages (excluding system and the placeholder) + history = [] + for msg in conv.messages: + if msg.role != "system": + history.append({"role": msg.role, "content": msg.content}) +``` + +with: + +```python + # Build history from existing messages (excluding system and the placeholder). + # Tool calls from prior turns are replayed as assistant tool_call + tool result + # messages so the LLM retains tool context on follow-up turns. + history = [] + for msg in conv.messages: + if msg.role == "system": + continue + msg_dict = {"role": msg.role, "content": msg.content or ""} + if msg.tool_calls: + msg_dict["tool_calls"] = [ + {"function": {"name": tc["function"], "arguments": tc["arguments"]}} + for tc in msg.tool_calls + ] + history.append(msg_dict) + for tc in msg.tool_calls: + history.append({"role": "tool", "content": json.dumps(tc.get("result", {}))}) + else: + history.append(msg_dict) +``` + +`json` is already imported at the top of `chat.py`. + +- [ ] **Step 4: Run full test suite** + +```bash +make test +``` + +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/fabledassistant/routes/chat.py tests/test_article_reading.py +git commit -m "fix(chat): replay tool_calls in history so tool context survives follow-up turns" +``` + +--- + +## Task 4: Extend `add_message` to accept `tool_calls` + +**Files:** +- Modify: `src/fabledassistant/services/chat.py:183-207` + +The Discuss endpoint (Task 5) needs to store a synthetic assistant message with `tool_calls`. The existing `add_message` doesn't support this parameter. + +- [ ] **Step 1: Update `add_message` signature and body** + +In `src/fabledassistant/services/chat.py`, replace the `add_message` function (lines 183–207): + +```python +async def add_message( + conversation_id: int, + role: str, + content: str, + context_note_id: int | None = None, + status: str | None = None, + tool_calls: list | None = None, +) -> Message: + async with async_session() as session: + kwargs: dict = dict( + conversation_id=conversation_id, + role=role, + content=content, + context_note_id=context_note_id, + ) + if status is not None: + kwargs["status"] = status + if tool_calls is not None: + kwargs["tool_calls"] = tool_calls + msg = Message(**kwargs) + session.add(msg) + # Touch conversation updated_at + conv = await session.get(Conversation, conversation_id) + if conv: + conv.updated_at = datetime.now(timezone.utc) + await session.commit() + await session.refresh(msg) + return msg +``` + +- [ ] **Step 2: Run full test suite** + +```bash +make test +``` + +Expected: all pass (existing callers only use positional/keyword args that are unchanged). + +- [ ] **Step 3: Commit** + +```bash +git add src/fabledassistant/services/chat.py +git commit -m "feat(chat): add tool_calls parameter to add_message" +``` + +--- + +## Task 5: Add Discuss endpoint and update frontend + +**Files:** +- Modify: `src/fabledassistant/routes/briefing.py` +- Modify: `frontend/src/views/BriefingView.vue` + +New route: `POST /api/briefing/articles//discuss`. Fetches stored article from DB, stores a synthetic `read_article` tool exchange plus the user message, then triggers generation. Frontend replaces the inline-content approach with a call to this endpoint. + +- [ ] **Step 1: Add the discuss endpoint to briefing.py** + +At the top of `src/fabledassistant/routes/briefing.py`, add these imports (after the existing imports): + +```python +from fabledassistant.models.rss_feed import RssItem, RssFeed +from fabledassistant.services.chat import add_message, get_conversation +from fabledassistant.services.generation_buffer import GenerationState, create_buffer, get_buffer +from fabledassistant.services.generation_task import run_generation +from fabledassistant.services.settings import get_setting +``` + +Note: `get_setting` and `asyncio` are already imported. Add only what is missing. + +Then add the new route at the end of `briefing.py` (before any final lines), after the `list_news` route: + +```python +@briefing_bp.route("/articles//discuss", methods=["POST"]) +@_REQUIRE +async def discuss_article(item_id: int): + """Pre-load a briefing article as a read_article tool exchange and trigger generation.""" + uid = g.user.id + data = await request.get_json() or {} + conv_id = data.get("conv_id") + if not conv_id: + return jsonify({"error": "conv_id is required"}), 400 + + # Verify article belongs to this user (via feed ownership) + async with async_session() as session: + result = await session.execute( + select(RssItem).join(RssFeed, RssItem.feed_id == RssFeed.id) + .where(RssItem.id == item_id, RssFeed.user_id == uid) + ) + item = result.scalar_one_or_none() + if item is None: + return jsonify({"error": "Article not found"}), 404 + + # Verify conversation belongs to this user + conv = await get_conversation(uid, conv_id) + if conv is None: + return jsonify({"error": "Conversation not found"}), 404 + + # Reject if generation already running + existing = get_buffer(conv_id) + if existing and existing.state == GenerationState.RUNNING: + return jsonify({"error": "Generation already in progress"}), 409 + + article_content = item.content or "" + + # Store synthetic assistant message: read_article was already called with stored content + synthetic_tool_calls = [ + { + "function": "read_article", + "arguments": {"url": item.url}, + "result": { + "success": True, + "type": "article_content", + "url": item.url, + "content": article_content, + "truncated": False, + }, + } + ] + await add_message(conv_id, "assistant", "", status="complete", tool_calls=synthetic_tool_calls) + + # Store user message + await add_message(conv_id, "user", "Please summarize and discuss this article.") + + # Reload conversation so history includes the two new messages + conv = await get_conversation(uid, conv_id) + + # Build history (using the fixed builder from chat.py logic — duplicated here) + history = [] + for msg in conv.messages: + if msg.role == "system": + continue + msg_dict = {"role": msg.role, "content": msg.content or ""} + if msg.tool_calls: + msg_dict["tool_calls"] = [ + {"function": {"name": tc["function"], "arguments": tc["arguments"]}} + for tc in msg.tool_calls + ] + history.append(msg_dict) + for tc in msg.tool_calls: + history.append({"role": "tool", "content": json.dumps(tc.get("result", {}))}) + else: + history.append(msg_dict) + + model = await get_setting(uid, "default_model", "") or "" + from fabledassistant.config import Config as _Config + if not model: + model = _Config.OLLAMA_MODEL + + # Create placeholder assistant message and generation buffer + assistant_msg = await add_message(conv_id, "assistant", "", status="generating") + try: + buf = create_buffer(conv_id, assistant_msg.id) + except RuntimeError: + return jsonify({"error": "Generation already in progress"}), 409 + + asyncio.create_task(run_generation( + buf, history, model, + uid, conv_id, conv.title, + "Please summarize and discuss this article.", + think=True, + )) + + return jsonify({"assistant_message_id": assistant_msg.id, "status": "generating"}), 202 +``` + +- [ ] **Step 2: Run full test suite** + +```bash +make test +``` + +Expected: all pass. + +- [ ] **Step 3: Update `discussArticle` in BriefingView.vue** + +In `frontend/src/views/BriefingView.vue`, replace the `discussArticle` function: + +```typescript +async function discussArticle(item: NewsItem) { + if (!todayConvId.value || chatStore.streaming) return + if (!isToday.value) selectedConvId.value = todayConvId.value + await nextTick(() => { + document.querySelector('.briefing-center')?.scrollIntoView({ behavior: 'smooth', block: 'nearest' }) + }) + try { + await apiPost<{ assistant_message_id: number }>( + `/api/briefing/articles/${item.id}/discuss`, + { conv_id: todayConvId.value }, + ) + } catch { + return + } + // Reload conversation so the new messages appear (including the generating placeholder), + // then reconnect to the SSE stream using the existing reconnectIfGenerating helper. + await chatStore.fetchConversation(todayConvId.value) + await chatStore.reconnectIfGenerating(todayConvId.value) +} +``` + +`reconnectIfGenerating` is already exported from `useChatStore`. It finds the assistant message in `status="generating"` state and connects to the SSE stream automatically. No changes to `chat.ts` are needed. + +- [ ] **Step 4: TypeScript check** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/fabledassistant +npm --prefix frontend run type-check +``` + +Expected: no errors. + +- [ ] **Step 5: Commit** + +```bash +git add src/fabledassistant/routes/briefing.py frontend/src/views/BriefingView.vue frontend/src/stores/chat.ts +git commit -m "feat(briefing): add discuss endpoint and update frontend to use persisted article context" +``` + +--- + +## Task 6: Final verification + +- [ ] **Step 1: Run full test suite** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/fabledassistant +make test +``` + +Expected: all tests pass. + +- [ ] **Step 2: TypeScript check** + +```bash +npm --prefix frontend run type-check +``` + +Expected: no errors. + +- [ ] **Step 3: Push** + +```bash +git push origin dev +```