From 006f4f33b9d880b41c6bfa8bb450534889385d87 Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 3 Nov 2026 14:10:16 +0800 Subject: [PATCH 1/4] feat(fetch): read a teaser article's own page for its body Some feeds carry one sentence per item: the 2026-11-03 morning story from pjwu.me was summarized from a 27-character description while its page holds about 1300 characters. 17 of 51 feeds measured that day had a median under 500 characters of text. adapters/fetch/article_page.lengthen_articles reads the page of each RSS article whose text is shorter than summarize_snippet_length, extracts the body with trafilatura (html_text.page_text, no tag-strip fallback, which on a whole page would return nav and footer) and keeps whichever text is longer. Any failure keeps the feed text, one page at a time. Bounds: 8 pages in flight, 10 s per page including redirects and body, 5 MiB of raw bytes. Pages are asked for uncompressed and an encoded answer is refused, since httpx inflates before a cap could count it. Only http(s) is asked; IP literals that are not global, in any IPv4 spelling, and localhost names are refused on every hop. Hostnames are not resolved: pinning addresses needs a custom transport, which the outbound-http-honours-environment spec forbids; the owner accepted the residual risk. Video links (media_links in keywords.json: /watch, /shorts, youtu.be) get no page read, because a video page's text is the site's navigation. The User-Agent names cyris and nothing else. Co-Authored-By: Claude Opus 5.5 --- src/cyris/adapters/fetch/article_page.py | 207 ++++++++++++ src/cyris/adapters/fetch/html_text.py | 27 +- src/cyris/adapters/fetch/keywords.json | 6 + src/cyris/adapters/fetch/keywords.py | 8 + tests/test_article_page.py | 412 +++++++++++++++++++++++ tests/test_grade_d_no_defaults.py | 2 + tests/test_html_text.py | 22 +- 7 files changed, 677 insertions(+), 7 deletions(-) create mode 100644 src/cyris/adapters/fetch/article_page.py create mode 100644 tests/test_article_page.py diff --git a/src/cyris/adapters/fetch/article_page.py b/src/cyris/adapters/fetch/article_page.py new file mode 100644 index 00000000..df332dee --- /dev/null +++ b/src/cyris/adapters/fetch/article_page.py @@ -0,0 +1,207 @@ +"""A teaser feed article's own page, read for its body during one run. + +Some feeds carry one sentence per entry, and a summary of that sentence is all the +model can write. An RSS article whose text is shorter than the summarize window +has its page fetched and its body extracted through `html_text`, and the longer +of the two texts is kept. The text lives in the returned articles only: nothing +here writes it to a store, so the next run reads the feed's text again. +""" + +from __future__ import annotations + +import asyncio +import codecs +import ipaddress +import logging +import socket +from collections import Counter +from urllib.parse import urlsplit + +import httpx + +from cyris.adapters.fetch.html_text import page_text +from cyris.adapters.fetch.keywords import is_media_link +from cyris.domain.models import Article, SourceConfig + +logger = logging.getLogger(__name__) + +# An issue holds about a hundred articles and a third of them are teasers +# (measured 2026-11-03), so eight at once reads them in about five waves. Many +# teasers share a publisher, and eight keeps any one site to a reader's handful. +PAGE_CONCURRENCY = 8 +# One page, its redirects and its body included. With the concurrency above, a +# run's added time is at most ceil(teasers / PAGE_CONCURRENCY) * this: about 50s +# for 33 teasers, against a run container capped at 900s whose slowest run took +# 196s (`RUN_INACTIVITY_TIMEOUT_MS` in workers/app/src/index.js). +PAGE_TIMEOUT_SECONDS = 10 +# The most one page may hold, counted as it arrives: an article page is a few +# hundred kilobytes, and a link to a large file is not an article. +MAX_PAGE_BYTES = 5 * 1024 * 1024 +# Names the fetcher and where to read about it, as the RSS Worker's does; a site +# that blocks it is answered by the feed text, which the run keeps. +USER_AGENT = "cyris (+https://github.com/musingfox/cyris)" + +_HTML_TYPES = ("text/html", "application/xhtml+xml") + + +class _PageRefusedError(Exception): + """A page this module does not read: a private address, the wrong type, too large.""" + + +def _private_literal(host: str) -> bool: + """Whether `host` names this machine or a non-public address without a lookup. + + That is an IP address written out and not global, any IPv4 spelling a resolver + reads that `ipaddress` does not (`127.1`, `2130706433`, `0x7f000001`, octal), + and localhost or a name under it (RFC 6761 reserves them for loopback). + """ + # WHY no DNS: a name that resolves to a private address is not refused here; the owner + # accepted that risk (architecture.md §3.2), and through the proxy the proxy resolves it. + name = host.strip("[]").lower() + name = name.removesuffix(".") + if name == "localhost" or name.endswith(".localhost"): + return True + try: + return not ipaddress.ip_address(name).is_global + except ValueError: + pass + try: + socket.inet_aton(name) + except (OSError, ValueError): + return False + # A shorthand IPv4 address: refused whatever it names, as no page is linked so. + return True + + +def _web_page(url: str) -> bool: + try: + parts = urlsplit(url) + host = parts.hostname + except ValueError: + return False + return ( + parts.scheme in ("http", "https") + and bool(host) + and not _private_literal(host) + # A video's page text is the site's navigation, not the video. + and not is_media_link(host, parts.path) + ) + + +async def _refuse_private(request: httpx.Request) -> None: + # Every hop of a redirect passes here, not only the first address. + if _private_literal(request.url.host): + raise _PageRefusedError(f"private address {request.url.host}") + + +async def _fetch(client: httpx.AsyncClient, url: str) -> str: + """The body text of the page at `url`; raises when there is none to read.""" + async with client.stream("GET", url) as response: + if not response.is_success: + # No retry: a 4xx will not change, and a 5xx falls back to the feed text. + raise _PageRefusedError(f"HTTP {response.status_code}") + media_type = response.headers.get("content-type", "").split(";", 1)[0].strip().lower() + if media_type not in _HTML_TYPES: + raise _PageRefusedError(f"not a page ({media_type or 'no type'})") + # Asked for identity; an answer encoded anyway could inflate past any cap. + encoding = response.headers.get("content-encoding", "").strip().lower() + if encoding not in ("", "identity"): + raise _PageRefusedError(f"encoded ({encoding})") + body = bytearray() + async for chunk in response.aiter_raw(): + body += chunk + if len(body) > MAX_PAGE_BYTES: + raise _PageRefusedError(f"larger than {MAX_PAGE_BYTES} bytes") + markup = _markup(bytes(body), response.charset_encoding) + # Off the event loop: a page can be megabytes of markup. + return await asyncio.to_thread(page_text, markup) + + +def _markup(body: bytes, charset: str | None) -> str | bytes: + """A charset the server named is decoded here; without one, trafilatura reads + the page's own `` from the bytes.""" + if not charset: + return body + try: + codecs.lookup(charset) + except LookupError: + # A name no codec knows (`x-bogus`, `none`): utf-8 is what most pages are. + charset = "utf-8" + return body.decode(charset, errors="replace") + + +async def _lengthen_one( + client: httpx.AsyncClient, gate: asyncio.Semaphore, article: Article, outcomes: Counter +) -> Article: + async with gate: + try: + async with asyncio.timeout(PAGE_TIMEOUT_SECONDS): + text = await _fetch(client, article.url) + except TimeoutError: + reason = f"no page within {PAGE_TIMEOUT_SECONDS}s" + except (httpx.HTTPError, httpx.InvalidURL, ValueError, _PageRefusedError) as e: + reason = str(e) or type(e).__name__ + except Exception as e: + # Anything else, an extractor's own error included: the pages share one + # gather, and one page must not cost the others their body. + reason = f"unexpected {type(e).__name__}: {e}" + logger.warning("Reading %s failed unexpectedly", article.url, exc_info=True) + else: + if len(text) > len(article.content): + outcomes["lengthened"] += 1 + return article.model_copy(update={"content": text}) + reason = "page body no longer than the feed text" + outcomes["kept"] += 1 + logger.info("Article page of %s (%s) not used: %s", article.url, article.source_name, reason) + return article + + +async def lengthen_articles( + articles: list[Article], + *, + summarize_snippet_length: int, + sources: dict[str, SourceConfig], +) -> list[Article]: + """`articles` in order, each RSS teaser on its page's body where that is longer. + + A teaser is an article of an RSS source whose text is shorter than + `summarize_snippet_length`, the most of it a summary prompt reads. Anything + else, or a page that fails, is returned as it came: one page never fails the run. + """ + + def teaser(article: Article) -> bool: + source = sources.get(article.source_name) + return ( + source is not None + and source.type == "rss" + and len(article.content) < summarize_snippet_length + and _web_page(article.url) + ) + + short = [i for i, a in enumerate(articles) if teaser(a)] + if not short: + return list(articles) + + gate = asyncio.Semaphore(PAGE_CONCURRENCY) + outcomes: Counter = Counter() + async with httpx.AsyncClient( + timeout=PAGE_TIMEOUT_SECONDS, + follow_redirects=True, + # Uncompressed, so `MAX_PAGE_BYTES` counts what arrives, not what one chunk + # inflates to. + headers={"User-Agent": USER_AGENT, "Accept-Encoding": "identity"}, + event_hooks={"request": [_refuse_private]}, + ) as client: + lengthened = await asyncio.gather( + *(_lengthen_one(client, gate, articles[i], outcomes) for i in short) + ) + out = list(articles) + for i, article in zip(short, lengthened, strict=True): + out[i] = article + logger.info( + "Article pages: %d teaser(s), %d read from their page, %d kept their feed text", + len(short), + outcomes["lengthened"], + outcomes["kept"], + ) + return out diff --git a/src/cyris/adapters/fetch/html_text.py b/src/cyris/adapters/fetch/html_text.py index 18de452d..b55aa44e 100644 --- a/src/cyris/adapters/fetch/html_text.py +++ b/src/cyris/adapters/fetch/html_text.py @@ -1,4 +1,4 @@ -"""Feed-entry HTML to the body text prompts and language detection read.""" +"""Feed-entry and article-page HTML to the body text prompts and language detection read.""" from __future__ import annotations @@ -61,12 +61,29 @@ def plain_text(markup: str) -> str: """ if not markup: return "" - extracted = trafilatura.extract( - markup, favor_recall=True, include_comments=False, include_tables=True - ) + extracted = _extract(markup) if extracted: - return extracted.strip() + return extracted # trafilatura returns None for a fragment too short or too bare to look like an # article (a one-line teaser); an empty content would read as "no body" to the # summary prompt, so keep the words the old way instead. return _strip_tags(markup) + + +def page_text(markup: str | bytes) -> str: + """The body text of a whole web page, or "" when trafilatura finds no article in it. + + No tag-stripping fallback, unlike `plain_text`: on a page that would be the + navigation and footer, which only looks longer than a feed's teaser. Bytes are + decoded by trafilatura, which reads the page's own ``. + """ + if not markup: + return "" + return _extract(markup) + + +def _extract(markup: str | bytes) -> str: + extracted = trafilatura.extract( + markup, favor_recall=True, include_comments=False, include_tables=True + ) + return extracted.strip() if extracted else "" diff --git a/src/cyris/adapters/fetch/keywords.json b/src/cyris/adapters/fetch/keywords.json index 90dfb2da..4e6570d0 100644 --- a/src/cyris/adapters/fetch/keywords.json +++ b/src/cyris/adapters/fetch/keywords.json @@ -37,6 +37,12 @@ ".svg", ".webp" ], + "_comment_media_links": "A feed item linking here is a video (docs/milestones/multimedia-sources-podcast-youtube.md: an item linking to youtube.com/watch is a YouTube video; channel feeds also link /shorts/ and youtu.be), so no teaser page is read for it: a video page's text is its navigation. Same host and path keys as share_links.", + "media_links": [ + { "host": "youtube.com", "path_segment_prefix": "/watch" }, + { "host": "youtube.com", "path_segment_prefix": "/shorts" }, + { "host": "youtu.be", "path_contains": "/" } + ], "share_links": [ { "host": "facebook.com", "path_contains": "sharer" }, { "host": "linkedin.com", "path_segment_prefix": "/share" }, diff --git a/src/cyris/adapters/fetch/keywords.py b/src/cyris/adapters/fetch/keywords.py index 68f5c072..343761e6 100644 --- a/src/cyris/adapters/fetch/keywords.py +++ b/src/cyris/adapters/fetch/keywords.py @@ -95,6 +95,14 @@ def is_share_link(hostname: str, path: str) -> bool: ) +def is_media_link(hostname: str, path: str) -> bool: + """A video's own page, whose text is the site's navigation rather than the video.""" + return any( + host_matches(hostname, rule["host"]) and _path_matches(rule, path) + for rule in _vocabulary()["media_links"] + ) + + def is_rejected_path(path: str) -> bool: """Paths that disqualify a link wherever it is hosted.""" vocab = _vocabulary() diff --git a/tests/test_article_page.py b/tests/test_article_page.py new file mode 100644 index 00000000..be217f96 --- /dev/null +++ b/tests/test_article_page.py @@ -0,0 +1,412 @@ +"""Tests for `lengthen_articles`: a short feed article's own page, read for its body. + +respx answers every page; no test reaches a real site. +""" + +import asyncio +import gzip +import logging +import zlib +from datetime import UTC, datetime + +import httpx +import pytest +import respx + +from cyris.adapters.fetch import article_page +from cyris.adapters.fetch.article_page import PAGE_CONCURRENCY, lengthen_articles +from cyris.domain.models import Article, Lane, SourceConfig + +pytestmark = pytest.mark.unit + +WINDOW = 200 +FEED = SourceConfig(name="Teaser Feed", url="https://pjwu.test/rss.xml") +LETTERS = SourceConfig(name="Letters", type="newsletter", email_match="letters@pjwu.test") +SOURCES = {s.name: s for s in (FEED, LETTERS)} +TEASER = "One sentence about standards." +BODY = " ".join( + f"Paragraph {i} explains why standards set by others shape the work we choose to do." + for i in range(12) +) + + +def _page(body: str = BODY) -> str: + return ( + "Others' standards" + "" + f"

