Stop reader mode passing a site's cookie notice off as the article

Reported live: reader mode on a New Republic feature showed nothing but
the cookie disclaimer, and had cached it.

Reproduced outside the app by running ReaderExtractor's own injected
script (Readability.js and all) against the real page. With the article
body present, extraction is perfect. With the body absent from the DOM at
the moment the script runs, Readability's first pass -- which discards
"unlikely candidates", and the Didomi banner's class is
didomi-notice-banner -- comes back under its own 500-character threshold,
so it retries with that stripping switched off. The banner's single
paragraph is 510 characters, i.e. just over the threshold, so it comes
back as the article with the right title attached.

Nothing on the Kotlin side looked at that. Any non-blank content counted
as ExtractionResult.Success, was written to articles.extractedContent,
and was never reconsidered -- so one bad load became a permanent cookie
disclaimer, which is the "extractedContent cached: true" in the report.

ConsentNotice recognises such a result: a hard plain-text length cap
first (nothing long is ever rejected, however much consent boilerplate it
quotes), then either a known CMP container id -- Readability strips
classes but keeps ids -- or two or more distinct consent phrases.

The accept/retry/give-up decision moves out of the WebView callback into
decideEvalOutcome so it can be tested without a device. A consent notice
is now "not ready yet", which routes it into the retry the extractor
already does for an empty result; if it survives all six attempts that is
what the page serves, and it fails as ExtractionFailure.ConsentInterstitial
rather than being accepted.

A fixed extractor cannot repair what is already on disk, so
loadCachedExtraction drops a cached notice on read and re-extracts, in
both ReaderScreen and LinkReaderScreen. No schema change: the repair
happens when the article is next opened. Reader mode is still never
turned on heuristically -- replacing a discarded notice does not flip it.

The bug report now also carries the last extraction error, without which
a report about bad reader content cannot say whether extraction failed.

Tests: 59 -> 78.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

1 files changed, 89 insertions, 0 deletions

diff --git a/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt b/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt
new file mode 100644
index 0000000..8995e22
--- /dev/null
+++ b/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt
@@ -0,0 +1,89 @@
+package org.terst.feedreader.readability
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * The decision ReaderExtractor makes about each `evaluateJavascript` result:
+ * accept it, inject again because the page probably was not ready, or give up
+ * with a reason. It used to live inline in a WebView callback, which meant the
+ * only way to exercise it was on a device against a live page.
+ *
+ * The case that mattered and was never covered: a result that is perfectly
+ * well-formed and non-blank, and is the page's cookie-consent banner rather than
+ * the article. That counted as a success, got cached, and stayed cached.
+ */
+class EvalOutcomeTest {
+
+    private val maxAttempts = 6
+
+    private fun decide(article: ExtractedArticle?, attempt: Int, raw: String = "\"{}\"") =
+        decideEvalOutcome(article, raw, attempt, maxAttempts)
+
+    private fun article(content: String?, error: String? = null) =
+        ExtractedArticle(title = "t", content = content, textContent = content, error = error)
+
+    @Test
+    fun `real content is accepted on the first attempt`() {
+        val a = article("<p>" + "The actual article body. ".repeat(60) + "</p>")
+        val decision = decide(a, attempt = 1)
+        assertEquals(EvalDecision.Done(ExtractionResult.Success(a)), decision)
+    }
+
+    // Retrying cannot change Readability's opinion of the markup, so a thrown
+    // error is final however early it arrives.
+    @Test
+    fun `a readability error is final immediately`() {
+        val decision = decide(article(null, error = "TypeError: x is null"), attempt = 1)
+        assertEquals(
+            EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.ReadabilityError("TypeError: x is null"))),
+            decision
+        )
+    }
+
+    @Test
+    fun `an undecodable result retries and then reports a bad result`() {
+        assertEquals(EvalDecision.Retry, decide(null, attempt = 1, raw = "null"))
+        val last = decide(null, attempt = maxAttempts, raw = "null")
+        assertEquals(EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.BadResult("null"))), last)
+    }
+
+    @Test
+    fun `blank content retries and then reports no content`() {
+        assertEquals(EvalDecision.Retry, decide(article(""), attempt = 1))
+        assertEquals(
+            EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.NoContent)),
+            decide(article(""), attempt = maxAttempts)
+        )
+    }
+
+    // A cookie banner is exactly what a half-loaded page yields -- the banner is
+    // injected early, the article body may not be in the DOM yet -- so the right
+    // first move is the retry the extractor already does for an empty result.
+    @Test
+    fun `a consent notice retries rather than being accepted`() {
+        val decision = decide(article(ConsentNoticeTest.EXTRACTED_COOKIE_NOTICE), attempt = 1)
+        assertEquals(EvalDecision.Retry, decision)
+    }
+
+    @Test
+    fun `a consent notice that survives every attempt is reported as a failure`() {
+        val decision = decide(article(ConsentNoticeTest.EXTRACTED_COOKIE_NOTICE), attempt = maxAttempts)
+        assertEquals(
+            EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.ConsentInterstitial)),
+            decision
+        )
+    }
+
+    // The failure has to be distinguishable from "no content": one means the page
+    // has no article, the other means the page would not show it without consent.
+    @Test
+    fun `the consent failure says what happened`() {
+        val f: ExtractionFailure = ExtractionFailure.ConsentInterstitial
+        assertTrue(f.userMessage().isNotBlank())
+        assertTrue(f.diagnostic().isNotBlank())
+        assertTrue(f.userMessage().lowercase().contains("cookie") || f.userMessage().lowercase().contains("consent"))
+        assertTrue(f.userMessage() != ExtractionFailure.NoContent.userMessage())
+    }
+}