From 6854d5828cc90bc99dc5f1dc515be5ebd1d52bd4 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Tue, 14 Jul 2026 03:39:41 -0600 Subject: [PATCH] Fix resolved ambiguity fail-closed guarantees Consult persisted ambiguity choices before treating an empty resolution set as no-match so stale or out-of-scope choices fail loudly instead of letting create-on-miss lanes mint duplicates. Keep ambiguity readers read-only, share tier-5 first-word predicate logic, and harden coverage for fail-closed journal snapshots, indexer rescan purity, and speaker discovery candidate side effects. --- .../apps/speakers/tests/test_discovery.py | 28 ++++++++ solstone/think/entities/ambiguities.py | 13 ++-- solstone/think/entities/matching.py | 53 +++++++++++---- tests/test_entity_ambiguities.py | 32 +++++++++ tests/test_journal_index.py | 68 +++++++++++++++++++ tests/test_schedule_schema.py | 2 +- 6 files changed, 173 insertions(+), 23 deletions(-) diff --git a/solstone/apps/speakers/tests/test_discovery.py b/solstone/apps/speakers/tests/test_discovery.py index 3ed0af08a..a110881f1 100644 --- a/solstone/apps/speakers/tests/test_discovery.py +++ b/solstone/apps/speakers/tests/test_discovery.py @@ -288,6 +288,33 @@ def test_identify_ambiguous_name_returns_before_writes(speakers_env): scan_result = discover_unknown_speakers() cluster_id = scan_result["clusters"][0]["cluster_id"] + candidate_path = env.journal / "awareness" / "speaker_candidates.json" + candidate_path.parent.mkdir(parents=True, exist_ok=True) + candidate_path.write_text( + json.dumps( + { + "next_id": 2, + "candidates": [ + { + "cand_id": 1, + "centroid": embeddings[0].astype(float).tolist(), + "n_segments": 2, + "n_intervals": 10, + "total_duration_s": 60.0, + "source_segments": [], + "confirmed_entity": None, + "status": "pending", + } + ], + }, + indent=2, + sort_keys=True, + ) + + "\n", + encoding="utf-8", + ) + candidate_before = candidate_path.read_bytes() + result = identify_cluster(cluster_id, "Sarah") assert result["status"] == "ambiguous" @@ -312,6 +339,7 @@ def test_identify_ambiguous_name_returns_before_writes(speakers_env): ) assert not labels_path.exists() assert not corrections_path.exists() + assert candidate_path.read_bytes() == candidate_before assert load_ambiguities()[0]["normalized_query"] == "sarah" record_ambiguity_choice( diff --git a/solstone/think/entities/ambiguities.py b/solstone/think/entities/ambiguities.py index 7f5095b75..eb1881c3d 100644 --- a/solstone/think/entities/ambiguities.py +++ b/solstone/think/entities/ambiguities.py @@ -127,10 +127,8 @@ def ambiguity_id_for_key(key: str) -> str: def ambiguities_dir() -> Path: - """Return the entity ambiguities directory, creating it if needed.""" - path = Path(get_journal()) / "entities" - path.mkdir(parents=True, exist_ok=True) - return path + """Return the entity ambiguities directory path.""" + return Path(get_journal()) / "entities" def ambiguities_path() -> Path: @@ -192,10 +190,11 @@ def locked_modify_ambiguities( fn: Callable[[list[dict[str, Any]]], list[dict[str, Any]]], ) -> list[dict[str, Any]]: """Apply a locked read-modify-write cycle to ambiguities.jsonl.""" - with hold_lock(ambiguities_path()): - rows = load_ambiguities() + path = ambiguities_path() + with hold_lock(path): + rows = _load_jsonl_rows(path) new_rows = fn(rows) - save_ambiguities(new_rows) + _save_jsonl_rows(path, new_rows) return new_rows diff --git a/solstone/think/entities/matching.py b/solstone/think/entities/matching.py index 4f24f6f2b..2c758f3d3 100644 --- a/solstone/think/entities/matching.py +++ b/solstone/think/entities/matching.py @@ -127,6 +127,28 @@ def _prefix_token_match(name_a_lower: str, name_b_lower: str) -> bool: ) +def _first_word_key(name: str) -> str: + """Return a matchable first-word key for tier-5 matching.""" + first_word = name.split()[0].lower() if name else "" + if len(first_word) < 3: + return "" + return first_word + + +def _first_word_match(query_lower: str, entity_name: str) -> bool: + """True when a query equals an entity name's matchable first word.""" + return len(query_lower) >= 3 and _first_word_key(entity_name) == query_lower + + +def _single_token_first_word_match(query_first: str, entity_name: str) -> bool: + """True when a query first word matches a single-token entity name.""" + return ( + bool(entity_name) + and len(entity_name.split()) == 1 + and _first_word_match(query_first, entity_name) + ) + + def is_name_variant_match(name_a: str, name_b: str) -> bool: """Check if two names are plausible variants of each other. @@ -304,8 +326,8 @@ def find_matching_entity( email_map[email.lower()] = entity # Tier 5: First word - first_word = name.split()[0].lower() if name else "" - if first_word and len(first_word) >= 3: + first_word = _first_word_key(name) + if first_word: if first_word not in first_word_map: first_word_map[first_word] = [] first_word_map[first_word].append(entity) @@ -352,7 +374,7 @@ def find_matching_entity( # "Javier Garcia" → "Javier"). Reject when both names are # multi-token and merely share a first word (e.g., "Person B" # should NOT match "Person A"). - if len(matched_name.split()) == 1: + if _single_token_first_word_match(detected_first, str(matched_name)): return MatchResult(fw_matches[0], MatchTier.FIRST_WORD) # Tier 6: Token-subset match (unambiguous only) @@ -699,14 +721,11 @@ def _collect_low_confidence_candidates( # Tier 5: First-word match without the legacy uniqueness guard. if len(query) >= 3: - first_word_matches: list[EntityDict] = [] - for entity in entities: - name = str(entity.get("name") or "") - if not name: - continue - first_word = name.split()[0].lower() - if len(first_word) >= 3 and first_word == query_lower: - first_word_matches.append(entity) + first_word_matches = [ + entity + for entity in entities + if _first_word_match(query_lower, str(entity.get("name") or "")) + ] if first_word_matches: return MatchTier.FIRST_WORD, _rank_resolution_candidates( query, MatchTier.FIRST_WORD, first_word_matches @@ -717,9 +736,10 @@ def _collect_low_confidence_candidates( long_to_short_matches = [ entity for entity in entities - if entity.get("name") - and len(str(entity.get("name")).split()) == 1 - and str(entity.get("name")).split()[0].lower() == query_first + if _single_token_first_word_match( + query_first, + str(entity.get("name") or ""), + ) ] if long_to_short_matches: return MatchTier.FIRST_WORD, _rank_resolution_candidates( @@ -792,7 +812,7 @@ def record_entity_resolution( Low-confidence matches record an ambiguity row before returning, so callers can reuse their existing unresolved path without risking a later write. """ - if not query or not query.strip() or not entities: + if not query or not query.strip(): return EntityResolution(outcome=EntityResolutionOutcome.NO_MATCH) normalized_query = normalize_resolution_query(query) @@ -816,6 +836,9 @@ def record_entity_resolution( entity=entity, ) + if not entities: + return EntityResolution(outcome=EntityResolutionOutcome.NO_MATCH) + match = find_matching_entity(match_query, entities, fuzzy_threshold) if match and match.is_high_confidence: return EntityResolution( diff --git a/tests/test_entity_ambiguities.py b/tests/test_entity_ambiguities.py index f2274389f..443198f8a 100644 --- a/tests/test_entity_ambiguities.py +++ b/tests/test_entity_ambiguities.py @@ -6,6 +6,7 @@ from __future__ import annotations import ast +import hashlib import inspect import json from importlib import import_module @@ -61,6 +62,19 @@ def _ambiguity_file(journal: Path) -> Path: return journal / "entities" / "ambiguities.jsonl" +def _tree_snapshot(root: Path) -> dict[str, str]: + snapshot: dict[str, str] = {} + for path in sorted(root.rglob("*")): + if path.name.endswith(".lock"): + continue + rel = path.relative_to(root).as_posix() + if path.is_dir(): + snapshot[f"{rel}/"] = "" + elif path.is_file(): + snapshot[rel] = hashlib.sha256(path.read_bytes()).hexdigest() + return snapshot + + def _record( query: str, entities: list[dict], @@ -350,6 +364,22 @@ def test_stale_resolved_choice_fails_loudly_without_fallback(journal: Path) -> N ) +def test_resolved_choice_with_empty_entity_set_fails_loudly( + journal: Path, +) -> None: + entities = [_entity("Sarah Connor"), _entity("Sarah Lee")] + scope = ResolutionScope.journal() + + _record("Sarah", entities, scope=scope) + record_ambiguity_choice("Sarah", "sarah_lee", entities, scope=scope) + + with pytest.raises(EntityAmbiguityError): + resolution = _record("Sarah", [], scope=scope) + assert resolution.outcome != EntityResolutionOutcome.NO_MATCH + + assert not (journal / "entities" / "sarah" / "entity.json").exists() + + def test_lock_timeout_during_persistence_fails_closed( journal: Path, monkeypatch: pytest.MonkeyPatch, @@ -358,10 +388,12 @@ def test_lock_timeout_during_persistence_fails_closed( raise LockTimeout(path, 0.01) monkeypatch.setattr(ambiguities, "hold_lock", raise_timeout) + before = _tree_snapshot(journal) with pytest.raises(LockTimeout): _record("Sarah", [_entity("Sarah Connor"), _entity("Sarah Lee")]) + assert _tree_snapshot(journal) == before assert not _ambiguity_file(journal).exists() diff --git a/tests/test_journal_index.py b/tests/test_journal_index.py index 444317ffc..63cf46586 100644 --- a/tests/test_journal_index.py +++ b/tests/test_journal_index.py @@ -2119,3 +2119,71 @@ def test_scan_journal_is_pure_wrt_entity_state(journal_copy): assert snap_before == snap_between == snap_after, ( "scan_journal() mutated journal/entities/ — see docs/coding-standards.md § L6" ) + + +def test_scan_journal_rescan_does_not_record_entity_ambiguities(journal_copy): + """Index rescans must not create domain ambiguity rows for low-confidence names.""" + from solstone.think.entities import load_ambiguities + from solstone.think.indexer.journal import scan_journal + + journal_path = Path(journal_copy) + for entity_id, name in [ + ("sarah_connor", "Sarah Connor"), + ("sarah_lee", "Sarah Lee"), + ]: + entity_dir = journal_path / "entities" / entity_id + entity_dir.mkdir(parents=True, exist_ok=True) + (entity_dir / "entity.json").write_text( + json.dumps( + { + "id": entity_id, + "name": name, + "type": "Person", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + } + ), + encoding="utf-8", + ) + + today = datetime.now().strftime("%Y%m%d") + segment_dir = ( + journal_path / "chronicle" / today / "default" / "121000_300" / "talents" + ) + segment_dir.mkdir(parents=True) + (segment_dir / "sense.json").write_text( + json.dumps( + { + "density": "active", + "content_type": "meeting", + "activity_summary": "Sarah discussed a low-confidence indexer case.", + "entities": [ + { + "name": "Sarah", + "type": "Person", + "role": "mentioned", + "source": "screen", + "context": "Ambiguous against Sarah Connor and Sarah Lee.", + } + ], + "facets": [ + {"facet": "work", "activity": "search indexing", "level": "high"} + ], + "speculative_facet": None, + "meeting_detected": False, + "speakers": [], + "recommend": {"screen_record": False, "speaker_attribution": False}, + "emotional_register": "focused", + } + ), + encoding="utf-8", + ) + + ambiguity_path = journal_path / "entities" / "ambiguities.jsonl" + assert not ambiguity_path.exists() + + scan_journal(str(journal_path), full=True) + scan_journal(str(journal_path), full=True) + + assert not ambiguity_path.exists() + assert load_ambiguities() == [] diff --git a/tests/test_schedule_schema.py b/tests/test_schedule_schema.py index 264c3decd..bf2105574 100644 --- a/tests/test_schedule_schema.py +++ b/tests/test_schedule_schema.py @@ -171,7 +171,7 @@ def test_schedule_activity_enum_matches_default_activities_drift_detector(): def test_schedule_participation_entry_diverges_from_shared_fragment(): - """Schedule omits entity_id because the hook fills it via find_matching_entity.""" + """Schedule omits entity_id because the hook fills it via record_entity_resolution.""" schedule_schema = _load_schedule_schema() fragment = _load_json(PARTICIPATION_ENTRY_SCHEMA_PATH) -- 2.51.2