Others' standards

{body}

" + "
Site footer
" + ) + + +def _html(body: str = BODY, status: int = 200) -> httpx.Response: + return httpx.Response( + status, text=_page(body), headers={"Content-Type": "text/html; charset=utf-8"} + ) + + +def _article(url: str, content: str = TEASER, source: SourceConfig = FEED) -> Article: + return Article( + id=url, + title="Others' standards", + url=url, + content=content, + published_at=datetime(2026, 11, 3, tzinfo=UTC), + source_name=source.name, + source_lane=Lane.READ, + ) + + +async def _lengthen(articles: list[Article]) -> list[Article]: + return await lengthen_articles(articles, summarize_snippet_length=WINDOW, sources=SOURCES) + + +async def test_a_teaser_runs_on_its_pages_longer_body() -> None: + url = "https://pjwu.test/writing/others-standards/" + async with respx.mock: + page = respx.get(url).mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert page.call_count == 1 + assert "Paragraph 11 explains why standards" in out.content + assert len(out.content) > len(TEASER) + assert "Site footer" not in out.content + assert (out.url, out.title, out.id) == (url, "Others' standards", url) + + +async def test_the_request_names_cyris_and_impersonates_no_browser() -> None: + url = "https://pjwu.test/writing/others-standards/" + async with respx.mock: + page = respx.get(url).mock(return_value=_html()) + await _lengthen([_article(url)]) + + assert page.call_count == 1 + agent = page.calls.last.request.headers["user-agent"] + assert "cyris" in agent + assert "Mozilla" not in agent + assert "@" not in agent + + +async def test_the_request_asks_for_the_page_uncompressed() -> None: + url = "https://pjwu.test/writing/others-standards/" + async with respx.mock: + page = respx.get(url).mock(return_value=_html()) + await _lengthen([_article(url)]) + + assert page.call_count == 1 + assert page.calls.last.request.headers["accept-encoding"] == "identity" + + +@pytest.mark.parametrize("encoding", ["gzip", "deflate"]) +async def test_a_page_sent_encoded_anyway_is_refused_undecoded( + encoding: str, caplog: pytest.LogCaptureFixture +) -> None: + url = "https://pjwu.test/writing/others-standards/" + packed = ( + gzip.compress(_page().encode()) if encoding == "gzip" else zlib.compress(_page().encode()) + ) + async with respx.mock: + page = respx.get(url).mock( + return_value=httpx.Response( + 200, + content=packed, + headers={"Content-Type": "text/html", "Content-Encoding": encoding}, + ) + ) + with caplog.at_level(logging.INFO, logger=article_page.__name__): + (out,) = await _lengthen([_article(url)]) + + assert page.call_count == 1 + assert out.content == TEASER + # Refused for its encoding, not for what it inflated to. + assert f"encoded ({encoding})" in caplog.text + + +async def test_the_cap_counts_the_bytes_as_they_arrive(monkeypatch) -> None: + monkeypatch.setattr(article_page, "MAX_PAGE_BYTES", len(_page().encode())) + url = "https://pjwu.test/writing/exactly-the-cap/" + async with respx.mock: + page = respx.get(url).mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert page.call_count == 1 + assert "Paragraph 11" in out.content + + +async def test_a_redirected_page_is_followed_to_its_body() -> None: + url = "https://pinchlime.test/writing/others-standards/" + moved = "https://pjwu.test/writing/others-standards/" + async with respx.mock: + old = respx.get(url).mock(return_value=httpx.Response(301, headers={"Location": moved})) + new = respx.get(moved).mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert (old.call_count, new.call_count) == (1, 1) + assert "Paragraph 11" in out.content + assert out.url == url + + +@pytest.mark.parametrize("extra", [0, 1, 500], ids=["exactly-the-window", "one-over", "long"]) +async def test_a_feed_text_as_long_as_the_window_sends_no_request(extra: int) -> None: + content = "x" * (WINDOW + extra) + async with respx.mock: + anything = respx.route().mock(return_value=_html()) + (out,) = await _lengthen([_article("https://pjwu.test/long", content)]) + + assert not anything.called + assert out.content == content + + +@pytest.mark.parametrize( + "answer", + [ + httpx.Response(404, text="gone"), + httpx.Response(403, text="blocked", headers={"Content-Type": "text/html"}), + httpx.Response(500, text="oops"), + httpx.ReadTimeout("slow"), + httpx.ConnectError("refused"), + httpx.Response(200, content=b"%PDF-1.7", headers={"Content-Type": "application/pdf"}), + httpx.Response(200, text="

