diff --git a/solstone/apps/entities/call.py b/solstone/apps/entities/call.py index 839931f2a..6c44aaea1 100644 --- a/solstone/apps/entities/call.py +++ b/solstone/apps/entities/call.py @@ -152,7 +152,7 @@ def _candidate_display(candidate: dict) -> str: def _render_edge_resolution_error(body: dict, *, json_output: bool) -> None: if json_output: - typer.echo(json.dumps(body, indent=2, ensure_ascii=False), err=True) + typer.echo(json.dumps(body, indent=2, ensure_ascii=False)) raise typer.Exit(1) query = str(body.get("query") or "entity") @@ -195,7 +195,7 @@ def _format_kinds(kinds: object) -> str: count = info.get("count") if isinstance(info, dict) else None if count: parts.append(f"{kind}:{count}") - return ",".join(parts) + return ", ".join(parts) def _format_seen(item: dict) -> str: @@ -219,7 +219,7 @@ def _format_directed(item: dict) -> str: parts.append(f"out:{out_count}") if in_count: parts.append(f"in:{in_count}") - return f"directed={','.join(parts)}" if parts else "" + return f"directed={', '.join(parts)}" if parts else "" def _format_evidence(row: dict, *, include_source: bool = False) -> str: diff --git a/solstone/apps/entities/routes.py b/solstone/apps/entities/routes.py index d7f9d07aa..787e3abd4 100644 --- a/solstone/apps/entities/routes.py +++ b/solstone/apps/entities/routes.py @@ -275,27 +275,8 @@ def _candidate_payload(entity: EntityDict) -> dict[str, Any]: } -def _candidate_score(query: str, entity: EntityDict) -> float: - try: - from rapidfuzz import fuzz - except ImportError: - return 100.0 - - choices = [str(entity.get("name") or ""), str(entity.get("id") or "")] - aka = entity.get("aka") - if isinstance(aka, list): - choices.extend(str(item) for item in aka if item) - return max((fuzz.token_sort_ratio(query, choice) for choice in choices), default=0) - - -def _candidate_payloads( - query: str, candidates: list[EntityDict] | None -) -> list[dict[str, Any]]: - return [ - _candidate_payload(candidate) - for candidate in candidates or [] - if _candidate_score(query, candidate) >= 50 - ] +def _candidate_payloads(candidates: list[EntityDict] | None) -> list[dict[str, Any]]: + return [_candidate_payload(candidate) for candidate in candidates or []] def _resolution_payload(query: str, candidates: list[EntityDict] | None) -> Any: @@ -303,21 +284,25 @@ def _resolution_payload(query: str, candidates: list[EntityDict] | None) -> Any: { "resolved": None, "query": query, - "candidates": _candidate_payloads(query, candidates), + "candidates": _candidate_payloads(candidates), } ) -def _resolve_edge_entity( - query: str, facet: str | None -) -> tuple[str | None, Any | None]: +class _EdgeResolutionFailed(Exception): + def __init__(self, response: Any) -> None: + super().__init__("edge entity resolution failed") + self.response = response + + +def _resolve_edge_entity(query: str, facet: str | None) -> str: query = query.strip() if not query: - return None, _resolution_payload(query, []) + raise _EdgeResolutionFailed(_resolution_payload(query, [])) exact = load_journal_entity(query) if exact is not None: - return str(exact.get("id") or query), None + return str(exact.get("id") or query) if facet: resolved, candidates = resolve_entity(facet, query) @@ -325,29 +310,30 @@ def _resolve_edge_entity( resolved, candidates = resolve_journal_entity(query) if resolved is None: - return None, _resolution_payload(query, candidates) + raise _EdgeResolutionFailed(_resolution_payload(query, candidates)) entity_id = str(resolved.get("id") or "") if entity_id and load_journal_entity(entity_id) is not None: - return entity_id, None + return entity_id logger.warning("resolved entity outside journal identity space: %r", resolved) - return None, error_response( - ENTITY_OPERATION_FAILED, - detail=f"Resolved entity '{query}' is not a journal entity.", + raise _EdgeResolutionFailed( + error_response( + ENTITY_OPERATION_FAILED, + detail=f"Resolved entity '{query}' is not a journal entity.", + ) ) -def _edge_loader_error(exc: Exception) -> Any | None: +def _edge_loader_error(exc: Exception) -> Any: if isinstance(exc, FileNotFoundError): return error_response(EDGE_INDEX_UNAVAILABLE) if isinstance(exc, sqlite3.OperationalError): - if "no such table: edges" in str(exc): - return error_response(EDGE_INDEX_UNAVAILABLE) - return None + logger.exception("edge index sqlite error") + return error_response(EDGE_INDEX_UNAVAILABLE) if isinstance(exc, ValueError): return error_response(INVALID_REQUEST_VALUE, detail=str(exc)) - return None + raise exc @entities_bp.route("/api/network") @@ -359,10 +345,7 @@ def get_entity_network() -> Any: try: filters = _edge_filters() - entity_id, resolution = _resolve_edge_entity(entity_query, filters["facet"]) - if resolution is not None: - return resolution - assert entity_id is not None + entity_id = _resolve_edge_entity(entity_query, filters["facet"]) return jsonify( load_entity_network( entity_id, @@ -372,11 +355,10 @@ def get_entity_network() -> Any: evidence_limit=_query_int("evidence_limit", 5), ) ) + except _EdgeResolutionFailed as exc: + return exc.response except (FileNotFoundError, sqlite3.OperationalError, ValueError) as exc: - response = _edge_loader_error(exc) - if response is not None: - return response - raise + return _edge_loader_error(exc) @entities_bp.route("/api/history") @@ -388,17 +370,11 @@ def get_entity_history() -> Any: try: filters = _edge_filters() - entity_id, resolution = _resolve_edge_entity(entity_query, filters["facet"]) - if resolution is not None: - return resolution - assert entity_id is not None + entity_id = _resolve_edge_entity(entity_query, filters["facet"]) peer_query = _query_str("peer") if peer_query: - peer_id, resolution = _resolve_edge_entity(peer_query, filters["facet"]) - if resolution is not None: - return resolution - assert peer_id is not None + peer_id = _resolve_edge_entity(peer_query, filters["facet"]) else: principal = get_journal_principal() peer_id = str(principal.get("id") or "") if principal else "" @@ -417,11 +393,10 @@ def get_entity_history() -> Any: offset=_query_int("offset", 0), ) ) + except _EdgeResolutionFailed as exc: + return exc.response except (FileNotFoundError, sqlite3.OperationalError, ValueError) as exc: - response = _edge_loader_error(exc) - if response is not None: - return response - raise + return _edge_loader_error(exc) @entities_bp.route("/api/overview") @@ -435,10 +410,7 @@ def get_network_overview() -> Any: ) ) except (FileNotFoundError, sqlite3.OperationalError, ValueError) as exc: - response = _edge_loader_error(exc) - if response is not None: - return response - raise + return _edge_loader_error(exc) @entities_bp.route("/api/") diff --git a/solstone/apps/entities/talent/entities/SKILL.md b/solstone/apps/entities/talent/entities/SKILL.md index 3a6d3605f..0274c796a 100644 --- a/solstone/apps/entities/talent/entities/SKILL.md +++ b/solstone/apps/entities/talent/entities/SKILL.md @@ -2,9 +2,9 @@ name: entities description: > Tracked entities — people, companies, projects, tools — within facets. - Detect, attach, move, merge, update, alias, search. + Detect, attach, move, merge, update, alias, search, network, history, overview. TRIGGER: entity, person, company, relationship, who is, contact, sol call - entities detect/attach/merge/search. + entities detect/attach/merge/search/network/history/overview. --- # Entities CLI Skill diff --git a/solstone/apps/entities/tests/test_call_edges.py b/solstone/apps/entities/tests/test_call_edges.py index 7764322eb..f9d161c06 100644 --- a/solstone/apps/entities/tests/test_call_edges.py +++ b/solstone/apps/entities/tests/test_call_edges.py @@ -54,51 +54,133 @@ def entities_client_without_index( return journal_copy -def _combined_output(result) -> str: - try: - stderr = result.stderr - except ValueError: - stderr = "" - return result.output + (stderr or "") - - -def test_network_truncated_header_and_evidence(indexed_entities_client) -> None: +def _captured_output(result) -> str: + return result.output + + +def _route_payload(journal: Path, path: str, query_string: dict[str, object]) -> dict: + response = make_test_client(journal).get(path, query_string=query_string) + assert response.status_code == 200 + data = response.get_json() + assert isinstance(data, dict) + return data + + +def _plural(count: int, singular: str, plural: str | None = None) -> str: + word = singular if count == 1 else plural or f"{singular}s" + return f"{count} {word}" + + +def _display_entity(entity_id: object, name: object = None) -> str: + entity_id_text = str(entity_id or "") + name_text = str(name or "") + if name_text and name_text != entity_id_text: + return f"{name_text} ({entity_id_text})" + return entity_id_text + + +def _format_kinds(kinds: object) -> str: + if not isinstance(kinds, dict): + return "" + parts = [] + for kind in sorted(kinds): + info = kinds.get(kind) + count = info.get("count") if isinstance(info, dict) else None + if count: + parts.append(f"{kind}:{count}") + return ", ".join(parts) + + +def test_network_truncated_header_and_evidence(indexed_entities_client: Path) -> None: + payload = _route_payload( + indexed_entities_client, + "/app/entities/api/network", + { + "entity": "romeo_montague", + "limit": "1", + "evidence_limit": "1", + }, + ) result = runner.invoke( entities_app, ["network", "romeo_montague", "--limit", "1", "--evidence-limit", "1"], ) assert result.exit_code == 0 - assert "5 recorded connections for romeo_montague (showing 1):" in result.output - assert "Juliet Capulet (juliet_capulet)" in result.output + total = int(payload["total_neighbors"]) + shown = len(payload["neighbors"]) + assert shown < total + expected_header = ( + f"{_plural(total, 'recorded connection')} for romeo_montague (showing {shown}):" + ) + assert expected_header in result.output + neighbor = payload["neighbors"][0] + evidence = neighbor["evidence"][0] + assert _display_entity(neighbor["entity_id"], neighbor.get("name")) in result.output assert "score=" in result.output assert "kinds=" in result.output assert "seen=" in result.output - assert "20260310 attended-with - Joint Board Meeting" in result.output + assert str(evidence["day"]) in result.output + assert str(evidence["kind"]) in result.output + assert str(evidence["label"]) in result.output -def test_history_truncated_header_and_source_path(indexed_entities_client) -> None: +def test_history_truncated_header_and_source_path( + indexed_entities_client: Path, +) -> None: + payload = _route_payload( + indexed_entities_client, + "/app/entities/api/history", + { + "entity": "juliet_capulet", + "peer": "romeo_montague", + "limit": "1", + }, + ) result = runner.invoke( entities_app, ["history", "juliet_capulet", "romeo_montague", "--limit", "1"], ) assert result.exit_code == 0 - assert ( - "8 evidence rows for juliet_capulet <-> Romeo Montague (romeo_montague) " - "(showing 1-1):" - ) in result.output - assert "20260310 attended-with - Joint Board Meeting" in result.output - assert "[event-legacy] facets/montague/events/20260310.jsonl" in result.output - - -def test_overview_truncated_header(indexed_entities_client) -> None: + total = int(payload["total"]) + shown = len(payload["evidence"]) + assert shown < total + peer = _display_entity(payload["peer_id"], payload.get("peer_name")) + expected_header = ( + f"{_plural(total, 'evidence row')} for juliet_capulet <-> {peer} " + f"(showing 1-{shown}):" + ) + assert expected_header in result.output + evidence = payload["evidence"][0] + assert str(evidence["day"]) in result.output + assert str(evidence["kind"]) in result.output + assert str(evidence["label"]) in result.output + assert f"[{evidence['source']}]" in result.output + assert str(evidence["path"]) in result.output + + +def test_overview_truncated_header(indexed_entities_client: Path) -> None: + payload = _route_payload( + indexed_entities_client, + "/app/entities/api/overview", + {"limit": "1"}, + ) result = runner.invoke(entities_app, ["overview", "--limit", "1"]) assert result.exit_code == 0 - assert "Network overview: 30 edges across 8 entities (showing 1):" in result.output - assert "Kinds: attended-with:25,mentioned:2,spoke-with:3" in result.output - assert "Romeo Montague (romeo_montague)" in result.output + totals = payload["totals"] + shown = len(payload["entities"]) + assert shown < int(totals["entities"]) + expected_header = ( + f"Network overview: {_plural(int(totals['edges']), 'edge')} across " + f"{_plural(int(totals['entities']), 'entity', 'entities')} " + f"(showing {shown}):" + ) + assert expected_header in result.output + assert f"Kinds: {_format_kinds(payload['kinds'])}" in result.output + entity = payload["entities"][0] + assert _display_entity(entity["entity_id"], entity.get("name")) in result.output def test_network_empty_state_exits_zero(indexed_entities_client) -> None: @@ -114,7 +196,7 @@ def test_resolution_failure_names_query_with_candidates( result = runner.invoke(entities_app, ["network", "Jliet"]) assert result.exit_code == 1 - output = _combined_output(result) + output = _captured_output(result) assert "Error: Entity 'Jliet' not found. Did you mean:" in output assert "Juliet Capulet (juliet_capulet)" in output @@ -125,11 +207,31 @@ def test_resolution_failure_names_query_without_candidates( result = runner.invoke(entities_app, ["network", "zzzznotreal"]) assert result.exit_code == 1 - assert "Error: Entity 'zzzznotreal' not found." in _combined_output(result) - assert "Did you mean" not in _combined_output(result) + assert "Error: Entity 'zzzznotreal' not found." in _captured_output(result) + assert "Did you mean" not in _captured_output(result) -def test_history_json_is_route_payload(indexed_entities_client) -> None: +def test_resolution_failure_json_is_route_payload(indexed_entities_client) -> None: + result = runner.invoke(entities_app, ["network", "Jliet", "--json"]) + + assert result.exit_code == 1 + data = json.loads(result.output) + assert data["resolved"] is None + assert data["query"] == "Jliet" + assert data["candidates"] + assert "Error:" not in result.output + + +def test_history_json_is_route_payload(indexed_entities_client: Path) -> None: + expected = _route_payload( + indexed_entities_client, + "/app/entities/api/history", + { + "entity": "juliet_capulet", + "peer": "romeo_montague", + "limit": "1", + }, + ) result = runner.invoke( entities_app, ["history", "juliet_capulet", "romeo_montague", "--limit", "1", "--json"], @@ -138,10 +240,7 @@ def test_history_json_is_route_payload(indexed_entities_client) -> None: assert result.exit_code == 0 data = json.loads(result.output) assert "success" not in data - assert data["entity_id"] == "juliet_capulet" - assert data["peer_id"] == "romeo_montague" - assert data["limit"] == 1 - assert data["total"] == 8 + assert data == expected def test_kinds_bad_kind_detail_survives_real_client( @@ -153,7 +252,7 @@ def test_kinds_bad_kind_detail_survives_real_client( ) assert result.exit_code == 1 - assert "Error: Unknown edge kind: 'bad-kind'" in _combined_output(result) + assert "Error: Unknown edge kind: 'bad-kind'" in _captured_output(result) def test_kinds_accepts_comma_and_repeat_forms(indexed_entities_client) -> None: @@ -184,7 +283,7 @@ def test_unbuilt_index_message_survives_real_client( result = runner.invoke(entities_app, ["overview"]) assert result.exit_code == 1 - output = _combined_output(result) + output = _captured_output(result) assert ( "I couldn't read your connections because the index hasn't been built yet." in output diff --git a/solstone/apps/entities/tests/test_edges_routes.py b/solstone/apps/entities/tests/test_edges_routes.py index 7154db0e1..4b713892c 100644 --- a/solstone/apps/entities/tests/test_edges_routes.py +++ b/solstone/apps/entities/tests/test_edges_routes.py @@ -120,7 +120,7 @@ def test_resolution_failure_returns_200_query_and_candidates(indexed_client): assert {"name", "id", "type"} <= set(data["candidates"][0]) -def test_resolution_failure_can_return_empty_candidates(indexed_client): +def test_resolution_failure_empty_candidates_on_full_miss(indexed_client): response = indexed_client.get( "/app/entities/api/network", query_string={"entity": "zzzznotreal"}, @@ -157,12 +157,12 @@ def test_missing_index_maps_to_edge_index_unavailable(journal_copy: Path): assert "journal indexer --rescan" in data["error"] -def test_pre_edges_index_maps_to_edge_index_unavailable(journal_copy: Path): +def test_malformed_edges_schema_maps_to_edge_index_unavailable(journal_copy: Path): db_path = _index_db(journal_copy) db_path.parent.mkdir(parents=True, exist_ok=True) conn = sqlite3.connect(db_path) try: - conn.execute("CREATE TABLE chunks(id TEXT)") + conn.execute("CREATE TABLE edges(src TEXT)") conn.commit() finally: conn.close() diff --git a/solstone/think/entities/matching.py b/solstone/think/entities/matching.py index 16b3f6c80..23466e47e 100644 --- a/solstone/think/entities/matching.py +++ b/solstone/think/entities/matching.py @@ -536,6 +536,7 @@ def _closest_candidates( query: str, entities: list[EntityDict], limit: int = 3, + min_score: float | None = None, ) -> list[EntityDict]: candidates: list[EntityDict] = [] @@ -560,13 +561,17 @@ def _closest_candidates( limit=limit, ) seen_names: set[str] = set() - for matched_str, _score, _index in results: + for matched_str, score, _index in results: + if min_score is not None and score < min_score: + continue entity = fuzzy_candidates[matched_str] name = entity.get("name", "") if name and name not in seen_names: seen_names.add(name) candidates.append(entity) except ImportError: + if min_score is not None: + return [] candidates = entities[:limit] return candidates @@ -648,4 +653,4 @@ def resolve_journal_entity( if match: return match, None - return None, _closest_candidates(query, entities) + return None, _closest_candidates(query, entities, min_score=50) diff --git a/tests/test_matching.py b/tests/test_matching.py index cfb987ad7..2fa7fde5d 100644 --- a/tests/test_matching.py +++ b/tests/test_matching.py @@ -3,11 +3,14 @@ """Tests for entity matching and name variant resolution.""" +import json + from solstone.think.entities.matching import ( MatchTier, build_name_resolution_map, find_matching_entity, is_name_variant_match, + resolve_entity, resolve_journal_entity, ) @@ -468,3 +471,42 @@ class TestResolveJournalEntity: assert entity is None assert candidates assert any(candidate["id"] == "juliet_capulet" for candidate in candidates) + + def test_returns_empty_candidates_on_full_miss(self): + entity, candidates = resolve_journal_entity("zzzznotreal") + + assert entity is None + assert candidates == [] + + +class TestResolveEntity: + def test_returns_closest_candidates_on_miss(self, tmp_path, monkeypatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + entities = [ + ("alice_johnson", "Alice Johnson"), + ("benvolio_montague", "Benvolio Montague"), + ("charlie_brown", "Charlie Brown"), + ("juliet_capulet", "Juliet Capulet"), + ("mercutio", "Mercutio"), + ("romeo_montague", "Romeo Montague"), + ] + for entity_id, name in entities: + entity_dir = tmp_path / "entities" / entity_id + entity_dir.mkdir(parents=True) + (entity_dir / "entity.json").write_text( + json.dumps({"id": entity_id, "name": name, "type": "Person"}) + ) + relationship_dir = tmp_path / "facets" / "work" / "entities" / entity_id + relationship_dir.mkdir(parents=True) + (relationship_dir / "entity.json").write_text( + json.dumps({"entity_id": entity_id, "description": name}) + ) + + entity, candidates = resolve_entity("work", "Jliet") + + assert entity is None + assert [candidate["id"] for candidate in candidates] == [ + "juliet_capulet", + "alice_johnson", + "charlie_brown", + ]