bvandeusen
8205590f8d
The Discuss button on news cards was producing one-shot replies because the model got the whole trafilatura blob dropped into history with a canned "summarize and discuss this article" prompt — no length guard, no prep, no invitation to converse. Large articles got silently truncated by Ollama; small articles got a tepid reply. This reworks discuss_article around a three-layer cache: context_prepared → content_full → fresh trafilatura fetch First click on a small article fetches once, writes through to both caches, and passes the body straight into the synthetic read_article tool-result. First click on a large article additionally runs a parallel map step (services/article_context.py) that chunks the body on paragraph boundaries, summarizes each ~8k chunk to ~300 words of dense factual prose via the background model, and concatenates the summaries under section headers — all pinned to num_ctx=16384 so the map step doesn't itself fall victim to silent truncation. Repeat clicks on either path skip straight to the chat turn. The canned summary prompt is replaced with a conversational seed that invites the user into an actual discussion rather than a one-shot synopsis, matching the goal of "have a conversation about an article, not just read it." discuss_topic is intentionally left untouched — it's the multi-article aggregation path and needs a separate rework. Follow-up task will decide whether to retire it or rework it on the cached-context approach. Closes task #106. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
217 lines
7.9 KiB
Python
217 lines
7.9 KiB
Python
import json
|
|
import pytest
|
|
from unittest.mock import patch, AsyncMock
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# read_article tool tests
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_read_article_success():
|
|
"""read_article returns success with content when _fetch_full_article succeeds."""
|
|
from fabledassistant.services.tools import execute_tool
|
|
|
|
with patch(
|
|
"fabledassistant.services.rss._fetch_full_article",
|
|
new_callable=AsyncMock,
|
|
return_value="Article body 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 body text here."
|
|
assert result["truncated"] is False
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_read_article_fetch_failure():
|
|
"""read_article returns success=False when _fetch_full_article returns None."""
|
|
from fabledassistant.services.tools import execute_tool
|
|
|
|
with patch(
|
|
"fabledassistant.services.rss._fetch_full_article",
|
|
new_callable=AsyncMock,
|
|
return_value=None,
|
|
):
|
|
result = await execute_tool(user_id=1, tool_name="read_article", arguments={"url": "https://example.com/broken"})
|
|
|
|
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 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_callable=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 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
|
|
assert "No URL provided" in result["error"]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# History builder tests
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def _build_history(messages: list[dict]) -> list[dict]:
|
|
"""Replicate the fixed history builder from routes/chat.py."""
|
|
history = []
|
|
for msg in messages:
|
|
if msg["role"] == "system":
|
|
continue
|
|
msg_dict = {"role": msg["role"], "content": msg.get("content") or ""}
|
|
tool_calls = msg.get("tool_calls")
|
|
if tool_calls:
|
|
msg_dict["tool_calls"] = [
|
|
{"function": {"name": tc["function"], "arguments": tc["arguments"]}}
|
|
for tc in tool_calls
|
|
]
|
|
history.append(msg_dict)
|
|
for tc in tool_calls:
|
|
history.append({"role": "tool", "content": json.dumps(tc.get("result", {}))})
|
|
else:
|
|
history.append(msg_dict)
|
|
return history
|
|
|
|
|
|
def test_history_builder_replays_tool_calls():
|
|
"""History builder with tool_calls produces assistant entry + tool result entry."""
|
|
messages = [
|
|
{
|
|
"role": "assistant",
|
|
"content": "",
|
|
"tool_calls": [
|
|
{
|
|
"function": "read_article",
|
|
"arguments": {"url": "https://example.com"},
|
|
"result": {"success": True, "content": "Article text"},
|
|
}
|
|
],
|
|
},
|
|
{"role": "user", "content": "Summarize it", "tool_calls": None},
|
|
]
|
|
history = _build_history(messages)
|
|
|
|
assert len(history) == 3
|
|
assert history[0]["role"] == "assistant"
|
|
assert history[0]["tool_calls"][0]["function"]["name"] == "read_article"
|
|
assert history[1]["role"] == "tool"
|
|
assert json.loads(history[1]["content"])["success"] is True
|
|
assert history[2]["role"] == "user"
|
|
assert history[2]["content"] == "Summarize it"
|
|
|
|
|
|
def test_history_builder_no_tool_calls_unchanged():
|
|
"""History builder with tool_calls=None produces same output as before."""
|
|
messages = [
|
|
{"role": "user", "content": "Hello", "tool_calls": None},
|
|
{"role": "assistant", "content": "Hi there!", "tool_calls": None},
|
|
]
|
|
history = _build_history(messages)
|
|
|
|
assert len(history) == 2
|
|
assert history[0] == {"role": "user", "content": "Hello"}
|
|
assert history[1] == {"role": "assistant", "content": "Hi there!"}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# prepare_article_context tests
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_prepare_article_context_small_passthrough():
|
|
"""Articles under CHAR_BUDGET pass through unchanged with zero LLM calls."""
|
|
from fabledassistant.services import article_context
|
|
|
|
body = "A short article.\n\nWith two paragraphs."
|
|
with patch(
|
|
"fabledassistant.services.article_context.generate_completion",
|
|
new_callable=AsyncMock,
|
|
) as mock_gen:
|
|
out = await article_context.prepare_article_context(
|
|
"Title", "https://example.com", body, "test-model",
|
|
)
|
|
|
|
assert out == body
|
|
mock_gen.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_prepare_article_context_large_runs_map_reduce():
|
|
"""Articles over CHAR_BUDGET are chunked and map-reduced via the background model."""
|
|
from fabledassistant.services import article_context
|
|
|
|
# CHAR_BUDGET is 48_000 — build a body well over that with paragraph breaks
|
|
# so the chunker has natural splits to work with.
|
|
paragraph = ("Lorem ipsum dolor sit amet, consectetur adipiscing elit. " * 40).strip()
|
|
body = "\n\n".join([paragraph] * 30) # ~70k+ chars across 30 paragraphs
|
|
assert len(body) > article_context.CHAR_BUDGET
|
|
|
|
with patch(
|
|
"fabledassistant.services.article_context.generate_completion",
|
|
new_callable=AsyncMock,
|
|
return_value="Summary of this section with specific claims preserved.",
|
|
) as mock_gen:
|
|
out = await article_context.prepare_article_context(
|
|
"Long Article", "https://example.com/long", body, "test-model",
|
|
)
|
|
|
|
# At least one LLM call fired (the map step ran)
|
|
assert mock_gen.await_count >= 1
|
|
# Output carries the oversized-article header and section markers
|
|
assert "longer than the chat window" in out
|
|
assert "## Section 1" in out
|
|
# Map output is much smaller than the raw body
|
|
assert len(out) < len(body)
|
|
|
|
|
|
def test_chunk_by_paragraph_respects_boundaries():
|
|
"""Chunker splits on paragraph breaks, not mid-sentence."""
|
|
from fabledassistant.services.article_context import _chunk_by_paragraph, CHUNK_CHARS
|
|
|
|
paragraphs = [f"Paragraph {i}. " + ("x" * 1000) for i in range(20)]
|
|
body = "\n\n".join(paragraphs)
|
|
|
|
chunks = _chunk_by_paragraph(body)
|
|
|
|
# Each chunk stays under the budget
|
|
for chunk in chunks:
|
|
assert len(chunk) <= CHUNK_CHARS
|
|
# Total content is preserved (modulo overlap duplication, so ≥ original)
|
|
assert sum(len(c) for c in chunks) >= len(body) * 0.9
|
|
|
|
|
|
def test_chunk_by_paragraph_handles_oversized_paragraph():
|
|
"""A single paragraph larger than CHUNK_CHARS gets split mid-paragraph."""
|
|
from fabledassistant.services.article_context import _chunk_by_paragraph, CHUNK_CHARS
|
|
|
|
body = "x" * (CHUNK_CHARS * 3) # one huge paragraph, no breaks
|
|
chunks = _chunk_by_paragraph(body)
|
|
|
|
assert len(chunks) >= 3
|
|
for chunk in chunks:
|
|
assert len(chunk) <= CHUNK_CHARS
|