Tiny.

", headers={"Content-Type": "text/html"}), + httpx.Response(200, text="", headers={"Content-Type": "text/html"}), + ], + ids=["404", "403", "500", "timeout", "refused", "pdf", "shorter-body", "empty"], +) +async def test_a_page_that_fails_or_says_less_keeps_the_feed_text(answer) -> None: + url = "https://pjwu.test/writing/others-standards/" + async with respx.mock: + page = respx.get(url).mock(side_effect=[answer]) + (out,) = await _lengthen([_article(url)]) + + assert page.call_count == 1 + assert out.content == TEASER + + +async def test_a_page_slower_than_the_timeout_keeps_the_feed_text(monkeypatch) -> None: + monkeypatch.setattr(article_page, "PAGE_TIMEOUT_SECONDS", 0.05) + url = "https://pjwu.test/writing/slow/" + asked = [] + + async def stall(request): + asked.append(request.url) + await asyncio.sleep(5) + return _html() + + async with respx.mock: + page = respx.get(url).mock(side_effect=stall) + (out,) = await asyncio.wait_for(_lengthen([_article(url)]), timeout=2) + + assert asked == [url] + # respx counts a call when its answer returns, and this one never does. + assert page.call_count == 0 + assert out.content == TEASER + + +async def test_a_page_larger_than_the_cap_keeps_the_feed_text(monkeypatch) -> None: + monkeypatch.setattr(article_page, "MAX_PAGE_BYTES", 1024) + url = "https://pjwu.test/writing/huge/" + async with respx.mock: + page = respx.get(url).mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert page.call_count == 1 + assert out.content == TEASER + + +@pytest.mark.parametrize( + "url", + [ + "newsletter:3f2a9c", + "ftp://pjwu.test/writing/others-standards.txt", + "mailto:editor@pjwu.test", + "file:///etc/passwd", + "https://", + "http://169.254.169.254/latest/meta-data/", + "http://127.0.0.1:8080/admin", + "http://[::1]/", + ], +) +async def test_an_address_that_is_not_a_public_web_page_is_never_fetched(url: str) -> None: + async with respx.mock: + anything = respx.route().mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert not anything.called + assert out.content == TEASER + + +async def test_a_redirect_to_a_private_address_is_not_followed() -> None: + url = "https://pjwu.test/writing/moved/" + async with respx.mock: + hop = respx.get(url).mock( + return_value=httpx.Response(302, headers={"Location": "http://10.0.0.7/internal"}) + ) + inside = respx.get("http://10.0.0.7/internal").mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert hop.call_count == 1 + assert not inside.called + assert out.content == TEASER + + +# Loopback written the ways a resolver reads it, beyond the dotted quad: socket.inet_aton +# takes short, integer, hex and octal IPv4, and RFC 6761 reserves localhost and its names. +LOCAL_HOSTS = [ + "127.1", + "2130706433", + "0x7f000001", + "0177.0.0.1", + "localhost", + "localhost.", + "LOCALHOST", + "app.localhost", +] + + +@pytest.mark.parametrize("host", LOCAL_HOSTS) +async def test_a_local_host_in_any_spelling_is_never_fetched(host: str) -> None: + async with respx.mock: + anything = respx.route().mock(return_value=_html()) + (out,) = await _lengthen([_article(f"http://{host}/admin")]) + + assert not anything.called + assert out.content == TEASER + + +@pytest.mark.parametrize("host", LOCAL_HOSTS) +async def test_a_redirect_to_a_local_host_in_any_spelling_is_not_followed(host: str) -> None: + url = "https://pjwu.test/writing/moved/" + async with respx.mock: + hop = respx.get(url).mock( + return_value=httpx.Response(302, headers={"Location": f"http://{host}/admin"}) + ) + inside = respx.route().mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert hop.call_count == 1 + assert not inside.called + assert out.content == TEASER + + +@pytest.mark.parametrize( + "url", + [ + "https://www.youtube.com/watch?v=Nbwv5wHQoj0", + "https://www.youtube.com/shorts/Nbwv5wHQoj0", + "https://youtu.be/Nbwv5wHQoj0", + ], + ids=["watch", "shorts", "short-link"], +) +async def test_a_video_whose_description_is_short_sends_no_request(url: str) -> None: + # A video page's extracted text is its footer navigation, which beats a short + # description on length. + description = "A short description of the video." + async with respx.mock: + anything = respx.route().mock(return_value=_html()) + (out,) = await _lengthen([_article(url, description)]) + + assert not anything.called + assert out.content == description + + +@pytest.mark.parametrize( + "source", [LETTERS, SourceConfig(name="Gone")], ids=["newsletter", "unlisted"] +) +async def test_only_an_rss_sources_article_is_fetched(source: SourceConfig) -> None: + url = "https://pjwu.test/p/issue-12" + async with respx.mock: + anything = respx.route().mock(return_value=_html()) + (out,) = await _lengthen([_article(url, source=source)]) + + assert not anything.called + assert out.content == TEASER + + +async def test_pages_are_fetched_at_most_the_concurrency_at_once() -> None: + in_flight = 0 + peak = 0 + + async def answer(request): + nonlocal in_flight, peak + in_flight += 1 + peak = max(peak, in_flight) + await asyncio.sleep(0.01) + in_flight -= 1 + return _html() + + urls = [f"https://pjwu.test/writing/{i}/" for i in range(PAGE_CONCURRENCY * 3)] + async with respx.mock: + pages = respx.get(url__regex=r"https://pjwu\.test/writing/\d+/").mock(side_effect=answer) + out = await _lengthen([_article(u) for u in urls]) + + assert pages.call_count == len(urls) + assert peak == PAGE_CONCURRENCY + assert [a.url for a in out] == urls + assert all("Paragraph 11" in a.content for a in out) + + +async def test_one_failed_page_leaves_the_others_lengthened_in_order() -> None: + ok, down, long = (f"https://pjwu.test/writing/{name}/" for name in ("ok", "down", "long")) + articles = [_article(ok), _article(down), _article(long, "y" * WINDOW)] + async with respx.mock: + good = respx.get(ok).mock(return_value=_html()) + bad = respx.get(down).mock(side_effect=httpx.ConnectError("refused")) + skipped = respx.get(long).mock(return_value=_html()) + out = await _lengthen(articles) + + assert (good.call_count, bad.call_count) == (1, 1) + assert not skipped.called + assert [a.url for a in out] == [ok, down, long] + assert "Paragraph 11" in out[0].content + assert out[1].content == TEASER + assert out[2].content == "y" * WINDOW + + +@pytest.mark.parametrize("charset", ["x-bogus", "none"]) +async def test_a_page_with_an_unknown_charset_costs_the_others_nothing(charset: str) -> None: + ok, odd = "https://pjwu.test/writing/ok/", "https://pjwu.test/writing/odd/" + async with respx.mock: + good = respx.get(ok).mock(return_value=_html()) + unknown = respx.get(odd).mock( + return_value=httpx.Response( + 200, + content=_page().encode(), + headers={"Content-Type": f"text/html; charset={charset}"}, + ) + ) + out = await _lengthen([_article(ok), _article(odd)]) + + assert (good.call_count, unknown.call_count) == (1, 1) + assert "Paragraph 11" in out[0].content + # Read as utf-8, the page's own body is still there to use. + assert "Paragraph 11" in out[1].content + + +async def test_an_extractor_that_raises_costs_the_others_nothing(monkeypatch) -> None: + ok, odd = "https://pjwu.test/writing/ok/", "https://pjwu.test/writing/odd/" + real = article_page.page_text + + def page_text(markup): + if "Odd one out" in markup: + raise RuntimeError("lxml gave up") + return real(markup) + + monkeypatch.setattr(article_page, "page_text", page_text) + async with respx.mock: + good = respx.get(ok).mock(return_value=_html()) + broken = respx.get(odd).mock(return_value=_html(f"Odd one out. {BODY}")) + out = await _lengthen([_article(odd), _article(ok)]) + + assert (good.call_count, broken.call_count) == (1, 1) + assert out[0].content == TEASER + assert "Paragraph 11" in out[1].content + + +async def test_the_articles_handed_in_are_not_changed() -> None: + url = "https://pjwu.test/writing/others-standards/" + article = _article(url) + async with respx.mock: + page = respx.get(url).mock(return_value=_html()) + await _lengthen([article]) + + assert page.call_count == 1 + assert article.content == TEASER diff --git a/tests/test_grade_d_no_defaults.py b/tests/test_grade_d_no_defaults.py index 305ab1e7..67710a2d 100644 --- a/tests/test_grade_d_no_defaults.py +++ b/tests/test_grade_d_no_defaults.py @@ -162,6 +162,7 @@ def test_the_example_config_builds_every_settings_table(): def _grade_d_parameters(): + from cyris.adapters.fetch import article_page from cyris.service_layer import ( cluster_news, digest_pipeline, @@ -198,6 +199,7 @@ def _grade_d_parameters(): (prompts.build_filter_system_prompt, "style_prompt"), (prompts.build_summarize_system_prompt, "style_prompt"), (prompts.build_news_cluster_system_prompt, "style_prompt"), + (article_page.lengthen_articles, "summarize_snippet_length"), ] diff --git a/tests/test_html_text.py b/tests/test_html_text.py index d693debe..26b668f6 100644 --- a/tests/test_html_text.py +++ b/tests/test_html_text.py @@ -1,9 +1,9 @@ -"""Tests for `plain_text`, the feed-entry HTML to body-text conversion.""" +"""Tests for `plain_text` and `page_text`, feed-entry and web-page HTML to body text.""" import pytest from captioned_post import FIRST_SENTENCE, POST_HTML -from cyris.adapters.fetch.html_text import plain_text +from cyris.adapters.fetch.html_text import page_text, plain_text pytestmark = pytest.mark.unit @@ -84,3 +84,21 @@ def older_parser_feed(self, data): monkeypatch.setattr("cyris.adapters.fetch.html_text._TextCollector.feed", older_parser_feed) assert plain_text("

