Implement merge_concrete_entities function and add some tests

Author: bmerkle

src/typeagent/knowpro/knowledge.py

@@ -2,6 +2,7 @@
 # Licensed under the MIT License.
 
 import asyncio
+from dataclasses import dataclass
 
 from typechat import Result, TypeChatLanguageModel
 
@@ -74,14 +75,80 @@ async def extract_knowledge_from_text_batch(
 def merge_concrete_entities(
     entities: list[kplib.ConcreteEntity],
 ) -> list[kplib.ConcreteEntity]:
-    """Merge a list of concrete entities into a single list of merged entities."""
-    raise NotImplementedError("TODO")
-    # merged_entities = concrete_to_merged_entities(entities)
+    """Merge a list of concrete entities by name, combining types and facets.
+
+    Entities with the same name are merged:
+    - Types are combined into a sorted unique list
+    - Facets with the same name have their values concatenated with "; "
+    """
+    if not entities:
+        return []
+
+    # Build a dict of merged entities keyed by name
+    merged: dict[str, _MergedEntity] = {}
+
+    for entity in entities:
+        name_key = entity.name
+        existing = merged.get(name_key)
+
+        if existing is None:
+            # First occurrence - create new merged entity
+            merged[name_key] = _MergedEntity(
+                name=entity.name,
+                types=set(entity.type),
+                facets=_facets_to_merged(entity.facets) if entity.facets else {},
+            )
+        else:
+            # Merge into existing
+            existing.types.update(entity.type)
+            if entity.facets:
+                for facet in entity.facets:
+                    facet_name = facet.name
+                    facet_value = str(facet.value) if facet.value else ""
+                    existing.facets.setdefault(facet_name, []).append(facet_value)
+
+    # Convert merged entities back to ConcreteEntity
+    result = []
+    for merged_entity in merged.values():
+        concrete = kplib.ConcreteEntity(
+            name=merged_entity.name,
+            type=sorted(merged_entity.types),
+        )
+        if merged_entity.facets:
+            concrete.facets = _merged_to_facets(merged_entity.facets)
+        result.append(concrete)
+
+    return result
+
+
+@dataclass
+class _MergedEntity:
+    """Internal helper for merging entities."""
+
+    name: str
+    types: set[str]
+    facets: dict[str, list[str]]
+
+
+def _facets_to_merged(facets: list[kplib.Facet]) -> dict[str, list[str]]:
+    """Convert a list of Facets to a merged facets dict."""
+    merged: dict[str, list[str]] = {}
+    for facet in facets:
+        name = facet.name
+        value = str(facet.value) if facet.value else ""
+        merged.setdefault(name, []).append(value)
+    return merged
+
 
-    # merged_concrete_entities = []
-    # for merged_entity in merged_entities.values():
-    #     merged_concrete_entities.append(merged_to_concrete_entity(merged_entity))
-    # return merged_concrete_entities
+def _merged_to_facets(merged_facets: dict[str, list[str]]) -> list[kplib.Facet]:
+    """Convert a merged facets dict back to a list of Facets."""
+    facets = []
+    for name, values in merged_facets.items():
+        if values:
+            # Deduplicate values while preserving order
+            unique_values = list(dict.fromkeys(values))
+            facets.append(kplib.Facet(name=name, value="; ".join(unique_values)))
+    return facets
 
 
 def merge_topics(topics: list[str]) -> list[str]:

tests/test_knowledge.py

@@ -10,8 +10,10 @@
     create_knowledge_extractor,
     extract_knowledge_from_text,
     extract_knowledge_from_text_batch,
+    merge_concrete_entities,
     merge_topics,
 )
+from typeagent.knowpro.kplib import ConcreteEntity, Facet
 
 
 class MockKnowledgeExtractor:
@@ -81,3 +83,151 @@ def test_merge_topics():
     assert "topic1" in merged_topics
     assert "topic2" in merged_topics
     assert "topic3" in merged_topics
