news archive: fix three bugs found reviewing 301bf90

1. A backfill pass silently wiped teacher identity from live rows.
   The provenance compare is `>=`, and backfill rows are llm-scored but
   carry no teacher_model/teacher_prompt_sha1 (neither source DB recorded
   one), so a bare assignment blanked what the live hook had stored.
   Re-running the backfill — e.g. after importing the live server's
   signals.db, which is the next planned step — would have erased it from
   every live row. COALESCE now keeps a stored identity when the incoming
   one is NULL, while a genuinely newer teacher still replaces it.
   Timestamps clamp for the same reason: backfill stamps historical
   values, so first_seen only moves earlier and last_seen only later.

2. A retried batch could be applied twice. executemany does not roll
   itself back, so a batch that half-landed and then hit a lock was
   re-applied whole by _with_lock_retry, double-counting seen_count on
   the rows that already landed. Writes now run in an explicit BEGIN
   IMMEDIATE with a rollback, which also takes the write lock up front so
   contention fails before any row is applied.

3. The archive hook could break a news fetch. archive_headlines swallows
   its own failures, but its ARGUMENTS are evaluated first, so a raising
   get_llm_client() escaped that guard. Guarded at the call site too:
   archiving is a side effect and must never cost us the headlines we
   just fetched.

Both 1 and 2 were verified to reproduce against 301bf90 before fixing
(teacher_model -> None; seen_count 3 instead of 2 on the rows that
landed pre-failure) and are now regression tests.

Also fixed two weak tests of my own: the disabled-by-config test never
pointed NEWS_ARCHIVE_PATH at its temp dir, so its assertion passed
regardless of behaviour and the write went to the real archive; and the
neutral-headline test asserted the headline was archived without showing
it is dropped downstream, which is the entire reason the hook sits where
it does. Positional binding of the provenance rank is now asserted rather
than trusted, since a miscount would shift every parameter silently.

483 -> 486 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bzv4ETVapFNauFDQht7QtA

news_archive.py

@@ -205,7 +205,11 @@ class NewsArchive:
             " teacher_prompt_sha1, origin"
             ") VALUES (?,?,?,?,?,?,?,?,?,1,?,?,?,?,?,?,?,?)"
             " ON CONFLICT(hash) DO UPDATE SET"
-            "  last_seen = excluded.last_seen,"
+            # The backfill stamps historical timestamps, so clamp rather
+            # than assign: first_seen only ever moves earlier, last_seen
+            # only ever moves later.
+            "  first_seen = MIN(headlines.first_seen, excluded.first_seen),"
+            "  last_seen = MAX(headlines.last_seen, excluded.last_seen),"
             "  seen_count = headlines.seen_count + 1,"
             # Upgrade-only: a weaker scorer never overwrites a stronger
             # one's label, but re-seeing a headline always counts.
@@ -218,10 +222,17 @@ class NewsArchive:
             "           ELSE headlines.currencies_json END,"
             f"  keywords_json = CASE WHEN {wins} THEN excluded.keywords_json"
             "           ELSE headlines.keywords_json END,"
-            f"  teacher_model = CASE WHEN {wins} THEN excluded.teacher_model"
+            # COALESCE, not a bare overwrite: the backfill re-ingests
+            # llm-scored headlines with NO teacher identity (neither source
+            # DB recorded one), and rank >= rank is true, so a plain
+            # assignment blanks what the live hook already stored.
+            f"  teacher_model = CASE WHEN {wins}"
+            "           THEN COALESCE(excluded.teacher_model,"
+            "                         headlines.teacher_model)"
             "           ELSE headlines.teacher_model END,"
             f"  teacher_prompt_sha1 = CASE WHEN {wins}"
-            "           THEN excluded.teacher_prompt_sha1"
+            "           THEN COALESCE(excluded.teacher_prompt_sha1,"
+            "                         headlines.teacher_prompt_sha1)"
             "           ELSE headlines.teacher_prompt_sha1 END,"
             f"  score_source = CASE WHEN {wins} THEN excluded.score_source"
             "           ELSE headlines.score_source END,"
@@ -233,6 +244,15 @@ class NewsArchive:
             "  published_at = COALESCE(excluded.published_at,"
             "                          headlines.published_at)")
 
+        # The rank is bound once per CASE arm. Positional binding means a
+        # miscount silently shifts every parameter, so assert it instead of
+        # trusting the arithmetic: 17 INSERT values + one rank per arm.
+        n_rank = sql.count("? >= ")
+        n_values = 17
+        assert sql.count("?") == n_values + n_rank, (
+            f"placeholder count drifted: {sql.count('?')} "
+            f"!= {n_values} + {n_rank}")
+
         params = []
         for row in rows:
             rank = _SOURCE_RANK.get(row["score_source"], 0)
@@ -244,11 +264,27 @@ class NewsArchive:
                 row["currencies_json"], row["keywords_json"],
                 row["score_source"], row["teacher_model"],
                 row["teacher_prompt_sha1"], row.get("origin", "live"),
-            ) + (rank,) * sql.count("? >="))
+            ) + (rank,) * n_rank)
+
+        def _write() -> None:
+            # BEGIN IMMEDIATE takes the write lock up front, so contention
+            # fails here — before any row is applied — and the rollback
+            # makes the batch atomic if it fails later. Without this a
+            # partial executemany would be re-applied by the retry and
+            # double-count seen_count on the rows that already landed.
+            self._conn.execute("BEGIN IMMEDIATE")
+            try:
+                self._conn.executemany(sql, params)
+                self._conn.commit()
+            except Exception:
+                try:
+                    self._conn.rollback()
+                except Exception:
+                    pass
+                raise
 
         with self._lock:
-            self._with_lock_retry(lambda: self._conn.executemany(sql, params))
-            self._conn.commit()
+            self._with_lock_retry(_write)
         return len(rows)