feat(export): add opt-in metric_errors column to DistanceExporter (#960)

* feat(export): add opt-in metric_errors column to DistanceExporter

Add a 'metric_errors' field to compute_pairs() output that lets
downstream consumers programmatically distinguish legitimate 'no path'
(None) from computation failures (None + error name).

Usage:
    rows = exporter.compute_pairs(include=[..., 'metric_errors'])
    # row['metric_errors'] == '' → all metrics succeeded
    # row['metric_errors'] == 'hop_count,weighted_distance' → those failed

Design decisions:
- Opt-in: column only appears when explicitly requested via include=
- Default export schema unchanged (backward compatible)
- Comma-separated metric names (not exception messages) — stable for
  programmatic filtering without exposing internal error details
- Helpers now return (value, error_name | None) tuples internally

Follow-up to #879, as discussed in its review thread.

* fix: address Qodo findings — track betweenness errors and remove unused constant

1. _betweenness() now returns (dict, error) tuple like the other helpers,
   so betweenness computation failures appear in metric_errors.
2. Removed unused _ERROR_COLUMNS constant (dead code).

All 77 tests in tests/export/ pass.

* docs(changelog): add entry for opt-in metric_errors column (#960)

---------

Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com>
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
This commit is contained in:
Karunasagar Mohansundar
2026-08-13 15:54:42 +05:30
committed by GitHub
co-authored by Mohd Kaif KaifAhmad1
parent 0fa3483b96
commit 2cfb5de43d
4 changed files with 216 additions and 28 deletions
+11
View File
@@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased] ## [Unreleased]
### Added
- **`DistanceExporter.compute_pairs()` gains an opt-in `metric_errors` column to distinguish legitimate `None` results from computation failures** (#960, follow-up to #879) by @Karunasagar12
- Previously, a `None` in `hop_count`/`weighted_distance`/`semantic_similarity`/betweenness could mean either "no path exists" or "the underlying computation raised" — logged as a warning per #879, but not otherwise surfaced, so the two cases were indistinguishable in exported CSV/JSONL/DataFrame data. `include=["metric_errors"]` now adds a `metric_errors` field per row: `""` when all requested metrics succeeded, or a comma-separated list of metric names that raised (e.g. `"hop_count,weighted_distance"`)
- Opt-in only — default `compute_pairs()`/`to_csv()`/`to_dataframe()`/`to_jsonl()` schema is unchanged unless `"metric_errors"` is explicitly requested
- The four metric helpers (`_betweenness`, `_hop_distance`, `_weighted_distance`, `_semantic_similarity`) now return `(value, error_name | None)` tuples internally; `compute_pairs()` aggregates the error names per row
- **Fixed during review** (Qodo): `_betweenness()` failures weren't tracked into `metric_errors` in the initial version — centrality computation could raise and the column would still report `""`. Now returns its error tuple like the other three helpers
- **Known limitation**: `include=["metric_errors"]` with no other metric names computes nothing, so the column is always `""` in that case — pass it alongside the metrics you want tracked, e.g. `include=["hop_count", "metric_errors"]`
- New `tests/export/test_distance_exporter_metric_errors.py`: 6 tests covering success, single/multiple failures, opt-out, the no-path-vs-error distinction, and default-schema stability; existing `tests/export/test_distance_exporter.py` updated for the new tuple return type
- Full `tests/export/` suite: 77 passed
### Changed ### Changed
- **`GraphBuilder`'s 6 public methods now have Google-style docstrings** (#878, closes #876) by @cakeni - **`GraphBuilder`'s 6 public methods now have Google-style docstrings** (#878, closes #876) by @cakeni
+58 -23
View File
@@ -11,12 +11,15 @@ Python API:
df = exporter.to_dataframe(include=["hops", "semantic_similarity", "distance_band"]) df = exporter.to_dataframe(include=["hops", "semantic_similarity", "distance_band"])
exporter.to_csv("distances.csv") exporter.to_csv("distances.csv")
exporter.to_jsonl("distances.jsonl") exporter.to_jsonl("distances.jsonl")
# Include error status columns for auditable exports:
df = exporter.to_dataframe(include=["hop_count", "metric_errors"])
""" """
import csv import csv
import io import io
import json import json
from typing import Any, Dict, List, Optional from typing import Any, Dict, List, Optional, Tuple
from ..utils.helpers import classify_path_distance from ..utils.helpers import classify_path_distance
from ..utils.logging import get_logger from ..utils.logging import get_logger
@@ -36,6 +39,9 @@ _ALL_COLUMNS = [
"distance_band", "source_betweenness", "target_betweenness", "distance_band", "source_betweenness", "target_betweenness",
] ]
# Error status columns — opt-in via include=["metric_errors"]
# (used by compute_pairs when "metric_errors" is in include set)
class DistanceExporter: class DistanceExporter:
"""Compute and export pairwise distance metrics for a ContextGraph.""" """Compute and export pairwise distance metrics for a ContextGraph."""
@@ -65,63 +71,77 @@ class DistanceExporter:
node = getattr(self.graph, "nodes", {}).get(node_id) node = getattr(self.graph, "nodes", {}).get(node_id)
return getattr(node, "node_type", "") if node else "" return getattr(node, "node_type", "") if node else ""
def _betweenness(self, graph_dict: Dict[str, Any]) -> Dict[str, float]: def _betweenness(self, graph_dict: Dict[str, Any]) -> Tuple[Dict[str, float], Optional[str]]:
"""Return (betweenness_dict, error). error is None on success."""
if self._centrality is None: if self._centrality is None:
return {} return {}, None
try: try:
result = self._centrality.calculate_betweenness_centrality(graph_dict) result = self._centrality.calculate_betweenness_centrality(graph_dict)
return result.get("betweenness", {}) if isinstance(result, dict) else {} return (result.get("betweenness", {}) if isinstance(result, dict) else {}), None
except Exception: except Exception:
logger.warning("Betweenness centrality computation failed; omitting from export", exc_info=True) logger.warning("Betweenness centrality computation failed; omitting from export", exc_info=True)
return {} return {}, "betweenness"
def _hop_distance(self, graph_dict: Dict[str, Any], src: str, tgt: str) -> Optional[int]: def _hop_distance(self, graph_dict: Dict[str, Any], src: str, tgt: str) -> Tuple[Optional[int], Optional[str]]:
"""Return (hop_count, error). error is None on success or a short description on failure."""
if self._path_finder is None: if self._path_finder is None:
return None return None, None # KG unavailable — not an error, just no data
try: try:
result = self._path_finder.bfs_shortest_path(graph_dict, src, tgt) result = self._path_finder.bfs_shortest_path(graph_dict, src, tgt)
path = result.get("path", []) if isinstance(result, dict) else (result or []) path = result.get("path", []) if isinstance(result, dict) else (result or [])
return len(path) - 1 if path else None return (len(path) - 1 if path else None), None
except Exception: except Exception:
logger.warning("Hop distance computation failed for %s -> %s; returning None sentinel", src, tgt, exc_info=True) logger.warning("Hop distance computation failed for %s -> %s; returning None sentinel", src, tgt, exc_info=True)
return None return None, "hop_count"
def _weighted_distance(self, graph_dict: Dict[str, Any], src: str, tgt: str) -> Optional[float]: def _weighted_distance(self, graph_dict: Dict[str, Any], src: str, tgt: str) -> Tuple[Optional[float], Optional[str]]:
"""Return (weighted_distance, error). error is None on success."""
if self._path_finder is None: if self._path_finder is None:
return None return None, None
try: try:
result = self._path_finder.dijkstra_shortest_path(graph_dict, src, tgt) result = self._path_finder.dijkstra_shortest_path(graph_dict, src, tgt)
if isinstance(result, dict): if isinstance(result, dict):
return float(result.get("total_weight", len(result.get("path", [])) - 1)) return float(result.get("total_weight", len(result.get("path", [])) - 1)), None
return None return None, None
except Exception: except Exception:
logger.warning("Weighted distance computation failed for %s -> %s; returning None sentinel", src, tgt, exc_info=True) logger.warning("Weighted distance computation failed for %s -> %s; returning None sentinel", src, tgt, exc_info=True)
return None return None, "weighted_distance"
def _semantic_similarity(self, graph_dict: Dict[str, Any], src: str, tgt: str) -> Optional[float]: def _semantic_similarity(self, graph_dict: Dict[str, Any], src: str, tgt: str) -> Tuple[Optional[float], Optional[str]]:
"""Return (similarity, error). error is None on success."""
if self._similarity is None: if self._similarity is None:
return None return None, None
try: try:
sim = self._similarity.cosine_similarity(graph_dict, src, tgt) sim = self._similarity.cosine_similarity(graph_dict, src, tgt)
return float(sim) if isinstance(sim, (int, float)) else None return (float(sim) if isinstance(sim, (int, float)) else None), None
except Exception: except Exception:
logger.warning("Semantic similarity computation failed for %s -> %s; returning None sentinel", src, tgt, exc_info=True) logger.warning("Semantic similarity computation failed for %s -> %s; returning None sentinel", src, tgt, exc_info=True)
return None return None, "semantic_similarity"
def compute_pairs( def compute_pairs(
self, self,
include: Optional[List[str]] = None, include: Optional[List[str]] = None,
node_subset: Optional[List[str]] = None, node_subset: Optional[List[str]] = None,
) -> List[Dict[str, Any]]: ) -> List[Dict[str, Any]]:
"""Compute all pairwise distance metrics and return as a list of dicts.""" """Compute all pairwise distance metrics and return as a list of dicts.
When ``include`` contains ``"metric_errors"``, each row gains a
``metric_errors`` field: an empty string when all metrics succeeded, or
a comma-separated list of metric names that raised during computation
(e.g. ``"hop_count,weighted_distance"``). This lets downstream consumers
distinguish legitimate ``None`` (no path) from computation failure.
"""
include_set = set(include or _ALL_COLUMNS) include_set = set(include or _ALL_COLUMNS)
track_errors = "metric_errors" in include_set
include_set.discard("metric_errors") # not a real metric to compute
graph_dict = self._build_graph_dict() graph_dict = self._build_graph_dict()
node_ids = node_subset or list(self.graph.nodes.keys()) node_ids = node_subset or list(self.graph.nodes.keys())
betweenness: Dict[str, float] = {} betweenness: Dict[str, float] = {}
betweenness_err: Optional[str] = None
if "source_betweenness" in include_set or "target_betweenness" in include_set: if "source_betweenness" in include_set or "target_betweenness" in include_set:
betweenness = self._betweenness(graph_dict) betweenness, betweenness_err = self._betweenness(graph_dict)
rows = [] rows = []
for i, src in enumerate(node_ids): for i, src in enumerate(node_ids):
@@ -129,6 +149,10 @@ class DistanceExporter:
if src == tgt: if src == tgt:
continue continue
row: Dict[str, Any] = {} row: Dict[str, Any] = {}
errors: List[str] = []
if betweenness_err:
errors.append(betweenness_err)
if "source_id" in include_set: if "source_id" in include_set:
row["source_id"] = src row["source_id"] = src
if "source_type" in include_set: if "source_type" in include_set:
@@ -140,15 +164,23 @@ class DistanceExporter:
hop_count: Optional[int] = None hop_count: Optional[int] = None
if "hop_count" in include_set or "distance_band" in include_set: if "hop_count" in include_set or "distance_band" in include_set:
hop_count = self._hop_distance(graph_dict, src, tgt) hop_count, hop_err = self._hop_distance(graph_dict, src, tgt)
if hop_err:
errors.append(hop_err)
if "hop_count" in include_set: if "hop_count" in include_set:
row["hop_count"] = hop_count row["hop_count"] = hop_count
if "weighted_distance" in include_set: if "weighted_distance" in include_set:
row["weighted_distance"] = self._weighted_distance(graph_dict, src, tgt) wd_val, wd_err = self._weighted_distance(graph_dict, src, tgt)
row["weighted_distance"] = wd_val
if wd_err:
errors.append(wd_err)
if "semantic_similarity" in include_set: if "semantic_similarity" in include_set:
row["semantic_similarity"] = self._semantic_similarity(graph_dict, src, tgt) ss_val, ss_err = self._semantic_similarity(graph_dict, src, tgt)
row["semantic_similarity"] = ss_val
if ss_err:
errors.append(ss_err)
if "distance_band" in include_set: if "distance_band" in include_set:
row["distance_band"] = classify_path_distance(hop_count) if hop_count is not None else "distant" row["distance_band"] = classify_path_distance(hop_count) if hop_count is not None else "distant"
@@ -158,6 +190,9 @@ class DistanceExporter:
if "target_betweenness" in include_set: if "target_betweenness" in include_set:
row["target_betweenness"] = betweenness.get(tgt) row["target_betweenness"] = betweenness.get(tgt)
if track_errors:
row["metric_errors"] = ",".join(errors) if errors else ""
rows.append(row) rows.append(row)
return rows return rows
+15 -5
View File
@@ -57,28 +57,36 @@ def exporter():
def test_hop_distance_logs_warning_on_exception(exporter, caplog): def test_hop_distance_logs_warning_on_exception(exporter, caplog):
with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"): with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"):
result = exporter._hop_distance({}, "a", "b") result = exporter._hop_distance({}, "a", "b")
assert result is None value, error = result
assert value is None
assert error == "hop_count"
assert any("Hop distance" in rec.message for rec in caplog.records) assert any("Hop distance" in rec.message for rec in caplog.records)
def test_weighted_distance_logs_warning_on_exception(exporter, caplog): def test_weighted_distance_logs_warning_on_exception(exporter, caplog):
with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"): with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"):
result = exporter._weighted_distance({}, "a", "b") result = exporter._weighted_distance({}, "a", "b")
assert result is None value, error = result
assert value is None
assert error == "weighted_distance"
assert any("Weighted distance" in rec.message for rec in caplog.records) assert any("Weighted distance" in rec.message for rec in caplog.records)
def test_semantic_similarity_logs_warning_on_exception(exporter, caplog): def test_semantic_similarity_logs_warning_on_exception(exporter, caplog):
with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"): with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"):
result = exporter._semantic_similarity({}, "a", "b") result = exporter._semantic_similarity({}, "a", "b")
assert result is None value, error = result
assert value is None
assert error == "semantic_similarity"
assert any("Semantic similarity" in rec.message for rec in caplog.records) assert any("Semantic similarity" in rec.message for rec in caplog.records)
def test_betweenness_logs_warning_on_exception(exporter, caplog): def test_betweenness_logs_warning_on_exception(exporter, caplog):
with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"): with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"):
result = exporter._betweenness({}) result = exporter._betweenness({})
assert result == {} value, error = result
assert value == {}
assert error == "betweenness"
assert any("Betweenness" in rec.message for rec in caplog.records) assert any("Betweenness" in rec.message for rec in caplog.records)
@@ -103,5 +111,7 @@ def test_hop_distance_no_warning_when_kg_unavailable(caplog):
exp._path_finder = None exp._path_finder = None
with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"): with caplog.at_level(logging.WARNING, logger="semantica.export.distance_exporter"):
result = exp._hop_distance({}, "a", "b") result = exp._hop_distance({}, "a", "b")
assert result is None value, error = result
assert value is None
assert error is None
assert len(caplog.records) == 0 assert len(caplog.records) == 0
@@ -0,0 +1,132 @@
"""Tests for DistanceExporter metric_errors column.
Verifies that when ``include=["metric_errors"]`` is passed to
``compute_pairs()``, the exported rows contain a ``metric_errors`` field
that distinguishes computation failures from legitimate None results.
"""
import logging
from unittest.mock import MagicMock
import pytest
from semantica.export.distance_exporter import DistanceExporter
@pytest.fixture
def mock_graph():
"""Minimal graph mock with two nodes."""
graph = MagicMock()
node_a = MagicMock(node_id="a", node_type="entity", content="A", properties={})
node_b = MagicMock(node_id="b", node_type="entity", content="B", properties={})
graph.nodes = {"a": node_a, "b": node_b}
graph.edges = []
return graph
@pytest.fixture
def exporter(mock_graph):
"""DistanceExporter with mocked KG components."""
exp = DistanceExporter(mock_graph)
exp._path_finder = MagicMock()
exp._similarity = MagicMock()
exp._centrality = MagicMock()
return exp
class TestMetricErrorsColumn:
"""Tests for the opt-in metric_errors export column."""
def test_metric_errors_empty_on_success(self, exporter):
"""When all metrics succeed, metric_errors is an empty string."""
exporter._path_finder.bfs_shortest_path.return_value = {"path": ["a", "x", "b"]}
exporter._path_finder.dijkstra_shortest_path.return_value = {"total_weight": 2.5, "path": ["a", "b"]}
exporter._similarity.cosine_similarity.return_value = 0.87
rows = exporter.compute_pairs(include=["hop_count", "weighted_distance", "semantic_similarity", "metric_errors"])
assert len(rows) == 2 # a->b and b->a
for row in rows:
assert "metric_errors" in row
assert row["metric_errors"] == ""
def test_metric_errors_records_single_failure(self, exporter):
"""When one metric fails, its name appears in metric_errors."""
exporter._path_finder.bfs_shortest_path.return_value = {"path": ["a", "b"]}
exporter._path_finder.dijkstra_shortest_path.side_effect = RuntimeError("negative cycle")
exporter._similarity.cosine_similarity.return_value = 0.5
rows = exporter.compute_pairs(include=["hop_count", "weighted_distance", "semantic_similarity", "metric_errors"])
for row in rows:
assert row["metric_errors"] == "weighted_distance"
assert row["hop_count"] == 1 # still computed
assert row["weighted_distance"] is None # failed
assert row["semantic_similarity"] == 0.5 # still computed
def test_metric_errors_records_multiple_failures(self, exporter):
"""When multiple metrics fail, all names appear comma-separated."""
exporter._path_finder.bfs_shortest_path.side_effect = RuntimeError("fail")
exporter._path_finder.dijkstra_shortest_path.side_effect = RuntimeError("fail")
exporter._similarity.cosine_similarity.side_effect = TypeError("fail")
exporter._centrality.calculate_betweenness_centrality.side_effect = RuntimeError("fail")
rows = exporter.compute_pairs(include=[
"hop_count", "weighted_distance", "semantic_similarity",
"source_betweenness", "metric_errors",
])
for row in rows:
errors = row["metric_errors"].split(",")
assert "hop_count" in errors
assert "weighted_distance" in errors
assert "semantic_similarity" in errors
assert "betweenness" in errors
assert row["hop_count"] is None
assert row["weighted_distance"] is None
assert row["semantic_similarity"] is None
def test_metric_errors_absent_when_not_requested(self, exporter):
"""When metric_errors is not in include, it doesn't appear in rows."""
exporter._path_finder.bfs_shortest_path.side_effect = RuntimeError("fail")
exporter._path_finder.dijkstra_shortest_path.return_value = {"total_weight": 1.0, "path": ["a", "b"]}
exporter._similarity.cosine_similarity.return_value = 0.9
rows = exporter.compute_pairs(include=["hop_count", "weighted_distance", "semantic_similarity"])
for row in rows:
assert "metric_errors" not in row
def test_metric_errors_distinguishes_no_path_from_error(self, exporter):
"""Core distinction: None from 'no path' has empty error; None from exception has the metric name."""
# bfs returns empty path (legitimate "no path") — NOT an error
exporter._path_finder.bfs_shortest_path.return_value = {"path": []}
# dijkstra raises (computation error)
exporter._path_finder.dijkstra_shortest_path.side_effect = ValueError("bad weight")
exporter._similarity.cosine_similarity.return_value = 0.3
rows = exporter.compute_pairs(include=["hop_count", "weighted_distance", "semantic_similarity", "metric_errors"])
for row in rows:
# Both are None, but only weighted_distance is an error
assert row["hop_count"] is None
assert row["weighted_distance"] is None
assert row["metric_errors"] == "weighted_distance"
def test_default_columns_unchanged_without_metric_errors(self, exporter):
"""Default column set (no metric_errors) produces the same schema as before."""
exporter._path_finder.bfs_shortest_path.return_value = {"path": ["a", "b"]}
exporter._path_finder.dijkstra_shortest_path.return_value = {"total_weight": 1.0, "path": ["a", "b"]}
exporter._similarity.cosine_similarity.return_value = 0.5
exporter._centrality.calculate_betweenness_centrality.return_value = {"betweenness": {"a": 0.5, "b": 0.3}}
rows = exporter.compute_pairs()
assert len(rows) == 2
expected_keys = {
"source_id", "source_type", "target_id", "target_type",
"hop_count", "weighted_distance", "semantic_similarity",
"distance_band", "source_betweenness", "target_betweenness",
}
assert set(rows[0].keys()) == expected_keys
assert "metric_errors" not in rows[0]