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 <noreply@anthropic.com>
This commit is contained in:
@@ -1,14 +1,18 @@
|
|||||||
package dev.marcospaulo.canalhandia;
|
package dev.marcospaulo.canalhandia;
|
||||||
|
|
||||||
import com.google.gson.JsonArray;
|
import com.google.gson.JsonArray;
|
||||||
|
import com.google.gson.JsonElement;
|
||||||
import com.google.gson.JsonObject;
|
import com.google.gson.JsonObject;
|
||||||
import com.google.gson.JsonParser;
|
import com.google.gson.JsonParser;
|
||||||
|
|
||||||
|
import java.io.IOException;
|
||||||
import java.net.URLEncoder;
|
import java.net.URLEncoder;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
import java.util.Locale;
|
import java.util.Locale;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
import java.util.function.Consumer;
|
||||||
|
import java.util.regex.Pattern;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Reads articles from the Portuguese Minecraft Wiki.
|
* Reads articles from the Portuguese Minecraft Wiki.
|
||||||
@@ -24,17 +28,22 @@ import java.util.Map;
|
|||||||
final class Wiki {
|
final class Wiki {
|
||||||
|
|
||||||
private static final String API = "https://pt.minecraft.wiki/api.php";
|
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) {
|
record Article(String title, String text) {
|
||||||
}
|
}
|
||||||
|
|
||||||
private final Fetcher fetcher;
|
private final Fetcher fetcher;
|
||||||
private final int maxChars;
|
private final int maxChars;
|
||||||
|
private final Consumer<String> warn;
|
||||||
/**
|
/**
|
||||||
* Keyed by the normalised <em>search term</em>, not by article title: two
|
* Keyed by the normalised <em>search term</em>, not by article title: two
|
||||||
* terms that resolve to the same article get their own entry, which costs a
|
* 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,
|
* 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.
|
||||||
*
|
*
|
||||||
* <p>Guarded by its own monitor. {@link Ai} calls {@link #lookup} from
|
* <p>Guarded by its own monitor. {@link Ai} calls {@link #lookup} from
|
||||||
* {@code runTaskAsynchronously}, so several players asking at once means
|
* {@code runTaskAsynchronously}, so several players asking at once means
|
||||||
@@ -45,8 +54,26 @@ final class Wiki {
|
|||||||
private final Map<String, Article> cache = new LinkedHashMap<>();
|
private final Map<String, Article> cache = new LinkedHashMap<>();
|
||||||
|
|
||||||
Wiki(Fetcher fetcher, int maxChars) {
|
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<String> warn) {
|
||||||
this.fetcher = fetcher;
|
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. */
|
/** The best article for a search term, or null if there is none. */
|
||||||
@@ -65,39 +92,54 @@ final class Wiki {
|
|||||||
}
|
}
|
||||||
String text = extract(title);
|
String text = extract(title);
|
||||||
if (text == null || text.isBlank()) {
|
if (text == null || text.isBlank()) {
|
||||||
|
warn.accept("Wiki: artigo \"" + title + "\" veio sem texto.");
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
Article article = new Article(title, trim(text));
|
Article article = new Article(title, trim(text));
|
||||||
remember(key, article);
|
remember(key, article);
|
||||||
return 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) {
|
} catch (Exception e) {
|
||||||
// A wiki outage must not break the answer; the caller falls back
|
// A wiki outage must not break the answer; the caller falls back
|
||||||
// to answering without a source and says so.
|
// to answering without a source and says so.
|
||||||
|
warn.accept("Wiki: falha ao consultar \"" + term + "\": " + e);
|
||||||
return null;
|
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="
|
String url = API + "?action=query&list=search&format=json&srlimit=1&srsearch="
|
||||||
+ URLEncoder.encode(term, StandardCharsets.UTF_8);
|
+ URLEncoder.encode(term, StandardCharsets.UTF_8);
|
||||||
JsonObject root = JsonParser.parseString(fetcher.get(url)).getAsJsonObject();
|
JsonObject root = JsonParser.parseString(fetcher.get(url)).getAsJsonObject();
|
||||||
if (!root.has("query")) {
|
JsonElement hits = root.has("query") ? root.getAsJsonObject("query").get("search") : null;
|
||||||
return 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");
|
JsonArray array = hits.getAsJsonArray();
|
||||||
return hits.isEmpty() ? null : hits.get(0).getAsJsonObject().get("title").getAsString();
|
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="
|
String url = API + "?action=query&prop=extracts&explaintext=1&format=json&titles="
|
||||||
+ URLEncoder.encode(title, StandardCharsets.UTF_8);
|
+ URLEncoder.encode(title, StandardCharsets.UTF_8);
|
||||||
JsonObject root = JsonParser.parseString(fetcher.get(url)).getAsJsonObject();
|
JsonObject root = JsonParser.parseString(fetcher.get(url)).getAsJsonObject();
|
||||||
if (!root.has("query")) {
|
JsonElement pages = root.has("query") ? root.getAsJsonObject("query").get("pages") : null;
|
||||||
return null;
|
if (pages == null || !pages.isJsonObject()) {
|
||||||
|
throw new IOException("extrato sem 'pages': " + AiText.forLog(root.toString()));
|
||||||
}
|
}
|
||||||
JsonObject pages = root.getAsJsonObject("query").getAsJsonObject("pages");
|
JsonObject byPageId = pages.getAsJsonObject();
|
||||||
for (String key : pages.keySet()) {
|
for (String key : byPageId.keySet()) {
|
||||||
JsonObject page = pages.getAsJsonObject(key);
|
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")) {
|
if (page.has("extract")) {
|
||||||
return page.get("extract").getAsString();
|
return page.get("extract").getAsString();
|
||||||
}
|
}
|
||||||
@@ -106,7 +148,7 @@ final class Wiki {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private String trim(String text) {
|
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;
|
return collapsed.length() > maxChars ? collapsed.substring(0, maxChars) : collapsed;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -123,7 +165,7 @@ final class Wiki {
|
|||||||
private void remember(String key, Article article) {
|
private void remember(String key, Article article) {
|
||||||
synchronized (cache) {
|
synchronized (cache) {
|
||||||
cache.put(key, article);
|
cache.put(key, article);
|
||||||
while (cache.size() > 200) {
|
while (cache.size() > MAX_ENTRIES) {
|
||||||
cache.remove(cache.keySet().iterator().next());
|
cache.remove(cache.keySet().iterator().next());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -97,4 +97,163 @@ class WikiTest {
|
|||||||
}, 7000);
|
}, 7000);
|
||||||
assertNull(wiki.lookup("Camelo"));
|
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<String> seen, Map<String, String> byUrlFragment) {
|
||||||
|
return new Fetcher() {
|
||||||
|
@Override
|
||||||
|
public String get(String url) {
|
||||||
|
seen.add(url);
|
||||||
|
for (Map.Entry<String, String> 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<String, String> byUrlFragment) {
|
||||||
|
return recording(new java.util.ArrayList<>(), byUrlFragment);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void percentEncodesAccentedTerms() {
|
||||||
|
java.util.List<String> 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<String> 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", "<html><head><title>403 Forbidden</title></head></html>")), 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<String> 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");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user