Tom & Jerry

") == "Tom & Jerry" + + +def test_a_page_with_no_article_reads_as_no_text_not_its_tags(): + """A whole page has no fallback: stripped, its markup would be navigation and footer.""" + page = "" + + assert plain_text(page) != "" + assert page_text(page) == "" + + +def test_a_page_given_as_bytes_is_decoded_by_its_own_meta_charset(): + body = "這一段正文說明了別人制定的標準，如何塑造我們選擇去做的工作。" * 6 + page = ( + '' + f"

{body}

" + ).encode("big5") + + assert body[:30] in page_text(page) From 58b34999c0e5d193ca38baa305055ef3abf0e634 Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 3 Nov 2026 14:10:16 +0800 Subject: [PATCH 2/4] feat(run): read teaser pages once per run, before the pipeline Deps.lengthen_articles is an optional callable that run_digest calls once on the articles a run keeps: after the cap and the vote-similarity holds, before DigestPipeline. Page text is used in that run only and is never stored; scoring has already read the feed text. A failure is logged and the run carries on with feed text. Rejected: reading pages inside a FetchSource, which would store page text in stored_articles and fetch the whole buffer before the cap; and wrapping the ArticleRepository, which would hide outbound fetches behind a store read. The owner chose this call site on 2026-11-03. build_deps wires it for `cyris run` (dry runs included) and for a `regenerate` rerun, which summarizes again and would otherwise rebuild an issue from teasers; rerender and the comparison commands get none. Co-Authored-By: Claude Opus 5.5 --- src/cyris/bootstrap.py | 23 ++++- src/cyris/entrypoints/cli.py | 6 +- src/cyris/service_layer/run_digest.py | 12 +++ tests/e2e/fakes.py | 13 ++- tests/e2e/harness.py | 9 +- tests/test_article_page_wiring.py | 132 ++++++++++++++++++++++++++ tests/test_cli_flows.py | 10 +- tests/test_e2e_digest.py | 14 +++ tests/test_run_digest.py | 86 +++++++++++++++++ 9 files changed, 296 insertions(+), 9 deletions(-) create mode 100644 tests/test_article_page_wiring.py diff --git a/src/cyris/bootstrap.py b/src/cyris/bootstrap.py index db8a55f7..5e66e3d9 100644 --- a/src/cyris/bootstrap.py +++ b/src/cyris/bootstrap.py @@ -2,7 +2,7 @@ import json import os -from collections.abc import Callable +from collections.abc import Awaitable, Callable from dataclasses import dataclass, field from functools import cache, partial from importlib.resources import files @@ -19,6 +19,7 @@ from cyris.adapters.store import ArticleStore from cyris.adapters.workers_ai_client import WorkersAIClient from cyris.config import Config, LLMProviderConfig +from cyris.domain.models import Article from cyris.service_layer.ports import ( ArticleRepository, AskClef, @@ -376,6 +377,11 @@ class Deps: send_discord_alert: Callable[..., Any] = send_discord_alert # None without a Workers AI (or embedding) token and an account id. ask_clef: AskClef | None = None + # Takes the run's articles and `summarize_snippet_length`, and returns them with + # each RSS teaser on its page's longer body, for this run only. None unless + # `build_deps` was asked for it: `run` (a `--dry-run` too) and a `regenerate` + # rerun fetch pages; a rerender and the comparison commands fetch none. + lengthen_articles: Callable[..., Awaitable[list[Article]]] | None = None def build_promotion_sync( @@ -441,10 +447,16 @@ def build_feed_checker() -> Any: def build_deps( - cfg: Config, on_progress: Callable[[str], None] | None = None, dry_run: bool = False + cfg: Config, + on_progress: Callable[[str], None] | None = None, + dry_run: bool = False, + fetch_pages: bool = False, ) -> Deps: """Wire the run's IO. `dry_run` decides it here, not deeper. + `fetch_pages` wires `lengthen_articles`; only a digest run asks for it, so a + command that merely builds the run's wiring sends no request to a publisher. + A preview skips the writes inside `run_digest`, but two of the edges wired below consume rather than read: the newsletter queue deletes what it hands over, and a vote sync stamps human verdicts before draining its own queue. @@ -546,6 +558,12 @@ def build_deps( topic_store = build_topic_store(cfg, d1) sync = None if dry_run else build_promotion_sync(cfg, store, topic_store) + lengthen_articles = None + if fetch_pages: + from cyris.adapters.fetch.article_page import lengthen_articles as _lengthen + + lengthen_articles = partial(_lengthen, sources=cfg.sources) + return Deps( cfg=cfg, store=store, @@ -577,4 +595,5 @@ def build_deps( send_email=build_send_email(), send_email_alert=build_send_email_alert(), ask_clef=build_ask_clef(), + lengthen_articles=lengthen_articles, ) diff --git a/src/cyris/entrypoints/cli.py b/src/cyris/entrypoints/cli.py index 51bb1261..ab6df3e1 100644 --- a/src/cyris/entrypoints/cli.py +++ b/src/cyris/entrypoints/cli.py @@ -261,7 +261,7 @@ def run( if output is not None: typer.echo(f"Store backend: {cfg.app.store.backend}") try: - deps = build_deps(cfg, on_progress=typer.echo, dry_run=dry_run) + deps = build_deps(cfg, on_progress=typer.echo, dry_run=dry_run, fetch_pages=True) except Exception as e: if if_due and not dry_run: _alert_could_not_start(cfg, e) @@ -352,7 +352,9 @@ def regenerate( raise typer.Exit(1) rerun = mode == "rerun" - deps = build_deps(cfg, on_progress=typer.echo if rerun else None, dry_run=True) + deps = build_deps( + cfg, on_progress=typer.echo if rerun else None, dry_run=True, fetch_pages=rerun + ) if publish and deps.publish_site is None: typer.echo("Cannot publish: publishing is not configured", err=True) raise typer.Exit(1) diff --git a/src/cyris/service_layer/run_digest.py b/src/cyris/service_layer/run_digest.py index 8a925f31..76d15c6e 100644 --- a/src/cyris/service_layer/run_digest.py +++ b/src/cyris/service_layer/run_digest.py @@ -689,6 +689,18 @@ def persist_tags(url_to_tags) -> None: summary["status"] = "no_pending" return RunReport(status="no_pending", failed_sources=failed_sources) + # After scoring and the holds, so only what this issue summarizes is fetched and + # the score stays on the feed's text; the longer text is never saved. + if deps.lengthen_articles is not None: + try: + digest_articles = await deps.lengthen_articles( + digest_articles, + summarize_snippet_length=cfg.app.digest.summarize_snippet_length, + ) + except Exception as e: + logger.error("Reading article pages failed, the run keeps feed text: %s", e) + summary["article_pages_error"] = str(e) + # Process all articles through digest pipeline digest_pipeline = DigestPipeline( deps.llm, diff --git a/tests/e2e/fakes.py b/tests/e2e/fakes.py index 1fbd53c4..3ca395d5 100644 --- a/tests/e2e/fakes.py +++ b/tests/e2e/fakes.py @@ -22,6 +22,8 @@ finds in the system prompt, from rules the test scripts (see `_llm_answer`). - `rss_articles`, `newsletter_*`, `promote_*`: the three Workers cyris pulls from, on the hosts the test configured, as `workers/*/src/index.js` answers. +- `article_page`: a feed article's own page, on the hosts of the scenario's RSS rows. + Every page is gone (404), so a run keeps each article's feed text. Every route refuses a missing or wrong credential, as its service does: the script names the one header and value each route requires, and Discord's webhook path. @@ -189,17 +191,19 @@ def _routes(self): ("newsletter_ack", "POST", "newsletter_host", r"/ack", self.newsletter_ack), ("promote_list", "GET", "promote_host", r"/promotions", self.promotions), ("promote_ack", "POST", "promote_host", r"/ack", self.promote_ack), + ("article_page", "GET", "article_hosts", r"/.*", self.article_page), ] def _claim(self, request): - """The route that answers this request, or None. A host key names a scripted host.""" + """The route that answers this request, or None. A host key names a scripted host, + or a list of them.""" dropped = set(self.script.get("drop", [])) for name, method, host, path, handler in self.routes: - host = self.script["hosts"].get(host, host) + hosts = self.script["hosts"].get(host, host) if ( name not in dropped and request.method == method - and request.pretty_host == host + and request.pretty_host in (hosts if isinstance(hosts, list) else [hosts]) and re.fullmatch(path, request.path.split("?", 1)[0]) ): return name, handler @@ -385,6 +389,9 @@ def rss_articles(self, request, entry): # The Worker binds the limit into SQL, and SQLite reads a negative one as none. return _json(200, rows if limit < 0 else rows[:limit]) + def article_page(self, request, entry): + return 404, b"Not found", "text/html" + def newsletters(self, request, entry): return _json(200, [_worker_item(Path(p)) for p in self.script["newsletters"]]) diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index 298f1166..bff36135 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -23,6 +23,7 @@ from datetime import UTC, datetime from pathlib import Path from typing import Any +from urllib.parse import urlsplit from fakes import TEST_SETTINGS, SqliteD1 @@ -282,7 +283,13 @@ def _write_script(run_dir: Path, scenario: Scenario) -> Path: "routes_out": str(run_dir / "routes.json"), "ready": str(run_dir / "proxy.ready"), "d1_sqlite": str(run_dir / "d1.sqlite"), - "hosts": HOSTS, + # Where the RSS rows' articles live, which a run asks for a teaser's page. + "hosts": HOSTS + | { + "article_hosts": sorted( + {urlsplit(r["url"]).hostname for r in scenario.rss_rows} + ) + }, "pages_project": PAGES_PROJECT, "pages_upload_jwt": PAGES_UPLOAD_JWT, "worker_domains": WORKER_DOMAINS, diff --git a/tests/test_article_page_wiring.py b/tests/test_article_page_wiring.py new file mode 100644 index 00000000..45bcd5c1 --- /dev/null +++ b/tests/test_article_page_wiring.py @@ -0,0 +1,132 @@ +"""Only a digest run is wired to read teaser articles' pages. + +`build_deps` leaves `lengthen_articles` unset unless asked, and the CLI asks for +it on `run` (a `--dry-run` too) and on a rerun alone: a rerender and the +comparison commands read no page. +""" + +from datetime import UTC, datetime +from pathlib import Path + +import httpx +import pytest +import respx +from fakes import make_config +from typer.testing import CliRunner + +from cyris import bootstrap +from cyris.config import AgentVaultConfig, HtmlOutputConfig, SourceConfig, StoreConfig +from cyris.domain.models import Article, Lane +from cyris.entrypoints.cli import app + +pytestmark = pytest.mark.integration + +runner = CliRunner() +FEED = SourceConfig(name="Teaser Feed", url="https://pjwu.test/rss.xml") +PAGE = "https://pjwu.test/writing/others-standards/" +BODY = " ".join(f"Paragraph {i} of the article's own page goes on at length." for i in range(30)) + + +def _config(tmp_path: Path, *, d1: bool = False): + store = ( + StoreConfig(backend="d1", database_id="db-ID-1", account_id="acct", api_token="tok") + if d1 + else StoreConfig() + ) + return make_config( + store=store, + agent_vault=AgentVaultConfig(path=tmp_path / "vault"), + html_output=HtmlOutputConfig(enabled=True, output_dir=str(tmp_path / "html")), + sources={FEED.name: FEED}, + ) + + +def _teaser() -> Article: + return Article( + id=PAGE, + title="Others' standards", + url=PAGE, + content="One sentence.", + published_at=datetime(2026, 11, 3, tzinfo=UTC), + source_name=FEED.name, + source_lane=Lane.READ, + ) + + +def test_a_deps_built_without_asking_reads_no_page(tmp_path: Path) -> None: + assert bootstrap.build_deps(_config(tmp_path)).lengthen_articles is None + + +async def test_a_deps_asked_for_pages_reads_a_teaser_of_the_configured_feeds( + tmp_path: Path, +) -> None: + lengthen = bootstrap.build_deps(_config(tmp_path), fetch_pages=True).lengthen_articles + async with respx.mock: + page = respx.get(PAGE).mock( + return_value=httpx.Response( + 200, + text=f"

{BODY}

", + headers={"Content-Type": "text/html"}, + ) + ) + (out,) = await lengthen([_teaser()], summarize_snippet_length=1000) + + assert page.call_count == 1 + assert "Paragraph 29" in out.content + + +class _DepsBuiltError(Exception): + pass + + +@pytest.fixture +def asked(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> list[dict]: + """The keywords each command passes `build_deps`, which then stops the command.""" + calls: list[dict] = [] + cfg = _config(tmp_path, d1=True) + monkeypatch.setattr(bootstrap, "load_effective_config", lambda *_: cfg) + + def record(*_args, **kwargs): + calls.append(kwargs) + raise _DepsBuiltError + + monkeypatch.setattr(bootstrap, "build_deps", record) + return calls + + +def _invoke(tmp_path: Path, *args: str): + files = ["--config", str(tmp_path / "c.toml"), "--sources", str(tmp_path / "s.yaml")] + return runner.invoke(app, [*args, *files]) + + +@pytest.mark.parametrize( + "args", + [ + ["run"], + ["run", "--dry-run"], + ["regenerate", "2026-11-03", "morning", "--mode", "rerun", "--output", "o.html"], + ], + ids=["run", "dry-run", "rerun"], +) +def test_a_digest_run_asks_for_pages(tmp_path: Path, asked: list[dict], args) -> None: + result = _invoke(tmp_path, *args) + + assert isinstance(result.exception, _DepsBuiltError), result.output + assert [call.get("fetch_pages") for call in asked] == [True] + + +@pytest.mark.parametrize( + "args", + [ + ["regenerate", "2026-11-03", "morning", "--mode", "rerender", "--output", "o.html"], + ["vote-sim"], + ["embed-compare"], + ["llm-compare", "--arm", "gemini:gemini-test"], + ], + ids=["rerender", "vote-sim", "embed-compare", "llm-compare"], +) +def test_no_other_command_asks_for_pages(tmp_path: Path, asked: list[dict], args) -> None: + result = _invoke(tmp_path, *args) + + assert isinstance(result.exception, _DepsBuiltError), result.output + assert [bool(call.get("fetch_pages")) for call in asked] == [False] diff --git a/tests/test_cli_flows.py b/tests/test_cli_flows.py index 278f6b90..79ce07ef 100644 --- a/tests/test_cli_flows.py +++ b/tests/test_cli_flows.py @@ -6,7 +6,9 @@ from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +import respx from fakes import settings_toml from typer.testing import CliRunner @@ -133,7 +135,12 @@ def test_run_happy_path(self, e2e_config: tuple[Path, Path, Path, Path]) -> None ], } - with _patch_feeds(feeds): + with _patch_feeds(feeds), respx.mock: + # Every article is a teaser, so the run asks for each one's page once; + # a 404 keeps the feed's text. + pages = respx.get(url__regex=r"https://(reuters|stratechery)\.com/.+").mock( + return_value=httpx.Response(404) + ) # Mock Claude API for news clustering cluster_response = _make_claude_response( json.dumps( @@ -200,6 +207,7 @@ def test_run_happy_path(self, e2e_config: tuple[Path, Path, Path, Path]) -> None # Assertions assert result.exit_code == 0, f"Run failed: {result.stdout}" + assert pages.call_count == 3 assert "HTML digest written to" in result.stdout # Verify digest file exists (date comes from the real clock) diff --git a/tests/test_e2e_digest.py b/tests/test_e2e_digest.py index 91c0370a..8b6f7d38 100644 --- a/tests/test_e2e_digest.py +++ b/tests/test_e2e_digest.py @@ -330,6 +330,8 @@ def receipts(run) -> Receipts: "newsletter_ack": 1, "promote_list": 1, "promote_ack": 1, + # Every RSS article is shorter than the summarize window, so each one's page is asked. + "article_page": len(ARTICLES), } # Derived from the code paths of one first publish, in the order they run: @@ -416,6 +418,18 @@ def test_d1_receives_exactly_these_statements_in_this_order(receipts) -> None: assert [statement_kind(s["sql"]) for s in d1_statements(receipts.records)] == EXPECTED_D1 +def test_each_rss_teasers_page_is_asked_once_under_cyris_own_name(receipts) -> None: + """The letters' pages are never asked: their host is no route's, so one would be unclaimed.""" + pages = by_route(receipts.records, "article_page") + + assert sorted(f"https://{r['host']}{r['path']}" for r in pages) == sorted( + a["url"] for a in ARTICLES.values() + ) + for r in pages: + assert "cyris" in r["headers"]["user-agent"] + assert "mozilla" not in r["headers"]["user-agent"].lower() + + def _within_run(r: "Receipts", iso: str) -> bool: return r.run.started_at <= datetime.fromisoformat(iso) <= r.run.finished_at diff --git a/tests/test_run_digest.py b/tests/test_run_digest.py index 63258afe..c321a3e8 100644 --- a/tests/test_run_digest.py +++ b/tests/test_run_digest.py @@ -3263,3 +3263,89 @@ def test_deps_carries_no_cutoff_field() -> None: from dataclasses import fields assert "embedding_threshold" not in {f.name for f in fields(Deps)} + + +def _teaser_deps(tmp_path: Path, articles: list[Article], llm: FakeLLM | None = None): + """Deps whose `lengthen_articles` prefixes each article it is handed, and records them.""" + handed: list[tuple[list[str], int]] = [] + + async def lengthen(articles, *, summarize_snippet_length): + handed.append(([a.url for a in articles], summarize_snippet_length)) + return [a.model_copy(update={"content": f"PAGE BODY {a.content}"}) for a in articles] + + deps, _ = make_deps(tmp_path, llm or FakeLLM(), FakeSource(articles)) + recorded: list[dict] = [] + deps = replace(deps, lengthen_articles=lengthen, record_run=recorded.append) + return deps, handed, recorded + + +async def test_a_teaser_is_summarized_from_its_page_but_scored_and_stored_as_its_feed( + tmp_path: Path, +) -> None: + article = Article( + id=1, + title="Enterprise AI Adoption", + url="https://example.com/ai", + content="Enterprises accelerate AI adoption.", + published_at=datetime.now(UTC) - timedelta(hours=1), + source_name="TechSource", + source_lane=Lane.READ, + source_tags=["tech"], + ) + llm = FakeLLM( + [ + json.dumps({"scores": [{"id": 1, "score": 85, "language": "en"}]}), + json.dumps({"sections": []}), + ] + ) + deps, handed, _ = _teaser_deps(tmp_path, [article], llm) + + report = await run_digest(deps, RunOptions()) + + assert report.status == "ok" + assert handed == [(["https://example.com/ai"], deps.cfg.app.digest.summarize_snippet_length)] + scoring, summarize = llm.calls[0]["prompt"], llm.calls[-1]["prompt"] + assert "Enterprises accelerate" in scoring and "PAGE BODY" not in scoring + assert "PAGE BODY Enterprises accelerate" in summarize + [stored] = deps.store.get_by_urls(["https://example.com/ai"]) + assert stored.content == "Enterprises accelerate AI adoption." + + +async def test_only_the_articles_left_after_the_cap_and_the_holds_are_lengthened( + tmp_path: Path, +) -> None: + articles = [ + _published("newest", "S", 1), + _published("held", "S", 2), + _published("capped", "S", 30), + ] + deps, handed, _ = _teaser_deps(tmp_path, articles) + deps.cfg.app.digest.max_articles_per_digest = 2 + deps.cfg.app.vote_similarity.enabled = True + + class Holds: + def active(self, at): + return {"https://example.com/held": object()} + + deps = replace(deps, suppressions=Holds()) + + await run_digest(deps, RunOptions()) + + assert [urls for urls, _ in handed] == [["https://example.com/newest"]] + + +async def test_a_failed_lengthening_keeps_the_feed_text_and_the_run(tmp_path: Path) -> None: + async def broken(articles, *, summarize_snippet_length): + raise RuntimeError("pages down") + + article = _published("only", "S", 1).model_copy(update={"source_lane": Lane.READ}) + llm = FakeLLM(json.dumps({"sections": []})) + deps, _, recorded = _teaser_deps(tmp_path, [article], llm) + deps = replace(deps, lengthen_articles=broken) + + report = await run_digest(deps, RunOptions()) + + assert report.status == "ok" + [summary] = recorded + assert summary["article_pages_error"] == "pages down" + assert "only body" in llm.calls[-1]["prompt"] From 51cc61c2ee77c3a59eb378eaac422164205b1fe3 Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 3 Nov 2026 14:10:17 +0800 Subject: [PATCH 3/4] =?UTF-8?q?docs(architecture):=20record=20teaser=20pag?= =?UTF-8?q?e=20reads=20and=20the=20=C2=A78=20exception?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §2 gains the direct-injection row, §3.2 describes what is read and refused, §5 grades the three bounds, and §8 plus CLAUDE.md name the one shape by which IO may be called from the service layer: an optional injected callable whose IO lives in adapters. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 3 ++- docs/architecture.md | 23 ++++++++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 21e4ef61..c10e9d91 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,8 @@ code is worse than none, because it is still trusted. Two more doc rules: ### Where IO goes When adding or swapping IO, work in `adapters/` and `bootstrap.build_deps()` — never touch -`service_layer/` or `domain/` (architecture.md §8). Only a genuine IO boundary gets a Protocol in +`service_layer/` or `domain/` (architecture.md §8), except to call an optional `Deps` callable from +`run_digest` whose IO lives in `adapters/` and whose None changes nothing. Only a genuine IO boundary gets a Protocol in `ports.py`; a single-implementation component is injected directly. Vote similarity suppresses by a relative outlier cutoff, `OUTLIER_MARGIN` in `src/cyris/domain/similarity.py`, not a per-model cosine: a hold lasts `SUPPRESSION_CLOCK_DAYS` (ADR-0017), and ADR-0006 is the history of diff --git a/docs/architecture.md b/docs/architecture.md index edc05c67..23ae1a22 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -187,7 +187,7 @@ test. | Wiring | Targets | Swap difficulty | |---|---|---| | **Via Protocol** (`ports.py`) | `LLMClient`, `ArticleRepository`, `TopicRepository` (`D1TopicStore`, `JsonTopicStore`), `FetchSource`, `Embedder` | **Low** — swapping the implementation never touches the core | -| **Direct injection** (no Protocol) | `HtmlDigestWriter`, `publish`, `sync_promotions`, `append_usage`, `notify`, `send_email`, `send_email_alert`, `D1TagStore`, `D1StoryStore`, `D1DigestStore`, `D1DigestInputStore`, `D1AreaStore`, `D1SourceTitles`, `ask_clef` (`ClefClient`), `page_serves`, `FeedChecker` (the wizard's feed check, `bootstrap.build_feed_checker`) | **Medium** — the core calls them directly; a second backend needs a Protocol first | +| **Direct injection** (no Protocol) | `HtmlDigestWriter`, `publish`, `sync_promotions`, `append_usage`, `notify`, `send_email`, `send_email_alert`, `D1TagStore`, `D1StoryStore`, `D1DigestStore`, `D1DigestInputStore`, `D1AreaStore`, `D1SourceTitles`, `ask_clef` (`ClefClient`), `page_serves`, `FeedChecker` (the wizard's feed check, `bootstrap.build_feed_checker`), `lengthen_articles` (`adapters/fetch/article_page.py`; wired by `build_deps(fetch_pages=True)`, which only `cyris run` and a `regenerate` rerun ask for) | **Medium** — the core calls them directly; a second backend needs a Protocol first | `ports.py`'s rule: *only genuine IO boundaries get a Protocol; single-implementation components are injected directly.* @@ -378,6 +378,21 @@ lives once, as the live-hold query in `adapters/store/suppressions.py`. On D1, e candidate it judged also leaves a row in `vote_similarity_shadow` naming its nearest upvoted and downvoted article and both cosines (§4), a record that changes nothing the run selects. +After the holds and before the pipeline, a run reads each **teaser's page** +(`Deps.lengthen_articles`, `adapters/fetch/article_page.py`): an RSS article whose text is shorter +than `digest.summarize_snippet_length` has its own page fetched and its body extracted with +trafilatura, and the longer of the two texts goes to the pipeline. Scoring has already read the +feed's text, and the page's text is never stored, so a later run or a rerun fetches it again. A page +that fails, times out, answers non-2xx or says less leaves the feed text. On the first address and +every redirect hop, only `http(s)` is asked, and a host written as a non-public IP address (in any +IPv4 spelling a resolver reads, such as `127.1` or `2130706433`) or as `localhost` or a name under +it is refused. Names are not resolved, so a hostname whose DNS points at a private address is still +asked: the owner accepted that risk on 2026-11-03, since the Cloudflare run container has nothing +listening locally and no private network, and resolving names here would break +`docs/spec/outbound-http-honours-environment.md`. An article linking to a video (`media_links` in +`adapters/fetch/keywords.json`) gets no page read: a video page's text is the site's navigation, not the video. Pages are read +eight at a time, ten seconds each, so a run's added time is at most ceil(teasers / 8) × 10s (§5). + **Areas** are the reader's closed set of topics (D1 `areas`, §4, §5). Scoring is offered the active names and returns each article's pick; a name the model suggests because no active area fits is counted on a proposed `areas` row for the reader to confirm, and never changes the scores. After the digest is built, each item gets the name of its first article's pick or else its @@ -549,7 +564,7 @@ table `schema.sql` creates must appear in the rows below, so a new table cannot | Datum | Today | Destination | Notes | |---|---|---|---| -| Article store | **D1 `stored_articles`** | same | `content` is written as plain text for every source (newsletter bodies already were; an RSS entry's HTML is converted by `CloudflareRssSource`, or by `RssSource` on the direct-polling path, through `adapters/fetch/html_text.py`). Rows stored before RSS content was converted keep their HTML: `INSERT OR IGNORE` never rewrites them, and a pending row or a rerun turns them back into articles as they are. `url` PRIMARY KEY is the dedup key; a colliding newsletter issue is re-keyed to `newsletter:{id}`. Besides a run's save, the onboarding votes step (§6) stores each card the reader marked and then accepts or rejects it with `triaged_at`, as a `/labels` answer is written; a card left alone gets no row and stays in the buffer for the first run | +| Article store | **D1 `stored_articles`** | same | `content` is written as plain text for every source (newsletter bodies already were; an RSS entry's HTML is converted by `CloudflareRssSource`, or by `RssSource` on the direct-polling path, through `adapters/fetch/html_text.py`). Rows stored before RSS content was converted keep their HTML: `INSERT OR IGNORE` never rewrites them, and a pending row or a rerun turns them back into articles as they are. `url` PRIMARY KEY is the dedup key; a colliding newsletter issue is re-keyed to `newsletter:{id}`. Besides a run's save, the onboarding votes step (§6) stores each card the reader marked and then accepts or rejects it with `triaged_at`, as a `/labels` answer is written; a card left alone gets no row and stays in the buffer for the first run. A teaser's page text (§3.2) is the run's alone and is never written here | | Tag vocabulary | **D1 `tags`** | same | Normalized tags emitted by clustering and scoring | | Article tags | **D1 `article_tags`** | same | URL-keyed article membership in the tag vocabulary | | Stories | **D1 `stories`** | same | Pre-truncation news clusters, keyed `{date}-{period}-{urlhash}` (content-derived from member URLs), replaced per window | @@ -630,6 +645,7 @@ plan's ceilings, and the priced alternatives — is `docs/hosting-and-cost.md`. | Propose title length | A | `TITLE_MAX_CHARS` = 80 in `adapters/store/source_titles.py` | unchanged — a headline fits in 80 characters; a longer title is mostly a dek the model does not need. `tests/test_area_source_titles.py` holds the value to this row | | Propose title lookback | A | `TITLE_LOOKBACK_DAYS` = 30 in `adapters/store/source_titles.py` | unchanged — the window `feed_health.py`'s `QUIET_DAYS` uses: a source silent that long is quiet, not described. `tests/test_area_source_titles.py` holds the value to this row | | RSS Worker read ceiling | A | `WORKER_ROW_CEILING` in `adapters/fetch/rss_worker_source.py`, mirroring the clamp on `GET /articles` in `workers/rss/src/index.js` | done 2026-10-02 — a memory bound, not the run cap: every row carries full feed content. It is sent by value because the Worker's default without one is lower; `tests/test_rss_worker_source.py` holds the two equal | +| Teaser page reads: 8 at once, 10s per page (redirects and body included), 5 MiB a page, and the User-Agent `cyris (+https://github.com/musingfox/cyris)` | A | `PAGE_CONCURRENCY`, `PAGE_TIMEOUT_SECONDS`, `MAX_PAGE_BYTES`, `USER_AGENT` in `adapters/fetch/article_page.py` | added 2026-11-03 — about a third of an issue's hundred articles are teasers, so eight at once reads them in about five waves and a run's added time stays under a minute, against a run container capped at 900s; a deployment has nothing to tune, and a site that refuses the honest User-Agent leaves the feed text. Which article is a teaser is the grade-D `digest.summarize_snippet_length`, not a constant of its own | | Interest map limits: 60 nodes, layout iterations and margin, drawing box | A | `MAX_INTEREST_NODES` in `domain/interest_graph.py`; `ITERATIONS` and `MARGIN` in `domain/graph_layout.py`; `INTEREST_MAP_WIDTH` and `INTEREST_MAP_HEIGHT` in `adapters/output/html_digest.py` | done 2026-10-17 — display limits, each with its reason beside it in code; a reader sets none of them | | Feed health thresholds: unhealthy at 3 consecutive failed polls, or no stored article for 30 days (never included) | A | `UNHEALTHY_FAILURE_STREAK`, `QUIET_DAYS` in `adapters/store/feed_health.py` | done 2026-10-08 — one Substack 429 is routine, three hourly polls failing in a row is not; feeds publish weekly or monthly, so a shorter silence is noise. `cyris doctor` and `/settings` both judge through `FeedHealth.problems`, so neither copies a number | | Pages publish timing: the 180s budget and 120s run reserve, the 120s verify budget of a publishing rerun, the stage and alias poll counts and intervals, the 20s per-request timeout and a deploy attempt's worst case | A | `adapters/output/publish.py`, `adapters/output/pages_deploy.py` | unchanged — reasons in the comments beside each constant; `tests/test_publish.py` pins the budget against `RUN_SLEEP_AFTER` in `workers/app/src/index.js`, and what the budget does not cover is in *Publishing* (§6) | @@ -1309,4 +1325,5 @@ on main after the change lands, citing the hash it landed as. Across local, container, and cloud, `service_layer/` and `domain/` are untouched. Every difference lives in `adapters/` and in `bootstrap.build_deps()`. That is the payoff of the Protocol + composition-root design, and the reason the D1 store landed without a single line changing in the -pipeline. +pipeline. The one exception is an optional `Deps` callable that `run_digest` calls, its IO in +`adapters/` and None leaving the run unchanged: `lengthen_articles`. From 91ea0adb5883b8431ddcfc5e50c33b7308fb9570 Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 3 Nov 2026 16:13:36 +0800 Subject: [PATCH 4/4] fix(fetch): close each redirect hop of a teaser page unread httpx's follow_redirects reads every redirect's body whole, and decompressed, before the final response reaches the caller, so neither the identity-encoding refusal nor MAX_PAGE_BYTES bounded it: a gzipped 302 could inflate past the cap in memory (Codex review on #152). _last_hop follows the redirects itself, closing each hop without reading it, within the client's max_redirects; the request hook still refuses a private host on every hop. Co-Authored-By: Claude Opus 5.5 --- src/cyris/adapters/fetch/article_page.py | 23 +++++++++++++++++++-- tests/test_article_page.py | 26 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/cyris/adapters/fetch/article_page.py b/src/cyris/adapters/fetch/article_page.py index df332dee..b519966e 100644 --- a/src/cyris/adapters/fetch/article_page.py +++ b/src/cyris/adapters/fetch/article_page.py @@ -94,9 +94,27 @@ async def _refuse_private(request: httpx.Request) -> None: raise _PageRefusedError(f"private address {request.url.host}") +async def _last_hop(client: httpx.AsyncClient, url: str) -> httpx.Response: + """The streamed response at the end of `url`'s redirects, its body not yet read. + + Each redirect is closed unread. httpx's `follow_redirects` reads every hop's + body whole, decompressed, before the caller sees the final response, so no cap + here could count it. + """ + request = client.build_request("GET", url) + for _ in range(client.max_redirects + 1): + response = await client.send(request, stream=True) + if response.next_request is None: + return response + await response.aclose() + request = response.next_request + raise _PageRefusedError(f"more than {client.max_redirects} redirects") + + async def _fetch(client: httpx.AsyncClient, url: str) -> str: """The body text of the page at `url`; raises when there is none to read.""" - async with client.stream("GET", url) as response: + response = await _last_hop(client, url) + try: if not response.is_success: # No retry: a 4xx will not change, and a 5xx falls back to the feed text. raise _PageRefusedError(f"HTTP {response.status_code}") @@ -113,6 +131,8 @@ async def _fetch(client: httpx.AsyncClient, url: str) -> str: if len(body) > MAX_PAGE_BYTES: raise _PageRefusedError(f"larger than {MAX_PAGE_BYTES} bytes") markup = _markup(bytes(body), response.charset_encoding) + finally: + await response.aclose() # Off the event loop: a page can be megabytes of markup. return await asyncio.to_thread(page_text, markup) @@ -186,7 +206,6 @@ def teaser(article: Article) -> bool: outcomes: Counter = Counter() async with httpx.AsyncClient( timeout=PAGE_TIMEOUT_SECONDS, - follow_redirects=True, # Uncompressed, so `MAX_PAGE_BYTES` counts what arrives, not what one chunk # inflates to. headers={"User-Agent": USER_AGENT, "Accept-Encoding": "identity"}, diff --git a/tests/test_article_page.py b/tests/test_article_page.py index be217f96..a3a32943 100644 --- a/tests/test_article_page.py +++ b/tests/test_article_page.py @@ -234,6 +234,32 @@ async def test_an_address_that_is_not_a_public_web_page_is_never_fetched(url: st assert out.content == TEASER +class _WatchedBody(httpx.AsyncByteStream): + def __init__(self) -> None: + self.read = False + + async def __aiter__(self): + self.read = True + yield b"x" * 1024 + + +async def test_a_redirects_own_body_is_never_read() -> None: + # httpx's own redirect following reads each hop's body whole, before any cap. + url = "https://pjwu.test/writing/moved/" + moved = "https://pjwu.test/writing/others-standards/" + hop_body = _WatchedBody() + async with respx.mock: + hop = respx.get(url).mock( + return_value=httpx.Response(302, headers={"Location": moved}, stream=hop_body) + ) + page = respx.get(moved).mock(return_value=_html()) + (out,) = await _lengthen([_article(url)]) + + assert (hop.call_count, page.call_count) == (1, 1) + assert not hop_body.read + assert "Paragraph 11" in out.content + + async def test_a_redirect_to_a_private_address_is_not_followed() -> None: url = "https://pjwu.test/writing/moved/" async with respx.mock: