docs: add article reading design spec
This commit is contained in:
@@ -0,0 +1,227 @@
|
||||
# Article Reading Design
|
||||
|
||||
> **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:** Allow the LLM to fetch and read the full text of any URL on demand, fix conversation history so tool context survives follow-up turns, and make the briefing Discuss button inject article content as a persisted tool exchange rather than raw user-message text.
|
||||
|
||||
**Architecture:** Four self-contained changes — history reconstruction fix (prerequisite), `read_article` tool, Discuss endpoint, and content cap removal.
|
||||
|
||||
**Tech Stack:** Python/Quart backend, trafilatura (already installed), SQLAlchemy async, Vue 3 frontend.
|
||||
|
||||
---
|
||||
|
||||
## Problem summary
|
||||
|
||||
Three interrelated issues observed in briefing conversations:
|
||||
|
||||
1. **Missing `read_article` tool** — when a user pastes a URL, the LLM calls `search_web` (a SearXNG text search), which returns generic site descriptions instead of article content.
|
||||
2. **History reconstruction bug** — `routes/chat.py:166` builds the `history` list with only `role` + `content`, silently dropping all `tool_calls` and their results from prior turns. Tool context is lost on every follow-up.
|
||||
3. **Discuss button UX** — inlines raw article text into the user message bubble. Feels clumsy, and the model sometimes searches notes on follow-ups anyway because the article isn't clearly marked as "loaded" context.
|
||||
|
||||
---
|
||||
|
||||
## Components
|
||||
|
||||
### 1. History reconstruction fix
|
||||
**File:** `src/fabledassistant/routes/chat.py`
|
||||
|
||||
The loop at line ~164 that builds `history` must be updated to replay tool exchanges:
|
||||
|
||||
```python
|
||||
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)
|
||||
```
|
||||
|
||||
The `tool_calls` JSONB column already stores `[{function, arguments, result}]` per call. No schema change needed.
|
||||
|
||||
### 2. `read_article` tool
|
||||
**Files:** `src/fabledassistant/services/research.py`, `src/fabledassistant/services/tools.py`, `src/fabledassistant/services/rss.py`
|
||||
|
||||
Move `_fetch_full_article` from `rss.py` to `research.py` (imported back into `rss.py` to avoid breaking existing calls). This makes it available to `execute_tool` without a circular import.
|
||||
|
||||
Tool definition added to `_TOOLS` in `tools.py`:
|
||||
|
||||
```python
|
||||
{
|
||||
"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"}
|
||||
},
|
||||
"required": ["url"],
|
||||
},
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
`execute_tool` handler:
|
||||
|
||||
```python
|
||||
elif tool_name == "read_article":
|
||||
from fabledassistant.services.research 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,
|
||||
}
|
||||
```
|
||||
|
||||
### 3. `add_message` — add `tool_calls` parameter
|
||||
**File:** `src/fabledassistant/services/chat.py`
|
||||
|
||||
`add_message` needs to accept and store `tool_calls` so the Discuss endpoint can create synthetic messages:
|
||||
|
||||
```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:
|
||||
```
|
||||
|
||||
Set `msg.tool_calls = tool_calls` when provided.
|
||||
|
||||
### 4. Discuss endpoint
|
||||
**File:** `src/fabledassistant/routes/briefing.py`
|
||||
|
||||
New route: `POST /api/briefing/articles/<int:item_id>/discuss`
|
||||
|
||||
Request body: `{"conv_id": <int>}`
|
||||
|
||||
Steps:
|
||||
1. Look up `rss_items` row by `item_id` — verify it belongs to the user via feed ownership. Return 404 if not found.
|
||||
2. Look up conversation by `conv_id` — verify it belongs to the user. Return 404 if not found.
|
||||
3. If generation already running for `conv_id` → return 409.
|
||||
4. Fetch stored content: `article_content = item.content or item.snippet or ""`
|
||||
5. Store synthetic assistant message (status=`"complete"`, role=`"assistant"`, content=`""`, tool_calls as below):
|
||||
```python
|
||||
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)
|
||||
```
|
||||
6. Store user message: `await add_message(conv_id, "user", "Please summarize and discuss this article.")`
|
||||
7. Build `history` from `conv.messages` (using the fixed builder above).
|
||||
8. Create assistant placeholder, create buffer, launch `run_generation` as normal.
|
||||
9. Return `{"assistant_message_id": ..., "status": "generating"}` 202.
|
||||
|
||||
### 5. Frontend: BriefingView.vue
|
||||
**File:** `frontend/src/views/BriefingView.vue`
|
||||
|
||||
Replace `discussArticle()`:
|
||||
|
||||
```typescript
|
||||
async function discussArticle(item: NewsItem) {
|
||||
if (!todayConvId.value) return
|
||||
if (!isToday.value) selectedConvId.value = todayConvId.value
|
||||
await nextTick(() => {
|
||||
document.querySelector('.briefing-center')?.scrollIntoView({ behavior: 'smooth', block: 'nearest' })
|
||||
})
|
||||
await apiClient.post(`/api/briefing/articles/${item.id}/discuss`, {
|
||||
conv_id: todayConvId.value,
|
||||
})
|
||||
// Re-fetch conversation so the new messages appear, then start SSE streaming.
|
||||
// The existing chatStore.fetchConversation + startStreaming pattern handles this.
|
||||
await chatStore.fetchConversation(todayConvId.value)
|
||||
chatStore.startStreaming(todayConvId.value)
|
||||
}
|
||||
```
|
||||
|
||||
The exact method names (`fetchConversation`, `startStreaming`) should match what `BriefingView.vue` already uses for the reply flow — confirm during implementation.
|
||||
|
||||
The article no longer appears as wall-of-text in the user bubble. The chat UI shows it as a `read_article` tool call card (already handled by `ToolCallCard.vue`).
|
||||
|
||||
### 6. Content cap removal
|
||||
**File:** `src/fabledassistant/services/rss.py`
|
||||
|
||||
Remove `[:CONTENT_MAX_CHARS]` from:
|
||||
- `content = _html_to_text(content)[:CONTENT_MAX_CHARS]` in `extract_item()`
|
||||
- `item.content = full_text[:CONTENT_MAX_CHARS]` in the enrichment task
|
||||
|
||||
The `CONTENT_MAX_CHARS` constant can be removed entirely. Trafilatura extracts only article body text (typically 2K–15K chars for news articles), so content is naturally bounded.
|
||||
|
||||
---
|
||||
|
||||
## Data flow
|
||||
|
||||
### User pastes a URL in chat
|
||||
1. User sends message with a URL
|
||||
2. LLM calls `read_article(url)`
|
||||
3. `execute_tool` calls `_fetch_full_article(url)` → trafilatura extracts clean text
|
||||
4. Tool result appended in-memory as `{role: "tool", content: json}`
|
||||
5. LLM responds based on article content
|
||||
6. Generation saves assistant message with `tool_calls=[{function:"read_article", arguments, result}]`
|
||||
7. Follow-up turns: history builder replays tool_call + tool result → article stays in context
|
||||
|
||||
### User clicks Discuss on a briefing article
|
||||
1. Frontend calls `POST /api/briefing/articles/{item_id}/discuss` with `{conv_id}`
|
||||
2. Backend fetches stored article text from DB (no network request)
|
||||
3. Backend stores synthetic assistant message with `read_article` tool result
|
||||
4. Backend stores user message `"Please summarize and discuss this article."`
|
||||
5. Generation runs — LLM sees pre-loaded article in history
|
||||
6. Follow-ups retain context via fixed history builder
|
||||
|
||||
---
|
||||
|
||||
## Error handling
|
||||
|
||||
| Scenario | Behaviour |
|
||||
|---|---|
|
||||
| `_fetch_full_article` returns `None` (network/extraction failure) | Tool returns `{success: False, error: "Could not fetch article content from [url]"}` — LLM reports conversationally |
|
||||
| Discuss: `item_id` not found or wrong user | 404 |
|
||||
| Discuss: `conv_id` not found or wrong user | 404 |
|
||||
| Discuss: article has no stored content | Falls back to empty string — LLM works with what it has |
|
||||
| Discuss: generation already running | 409 |
|
||||
| Messages with `tool_calls = None` | History builder unchanged — no regression for existing conversations |
|
||||
|
||||
---
|
||||
|
||||
## Tests
|
||||
|
||||
- **Unit:** `_fetch_full_article` returns `None` → `read_article` tool result has `success: False`
|
||||
- **Unit:** History builder with a stored message that has `tool_calls` → output includes assistant tool_call dict + a `{role: "tool"}` dict
|
||||
- **Unit:** History builder with messages where `tool_calls = None` → output unchanged from current behaviour
|
||||
- **Integration:** `POST /api/briefing/articles/{item_id}/discuss` → two messages stored (synthetic assistant + user message), generation triggered, returns 202
|
||||
Reference in New Issue
Block a user