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`. diff --git a/src/cyris/adapters/fetch/article_page.py b/src/cyris/adapters/fetch/article_page.py new file mode 100644 index 00000000..b519966e --- /dev/null +++ b/src/cyris/adapters/fetch/article_page.py @@ -0,0 +1,226 @@ +"""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 _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.""" + 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}") + 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) + finally: + await response.aclose() + # 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, + # 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/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.py b/tests/test_article_page.py new file mode 100644 index 00000000..a3a32943 --- /dev/null +++ b/tests/test_article_page.py @@ -0,0 +1,438 @@ +"""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}

" + "" + ) + + +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 + + +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: + 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_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_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) 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"]