From 0f3bdf209d50dd86cae1d05a0a60bc61de4eccaa Mon Sep 17 00:00:00 2001 From: marcos Date: Wed, 5 Aug 2026 15:25:45 +0000 Subject: [PATCH] fix: report wiki failures instead of grounding silently on nothing Every failure mode returned null, which the caller cannot tell apart from a term the wiki has no article for. A 403 would revert /ia to the confidently wrong answers grounding exists to stop, against a clean log. Also clamps maxChars so a config of 0 cannot switch grounding off for good, restores the interrupt flag on disable, and distinguishes a malformed response from an outage in the log. Co-Authored-By: Claude Opus 5 --- .../dev/marcospaulo/canalhandia/Wiki.java | 72 ++++++-- .../dev/marcospaulo/canalhandia/WikiTest.java | 159 ++++++++++++++++++ 2 files changed, 216 insertions(+), 15 deletions(-) diff --git a/src/main/java/dev/marcospaulo/canalhandia/Wiki.java b/src/main/java/dev/marcospaulo/canalhandia/Wiki.java index 748634f..a61a694 100644 --- a/src/main/java/dev/marcospaulo/canalhandia/Wiki.java +++ b/src/main/java/dev/marcospaulo/canalhandia/Wiki.java @@ -1,14 +1,18 @@ package dev.marcospaulo.canalhandia; import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParser; +import java.io.IOException; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; +import java.util.function.Consumer; +import java.util.regex.Pattern; /** * Reads articles from the Portuguese Minecraft Wiki. @@ -24,17 +28,22 @@ import java.util.Map; final class Wiki { private static final String API = "https://pt.minecraft.wiki/api.php"; + /** Entries are small and a server sees few distinct topics; this is generous. */ + private static final int MAX_ENTRIES = 200; + /** {@code [\r\n]} rather than {@code \n}, so a CRLF article collapses too. */ + private static final Pattern BLANK_LINES = Pattern.compile("[\r\n]{2,}"); record Article(String title, String text) { } private final Fetcher fetcher; private final int maxChars; + private final Consumer warn; /** * Keyed by the normalised search term, not by article title: two * terms that resolve to the same article get their own entry, which costs a * duplicate copy of the text and saves a round trip on each. Newest-last, - * evicted at 200 entries. Lost on restart, which is fine. + * evicted at {@value #MAX_ENTRIES} entries. Lost on restart, which is fine. * *

Guarded by its own monitor. {@link Ai} calls {@link #lookup} from * {@code runTaskAsynchronously}, so several players asking at once means @@ -45,8 +54,26 @@ final class Wiki { private final Map cache = new LinkedHashMap<>(); Wiki(Fetcher fetcher, int maxChars) { + this(fetcher, maxChars, message -> { + }); + } + + /** + * @param warn where failures are reported. Grounding is the entire point of + * this class, and every one of its failure modes returns null, which the + * caller cannot tell apart from a question the wiki simply has no article + * for. A 403 — the exact failure the custom user agent exists to prevent — + * would revert {@code /ia} to the confidently wrong answers it was built + * to stop, and do it against a clean server log. Grounding you cannot + * tell is broken is grounding you do not have. + */ + Wiki(Fetcher fetcher, int maxChars, Consumer warn) { this.fetcher = fetcher; - this.maxChars = maxChars; + // A config value of 0 would make every substring throw, and since the + // catch below turns that into null, grounding would switch itself off + // for good and silently. Cheaper to clamp than to diagnose. + this.maxChars = Math.max(1, maxChars); + this.warn = warn; } /** The best article for a search term, or null if there is none. */ @@ -65,39 +92,54 @@ final class Wiki { } String text = extract(title); if (text == null || text.isBlank()) { + warn.accept("Wiki: artigo \"" + title + "\" veio sem texto."); return null; } Article article = new Article(title, trim(text)); remember(key, article); return article; + } catch (InterruptedException e) { + // Swallowing this would leave an async worker running through a + // plugin disable or reload as if nothing had happened. + Thread.currentThread().interrupt(); + warn.accept("Wiki: consulta de \"" + term + "\" interrompida."); + return null; } catch (Exception e) { // A wiki outage must not break the answer; the caller falls back // to answering without a source and says so. + warn.accept("Wiki: falha ao consultar \"" + term + "\": " + e); return null; } } - private String search(String term) throws Exception { + private String search(String term) throws IOException, InterruptedException { String url = API + "?action=query&list=search&format=json&srlimit=1&srsearch=" + URLEncoder.encode(term, StandardCharsets.UTF_8); JsonObject root = JsonParser.parseString(fetcher.get(url)).getAsJsonObject(); - if (!root.has("query")) { - return null; + JsonElement hits = root.has("query") ? root.getAsJsonObject("query").get("search") : null; + // Kept distinct from an outage so the two do not read alike in the log: + // MediaWiki answers some warning shapes with a query object and no + // search key, which would otherwise surface as a bare NullPointerException. + if (hits == null || !hits.isJsonArray()) { + throw new IOException("busca sem 'search': " + AiText.forLog(root.toString())); } - JsonArray hits = root.getAsJsonObject("query").getAsJsonArray("search"); - return hits.isEmpty() ? null : hits.get(0).getAsJsonObject().get("title").getAsString(); + JsonArray array = hits.getAsJsonArray(); + return array.isEmpty() ? null : array.get(0).getAsJsonObject().get("title").getAsString(); } - private String extract(String title) throws Exception { + private String extract(String title) throws IOException, InterruptedException { String url = API + "?action=query&prop=extracts&explaintext=1&format=json&titles=" + URLEncoder.encode(title, StandardCharsets.UTF_8); JsonObject root = JsonParser.parseString(fetcher.get(url)).getAsJsonObject(); - if (!root.has("query")) { - return null; + JsonElement pages = root.has("query") ? root.getAsJsonObject("query").get("pages") : null; + if (pages == null || !pages.isJsonObject()) { + throw new IOException("extrato sem 'pages': " + AiText.forLog(root.toString())); } - JsonObject pages = root.getAsJsonObject("query").getAsJsonObject("pages"); - for (String key : pages.keySet()) { - JsonObject page = pages.getAsJsonObject(key); + JsonObject byPageId = pages.getAsJsonObject(); + for (String key : byPageId.keySet()) { + JsonObject page = byPageId.getAsJsonObject(key); + // A missing page carries "missing" and no extract. That is a plain + // no-result, not a malformed response. if (page.has("extract")) { return page.get("extract").getAsString(); } @@ -106,7 +148,7 @@ final class Wiki { } private String trim(String text) { - String collapsed = text.replaceAll("\n{2,}", "\n").trim(); + String collapsed = BLANK_LINES.matcher(text).replaceAll("\n").trim(); return collapsed.length() > maxChars ? collapsed.substring(0, maxChars) : collapsed; } @@ -123,7 +165,7 @@ final class Wiki { private void remember(String key, Article article) { synchronized (cache) { cache.put(key, article); - while (cache.size() > 200) { + while (cache.size() > MAX_ENTRIES) { cache.remove(cache.keySet().iterator().next()); } } diff --git a/src/test/java/dev/marcospaulo/canalhandia/WikiTest.java b/src/test/java/dev/marcospaulo/canalhandia/WikiTest.java index 048b5bf..181da9c 100644 --- a/src/test/java/dev/marcospaulo/canalhandia/WikiTest.java +++ b/src/test/java/dev/marcospaulo/canalhandia/WikiTest.java @@ -97,4 +97,163 @@ class WikiTest { }, 7000); assertNull(wiki.lookup("Camelo")); } + + // --- Added in review. The stub above matches on URL substrings, so nothing + // yet asserted what the request URLs actually say: dropping explaintext=1 + // would feed the model raw HTML as its source of truth, and switching back + // to raw concatenation would throw on the accented terms that are the norm + // in Portuguese, both while every test above stayed green. --- + + /** Records every requested URL, then answers by URL fragment. */ + private static Fetcher recording(java.util.List seen, Map byUrlFragment) { + return new Fetcher() { + @Override + public String get(String url) { + seen.add(url); + for (Map.Entry e : byUrlFragment.entrySet()) { + if (url.contains(e.getKey())) { + return e.getValue(); + } + } + throw new AssertionError("unexpected url: " + url); + } + + @Override + public String postJson(String url, String json, String bearer) { + throw new UnsupportedOperationException(); + } + }; + } + + private static Fetcher answering(Map byUrlFragment) { + return recording(new java.util.ArrayList<>(), byUrlFragment); + } + + @Test + void percentEncodesAccentedTerms() { + java.util.List seen = new java.util.ArrayList<>(); + Wiki wiki = new Wiki(recording(seen, Map.of( + "list=search", "{\"query\":{\"search\":[{\"title\":\"Poção\"}]}}", + "prop=extracts", "{\"query\":{\"pages\":{\"1\":{\"title\":\"Poção\"," + + "\"extract\":\"Uma poção de cura.\"}}}}")), 7000); + + assertNotNull(wiki.lookup("poção")); + + assertTrue(seen.get(0).contains("srsearch=po%C3%A7%C3%A3o"), seen.get(0)); + assertTrue(seen.get(1).contains("titles=Po%C3%A7%C3%A3o"), seen.get(1)); + } + + @Test + void asksForTheFullPlainTextArticle() { + java.util.List seen = new java.util.ArrayList<>(); + Wiki wiki = new Wiki(recording(seen, Map.of( + "list=search", "{\"query\":{\"search\":[{\"title\":\"Camelo\"}]}}", + "prop=extracts", "{\"query\":{\"pages\":{\"1\":{\"title\":\"Camelo\"," + + "\"extract\":\"texto\"}}}}")), 7000); + + wiki.lookup("Camelo"); + + assertTrue(seen.get(0).contains("srlimit=1"), seen.get(0)); + assertTrue(seen.get(1).contains("explaintext=1"), seen.get(1)); + assertFalse(seen.get(1).contains("exintro"), + "lead paragraphs alone made the model answer 'não tenho certeza'"); + } + + @Test + void cacheIgnoresCase() { + int[] calls = {0}; + Fetcher counting = new Fetcher() { + @Override + public String get(String url) { + calls[0]++; + return url.contains("list=search") + ? "{\"query\":{\"search\":[{\"title\":\"Creeper\"}]}}" + : "{\"query\":{\"pages\":{\"1\":{\"title\":\"Creeper\",\"extract\":\"polvora\"}}}}"; + } + + @Override + public String postJson(String url, String json, String bearer) { + throw new UnsupportedOperationException(); + } + }; + Wiki wiki = new Wiki(counting, 7000); + wiki.lookup("Creeper"); + int afterFirst = calls[0]; + assertNotNull(wiki.lookup("creeper")); + assertEquals(afterFirst, calls[0], "differing case is the same article"); + } + + @Test + void missingPageYieldsNull() { + Wiki wiki = new Wiki(answering(Map.of( + "list=search", "{\"query\":{\"search\":[{\"title\":\"Nada\"}]}}", + "prop=extracts", "{\"query\":{\"pages\":{\"-1\":{\"missing\":\"\"}}}}")), 7000); + assertNull(wiki.lookup("Nada")); + } + + @Test + void nonJsonBodyYieldsNull() { + // What a blocked request really looks like: an HTML error page, HTTP 200. + Wiki wiki = new Wiki(answering(Map.of( + "list=search", "403 Forbidden")), 7000); + assertNull(wiki.lookup("Camelo")); + } + + @Test + void malformedResponseYieldsNull() { + Wiki wiki = new Wiki(answering(Map.of( + "list=search", "{\"query\":{\"warnings\":{}}}")), 7000); + assertNull(wiki.lookup("Camelo")); + } + + @Test + void reportsFailuresSoASilentlyBrokenWikiIsVisible() { + java.util.List warnings = new java.util.ArrayList<>(); + Wiki wiki = new Wiki(new Fetcher() { + @Override + public String get(String url) throws java.io.IOException { + throw new java.io.IOException("403 Forbidden"); + } + + @Override + public String postJson(String url, String json, String bearer) { + throw new UnsupportedOperationException(); + } + }, 7000, warnings::add); + + assertNull(wiki.lookup("Camelo")); + + assertEquals(1, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("Camelo"), warnings.get(0)); + assertTrue(warnings.get(0).contains("403"), warnings.get(0)); + } + + @Test + void clampsNonPositiveMaxChars() { + // A misconfigured 0 would make every substring throw, silently turning + // grounding off for good. + Wiki wiki = new Wiki(answering(Map.of( + "list=search", "{\"query\":{\"search\":[{\"title\":\"T\"}]}}", + "prop=extracts", "{\"query\":{\"pages\":{\"1\":{\"title\":\"T\"," + + "\"extract\":\"texto longo\"}}}}")), 0); + assertEquals(1, wiki.lookup("T").text().length()); + } + + @Test + void restoresTheInterruptFlag() { + Wiki wiki = new Wiki(new Fetcher() { + @Override + public String get(String url) throws InterruptedException { + throw new InterruptedException("disable"); + } + + @Override + public String postJson(String url, String json, String bearer) { + throw new UnsupportedOperationException(); + } + }, 7000); + + assertNull(wiki.lookup("Camelo")); + assertTrue(Thread.interrupted(), "the interrupt must survive the catch"); + } }