+
+
+# Tests for merge_concrete_entities
+
+
+def test_merge_concrete_entities_empty_list() -> None:
+    """Test merging an empty list returns empty list."""
+    result = merge_concrete_entities([])
+    assert result == []
+
+
+def test_merge_concrete_entities_single_entity() -> None:
+    """Test merging a single entity preserves case."""
+    entity = ConcreteEntity(name="Alice", type=["Person"])
+    result = merge_concrete_entities([entity])
+
+    assert len(result) == 1
+    assert result[0].name == "Alice"
+    assert result[0].type == ["Person"]
+
+
+def test_merge_concrete_entities_distinct() -> None:
+    """Test merging distinct entities keeps them separate."""
+    entities = [
+        ConcreteEntity(name="Alice", type=["Person"]),
+        ConcreteEntity(name="Bob", type=["Person"]),
+    ]
+    result = merge_concrete_entities(entities)
+
+    assert len(result) == 2
+    names = {e.name for e in result}
+    assert names == {"Alice", "Bob"}
+
+
+def test_merge_concrete_entities_same_name_different_case() -> None:
+    """Test that entities with different case names are NOT merged (case-sensitive)."""
+    entities = [
+        ConcreteEntity(name="Alice", type=["Person"]),
+        ConcreteEntity(name="ALICE", type=["Employee"]),
+        ConcreteEntity(name="alice", type=["Manager"]),
+    ]
+    result = merge_concrete_entities(entities)
+
+    # Case-sensitive: all three are distinct
+    assert len(result) == 3
+    names = {e.name for e in result}
+    assert names == {"Alice", "ALICE", "alice"}
+
+
+def test_merge_concrete_entities_types_deduplicated_and_sorted() -> None:
+    """Test that merged types are deduplicated and sorted."""
+    entities = [
+        ConcreteEntity(name="Alice", type=["Person", "Employee"]),
+        ConcreteEntity(name="Alice", type=["Employee", "Manager"]),
+    ]
+    result = merge_concrete_entities(entities)
+
+    assert len(result) == 1
+    assert result[0].type == ["Employee", "Manager", "Person"]
+
+
+def test_merge_concrete_entities_with_facets() -> None:
+    """Test merging entities with facets."""
+    entities = [
+        ConcreteEntity(
+            name="Alice",
+            type=["Person"],
+            facets=[Facet(name="age", value="30")],
+        ),
+        ConcreteEntity(
+            name="Alice",
+            type=["Employee"],
+            facets=[Facet(name="department", value="Engineering")],
+        ),
+    ]
+    result = merge_concrete_entities(entities)
+
+    assert len(result) == 1
+    assert result[0].facets is not None
+    facet_names = {f.name for f in result[0].facets}
+    assert facet_names == {"age", "department"}
+
+
+def test_merge_concrete_entities_same_facet_combines_values() -> None:
+    """Test that facets with the same name have values combined."""
+    entities = [
+        ConcreteEntity(
+            name="Alice",
+            type=["Person"],
+            facets=[Facet(name="hobby", value="reading")],
+        ),
+        ConcreteEntity(
+            name="Alice",
+            type=["Person"],
+            facets=[Facet(name="hobby", value="swimming")],
+        ),
+    ]
+    result = merge_concrete_entities(entities)
+
+    assert len(result) == 1
+    assert result[0].facets is not None
+    hobby_facet = next(f for f in result[0].facets if f.name == "hobby")
+    assert hobby_facet.value == "reading; swimming"
+
+
+def test_merge_concrete_entities_facets_deduplicated() -> None:
+    """Test that duplicate facet values are deduplicated."""
+    entities = [
+        ConcreteEntity(
+            name="Alice",
+            type=["Person"],
+            facets=[Facet(name="hobby", value="reading")],
+        ),
+        ConcreteEntity(
+            name="Alice",
+            type=["Person"],
+            facets=[Facet(name="hobby", value="reading")],  # Duplicate
+        ),
+        ConcreteEntity(
+            name="Alice",
+            type=["Person"],
+            facets=[Facet(name="hobby", value="swimming")],
+        ),
+    ]
+    result = merge_concrete_entities(entities)
+
+    assert len(result) == 1
+    assert result[0].facets is not None
+    hobby_facet = next(f for f in result[0].facets if f.name == "hobby")
+    assert hobby_facet.value == "reading; swimming"
+
+
+def test_merge_concrete_entities_without_facets_with_facets() -> None:
+    """Test merging an entity without facets with one that has facets."""
+    entities = [
+        ConcreteEntity(name="Alice", type=["Person"]),
+        ConcreteEntity(
+            name="Alice",
+            type=["Employee"],
+            facets=[Facet(name="department", value="Engineering")],
+        ),
+    ]
+    result = merge_concrete_entities(entities)
+
+    assert len(result) == 1
+    assert result[0].facets is not None
+    assert len(result[0].facets) == 1
+    assert result[0].facets[0].name == "department"