Skip to content

Commit f20f45a

Browse files
authored
fix: make entity_registry.research() local-only by default (#811)
* fix: make entity_registry.research() local-only by default research() previously called _wikipedia_lookup() unconditionally, sending entity names to en.wikipedia.org on every uncached lookup. This violates the project's local-first and privacy-by-architecture principles documented in CLAUDE.md. Changes: - research() now returns "unknown" for uncached words by default - New allow_network=True parameter required for Wikipedia lookups - Wikipedia 404 now returns "unknown" instead of asserting "person" with 0.70 confidence, preventing entity registry poisoning - Added privacy warning docstring to _wikipedia_lookup() - Added tests for local-only default, opt-in network, 404 handling, and cache-not-persisted-on-local-only behaviour Refs: #809 * fix: improve research() cache read path and deduplicate test mocks - Use .get() instead of .setdefault() for cache reads in research() so the local-only path never mutates _data unnecessarily - Move .setdefault() to the network-write path only - Use result.setdefault() for word/confirmed keys to ensure consistent return shape across all _wikipedia_lookup error paths - Extract duplicated mock_result dict into _MOCK_SAOIRSE_PERSON constant shared by 3 test functions
1 parent f36d04e commit f20f45a

2 files changed

Lines changed: 118 additions & 30 deletions

File tree

mempalace/entity_registry.py

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,12 @@ def _wikipedia_lookup(word: str) -> dict:
178178
Look up a word via Wikipedia REST API.
179179
Returns inferred type (person/place/concept/unknown) + confidence + summary.
180180
Free, no API key, handles disambiguation pages.
181+
182+
**Privacy warning:** This function makes an outbound HTTPS request to
183+
en.wikipedia.org, sending the queried word over the network. It should
184+
only be called when the caller has explicitly opted in via
185+
``allow_network=True`` in :meth:`EntityRegistry.research`. The default
186+
behaviour of ``research()`` is local-only (no network calls).
181187
"""
182188
try:
183189
url = f"https://en.wikipedia.org/api/rest_v1/page/summary/{urllib.parse.quote(word)}"
@@ -244,13 +250,14 @@ def _wikipedia_lookup(word: str) -> dict:
244250

245251
except urllib.error.HTTPError as e:
246252
if e.code == 404:
247-
# Not in Wikipedia — strong signal it's a proper noun (unusual name, nickname)
253+
# Not in Wikipedia — this tells us nothing definitive about
254+
# the word. Return "unknown" so the caller can decide.
248255
return {
249-
"inferred_type": "person",
250-
"confidence": 0.70,
256+
"inferred_type": "unknown",
257+
"confidence": 0.3,
251258
"wiki_summary": None,
252259
"wiki_title": None,
253-
"note": "not found in Wikipedia — likely a proper noun or unusual name",
260+
"note": "not found in Wikipedia",
254261
}
255262
return {"inferred_type": "unknown", "confidence": 0.0, "wiki_summary": None}
256263
except (urllib.error.URLError, OSError, json.JSONDecodeError, KeyError):
@@ -502,20 +509,41 @@ def _disambiguate(self, word: str, context: str, person_info: dict) -> Optional[
502509

503510
# ── Research unknown words ───────────────────────────────────────────────
504511

505-
def research(self, word: str, auto_confirm: bool = False) -> dict:
512+
def research(self, word: str, auto_confirm: bool = False, allow_network: bool = False) -> dict:
506513
"""
507-
Research an unknown word via Wikipedia.
508-
Caches result. If auto_confirm=False, marks as unconfirmed (needs user review).
509-
Returns the lookup result.
514+
Research an unknown word.
515+
516+
By default this is **local-only**: it checks the wiki cache and
517+
returns ``"unknown"`` for uncached words. Pass
518+
``allow_network=True`` to explicitly opt in to an outbound
519+
Wikipedia lookup. This design honours the project's
520+
*local-first, zero API* and *privacy by architecture* principles
521+
— no data leaves the machine unless the caller requests it.
522+
523+
Caches result. If *auto_confirm* is ``False``, marks the entry
524+
as unconfirmed (needs user review).
510525
"""
511-
# Already cached?
512-
cache = self._data.setdefault("wiki_cache", {})
526+
# Check cache (read-only — no mutation when allow_network is False)
527+
cache = self._data.get("wiki_cache", {})
513528
if word in cache:
514529
return cache[word]
515530

531+
if not allow_network:
532+
return {
533+
"inferred_type": "unknown",
534+
"confidence": 0.0,
535+
"wiki_summary": None,
536+
"wiki_title": None,
537+
"word": word,
538+
"confirmed": False,
539+
"note": "network lookup disabled — pass allow_network=True to query Wikipedia",
540+
}
541+
542+
# Network path — ensure wiki_cache key exists before writing
543+
cache = self._data.setdefault("wiki_cache", {})
516544
result = _wikipedia_lookup(word)
517-
result["word"] = word
518-
result["confirmed"] = auto_confirm
545+
result.setdefault("word", word)
546+
result.setdefault("confirmed", auto_confirm)
519547

520548
cache[word] = result
521549
self.save()

tests/test_entity_registry.py

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@
88
EntityRegistry,
99
)
1010

11+
# Shared mock result for Wikipedia person lookup tests
12+
_MOCK_SAOIRSE_PERSON = {
13+
"inferred_type": "person",
14+
"confidence": 0.80,
15+
"wiki_summary": "Saoirse is an Irish given name.",
16+
"wiki_title": "Saoirse",
17+
}
18+
1119

1220
# ── COMMON_ENGLISH_WORDS ────────────────────────────────────────────────
1321

@@ -213,22 +221,49 @@ def test_lookup_ambiguous_word_as_concept(tmp_path):
213221
assert result["type"] == "concept"
214222

215223

216-
# ── research (Wikipedia) — mocked ──────────────────────────────────────
224+
# ── research — local-only by default ───────────────────────────────────
217225

218226

219-
def test_research_caches_result(tmp_path):
227+
def test_research_local_only_by_default(tmp_path):
228+
"""research() must NOT call Wikipedia unless allow_network=True."""
220229
registry = EntityRegistry.load(config_dir=tmp_path)
221230
registry.seed(mode="personal", people=[], projects=[])
222231

223-
mock_result = {
224-
"inferred_type": "person",
225-
"confidence": 0.80,
226-
"wiki_summary": "Saoirse is an Irish given name.",
227-
"wiki_title": "Saoirse",
228-
}
232+
with patch(
233+
"mempalace.entity_registry._wikipedia_lookup",
234+
side_effect=AssertionError("network call should not happen"),
235+
):
236+
result = registry.research("Saoirse")
229237

230-
with patch("mempalace.entity_registry._wikipedia_lookup", return_value=mock_result):
231-
result = registry.research("Saoirse", auto_confirm=True)
238+
assert result["inferred_type"] == "unknown"
239+
assert result["confidence"] == 0.0
240+
assert result["word"] == "Saoirse"
241+
assert "network lookup disabled" in result.get("note", "")
242+
243+
244+
def test_research_with_allow_network(tmp_path):
245+
"""research(allow_network=True) calls Wikipedia and caches result."""
246+
registry = EntityRegistry.load(config_dir=tmp_path)
247+
registry.seed(mode="personal", people=[], projects=[])
248+
249+
with patch(
250+
"mempalace.entity_registry._wikipedia_lookup",
251+
return_value=dict(_MOCK_SAOIRSE_PERSON),
252+
):
253+
result = registry.research("Saoirse", auto_confirm=True, allow_network=True)
254+
assert result["inferred_type"] == "person"
255+
256+
257+
def test_research_caches_result(tmp_path):
258+
"""Once cached via allow_network, subsequent calls use cache without network."""
259+
registry = EntityRegistry.load(config_dir=tmp_path)
260+
registry.seed(mode="personal", people=[], projects=[])
261+
262+
with patch(
263+
"mempalace.entity_registry._wikipedia_lookup",
264+
return_value=dict(_MOCK_SAOIRSE_PERSON),
265+
):
266+
result = registry.research("Saoirse", auto_confirm=True, allow_network=True)
232267
assert result["inferred_type"] == "person"
233268

234269
# Second call should use cache, not call Wikipedia again
@@ -240,24 +275,49 @@ def test_research_caches_result(tmp_path):
240275
assert cached["inferred_type"] == "person"
241276

242277

278+
def test_research_local_only_not_cached(tmp_path):
279+
"""Local-only result for uncached word should NOT be persisted to cache."""
280+
registry = EntityRegistry.load(config_dir=tmp_path)
281+
registry.seed(mode="personal", people=[], projects=[])
282+
283+
registry.research("Xander") # local-only, no network
284+
assert "Xander" not in registry._data.get("wiki_cache", {})
285+
286+
243287
def test_confirm_research_adds_to_people(tmp_path):
244288
registry = EntityRegistry.load(config_dir=tmp_path)
245289
registry.seed(mode="personal", people=[], projects=[])
246290

247-
mock_result = {
248-
"inferred_type": "person",
249-
"confidence": 0.80,
250-
"wiki_summary": "Saoirse is a name",
251-
"wiki_title": "Saoirse",
252-
}
253-
with patch("mempalace.entity_registry._wikipedia_lookup", return_value=mock_result):
254-
registry.research("Saoirse", auto_confirm=False)
291+
with patch(
292+
"mempalace.entity_registry._wikipedia_lookup",
293+
return_value=dict(_MOCK_SAOIRSE_PERSON),
294+
):
295+
registry.research("Saoirse", auto_confirm=False, allow_network=True)
255296

256297
registry.confirm_research("Saoirse", entity_type="person", relationship="friend")
257298
assert "Saoirse" in registry.people
258299
assert registry.people["Saoirse"]["source"] == "wiki"
259300

260301

302+
def test_wikipedia_404_returns_unknown(tmp_path):
303+
"""A 404 from Wikipedia should return 'unknown', not assert 'person'."""
304+
registry = EntityRegistry.load(config_dir=tmp_path)
305+
registry.seed(mode="personal", people=[], projects=[])
306+
307+
mock_result = {
308+
"inferred_type": "unknown",
309+
"confidence": 0.3,
310+
"wiki_summary": None,
311+
"wiki_title": None,
312+
"note": "not found in Wikipedia",
313+
}
314+
with patch("mempalace.entity_registry._wikipedia_lookup", return_value=mock_result):
315+
result = registry.research("Zzxqy", auto_confirm=False, allow_network=True)
316+
317+
assert result["inferred_type"] == "unknown"
318+
assert result["confidence"] < 0.5
319+
320+
261321
# ── extract_people_from_query ───────────────────────────────────────────
262322

263323

0 commit comments

Comments
 (0)