Why-it-belongs: top-up requires all three fields (idempotency fix)

Per Codex: generate_summary treated why_belongs alone as a complete explanation,
but get_explanation requires all three — so a partial older row (e.g. only
why_belongs) would never top up and the page would fall back forever. Now the
fully-cached check requires summary + what_happened + why_matters + why_belongs.
Test covers the partial-row top-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

goodnews/summarize.py

@@ -136,10 +136,13 @@ def generate_summary(conn: sqlite3.Connection, article_id: int, client: LocalMod
     explanation is topped up on the next call (so existing pages gain the section).
     """
     existing = conn.execute(
-        "SELECT summary, why_belongs FROM article_summaries WHERE article_id = ?", (article_id,)
+        "SELECT summary, what_happened, why_matters, why_belongs FROM article_summaries WHERE article_id = ?",
+        (article_id,),
     ).fetchone()
-    if existing and existing["summary"] and existing["why_belongs"]:
-        return existing["summary"]  # summary + a complete explanation already cached
+    # Fully cached only when the explanation is COMPLETE (all three) — matches
+    # get_explanation(), so a partial older row gets topped up on the next call.
+    if existing and existing["summary"] and existing["what_happened"] and existing["why_matters"] and existing["why_belongs"]:
+        return existing["summary"]
     row = conn.execute(
         "SELECT a.title, a.description, a.canonical_url, a.duplicate_of, s.accepted "
         "FROM articles a LEFT JOIN article_scores s ON s.article_id = a.id WHERE a.id = ?",

tests/test_explain.py

@@ -60,3 +60,27 @@ def test_share_renders_structured_or_fallback():
     # no explanation → the single "Why it’s here" reason line is the calm fallback
     fb = share.render_share_page(art, "http://b", summary="sum", explanation=None)
     assert "Why it’s here" in fb and "calm reason" in fb
+
+
+def test_generate_tops_up_partial_explanation(tmp_path, monkeypatch):
+    c = connect(str(tmp_path / "p.db")); init_db(c); _seed_article(c)
+    # an older row: summary + why_belongs only (what_happened/why_matters missing)
+    c.execute("INSERT INTO article_summaries (article_id,summary,why_belongs) VALUES (1,'old summary','b')")
+    c.commit()
+    assert summarize.get_explanation(c, 1) is None       # incomplete → falls back
+
+    calls = {"explain": 0}
+    def fake_explain(client, t, s, b):
+        calls["explain"] += 1
+        return {"what_happened": "w", "why_matters": "m", "why_belongs": "b2"}
+    monkeypatch.setattr(summarize, "explain_article", fake_explain)
+    monkeypatch.setattr(summarize, "_fetch_text", lambda url: "body")
+
+    class FakeClient:
+        model = "test"
+        def chat_text(self, messages): return "should not be used for summary (kept)"
+
+    out = summarize.generate_summary(c, 1, client=FakeClient())
+    assert out == "old summary"                          # existing summary reused, not regenerated
+    assert calls["explain"] == 1                          # explanation WAS topped up
+    assert summarize.get_explanation(c, 1) == {"what_happened": "w", "why_matters": "m", "why_belongs": "b2"}