From f06de0dab2c30950690eeb68b82fdfe4c561aa0b Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Mon, 27 Apr 2026 18:28:04 +0530 Subject: [PATCH] fix(context): address PR #512 review blockers and bot findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge blockers (ZohaibHassan16): - fix: distance-matrix raises HTTP 503 when metric=semantic but no similarity backend is available, instead of silently returning hop distances labeled as semantic - fix: distance-enriched export now requires node_subset (HTTP 422 if omitted), preventing unbounded all-pairs O(n^2) export over full graph - fix: DistanceExportRequest default include corrected from ["hops", "distance_band"] to ["source_id", "target_id", "hop_count", "distance_band"] so default exports are unambiguous and use the correct column name - fix: confidence decay edge weight index now reads graph_dict.get("edges") or graph_dict.get("relationships") to handle both graph dict shapes, fixing always-1.0 decay when session returns relationships key Bot findings (github-code-quality / chatgpt-codex): - fix: remove unused Iterable import from distance_exporter.py - fix: remove unused Response import from graph.py - fix: move logger init before optional KG import; replace empty except ImportError: pass with logger.debug in distance_exporter.py - fix: replace two bare except Exception: pass in temporal.distance_history with logger.warning including source, target, metric, and timestamp context - fix: remove mixed import style in test_qual003 — use only module import and reference CausalChainAnalyzer through it --- semantica/explorer/routes/export_import.py | 9 ++++++++- semantica/explorer/routes/graph.py | 13 ++++++++++--- semantica/explorer/routes/temporal.py | 15 +++++++++++---- semantica/explorer/schemas.py | 2 +- semantica/export/distance_exporter.py | 10 +++++----- tests/_smoke_review_fixes.py | 3 +-- 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/semantica/explorer/routes/export_import.py b/semantica/explorer/routes/export_import.py index d4cbba79..3529590a 100644 --- a/semantica/explorer/routes/export_import.py +++ b/semantica/explorer/routes/export_import.py @@ -247,7 +247,14 @@ async def export_distance_enriched( session: GraphSession = Depends(get_session), ): """FR-10 — Export pairwise distance metrics as CSV or JSONL for ML pipelines.""" - if body.node_subset and len(body.node_subset) > _DISTANCE_EXPORT_MAX_NODES: + if not body.node_subset: + raise HTTPException( + status_code=422, + detail=( + f"node_subset is required; provide up to {_DISTANCE_EXPORT_MAX_NODES} node IDs to export." + ), + ) + if len(body.node_subset) > _DISTANCE_EXPORT_MAX_NODES: raise HTTPException( status_code=413, detail=( diff --git a/semantica/explorer/routes/graph.py b/semantica/explorer/routes/graph.py index 7c41609d..9e9badaa 100644 --- a/semantica/explorer/routes/graph.py +++ b/semantica/explorer/routes/graph.py @@ -10,7 +10,7 @@ from typing import List, Optional logger = logging.getLogger(__name__) -from fastapi import APIRouter, Depends, HTTPException, Query, Response +from fastapi import APIRouter, Depends, HTTPException, Query from ...utils.helpers import classify_path_distance from ..dependencies import get_session @@ -226,9 +226,10 @@ async def find_path( try: graph_dict = await asyncio.to_thread(session.build_graph_dict) - # Build edge weight index once in O(E) so each hop lookup is O(1) + # Build edge weight index once in O(E) so each hop lookup is O(1). + # graph_dict may use "edges" or "relationships" depending on the graph source. edge_weight_index: dict = {} - for _e in graph_dict.get("edges", []): + for _e in graph_dict.get("edges") or graph_dict.get("relationships", []): _s, _t = _e.get("source"), _e.get("target") _w = float(_e.get("weight", 1.0)) edge_weight_index[(_s, _t)] = _w @@ -384,6 +385,12 @@ async def distance_matrix( detail=f"Too many nodes: {len(body.node_ids)} requested; maximum is 50 per request.", ) + if body.metric == "semantic" and session.similarity is None: + raise HTTPException( + status_code=503, + detail="metric='semantic' requires an embedding backend which is not available in this session.", + ) + started = time.perf_counter() graph_dict = await asyncio.to_thread(session.build_graph_dict) path_finder = session.path_finder diff --git a/semantica/explorer/routes/temporal.py b/semantica/explorer/routes/temporal.py index 6ebe9a8e..31cca479 100644 --- a/semantica/explorer/routes/temporal.py +++ b/semantica/explorer/routes/temporal.py @@ -153,8 +153,11 @@ async def distance_history( result = await asyncio.to_thread(path_fn, graph_dict, source, target) path_nodes = result.get("path", []) if isinstance(result, dict) else (result or []) hop_count = len(path_nodes) - 1 if path_nodes else None - except Exception: - pass + except Exception as exc: + logger.warning( + "distance_history path computation failed for source=%r target=%r metric=%r: %s", + source, target, metric, exc, exc_info=True, + ) now = datetime.now(UTC).replace(tzinfo=None) snap = DistanceSnapshot( timestamp=now, @@ -195,8 +198,12 @@ async def distance_history( result = await asyncio.to_thread(path_fn, graph_dict, source, target) path_nodes = result.get("path", []) if isinstance(result, dict) else (result or []) hop_count = len(path_nodes) - 1 if path_nodes else None - except Exception: - pass + except Exception as exc: + logger.warning( + "distance_history path computation failed for source=%r target=%r at=%s metric=%s: %s", + source, target, sample_time.isoformat(), metric, exc, exc_info=True, + ) + hop_count = None band = classify_path_distance(hop_count) if hop_count is not None else "distant" snap = DistanceSnapshot(timestamp=sample_time, hop_count=hop_count, distance_band=band) diff --git a/semantica/explorer/schemas.py b/semantica/explorer/schemas.py index 210cd2c2..226cd41e 100644 --- a/semantica/explorer/schemas.py +++ b/semantica/explorer/schemas.py @@ -410,5 +410,5 @@ class DistanceExportRequest(BaseModel): format: Literal["csv", "jsonl"] = "csv" node_subset: Optional[List[str]] = None include: List[str] = Field( - default_factory=lambda: ["hops", "distance_band"], + default_factory=lambda: ["source_id", "target_id", "hop_count", "distance_band"], ) diff --git a/semantica/export/distance_exporter.py b/semantica/export/distance_exporter.py index 40ebfc07..be78637d 100644 --- a/semantica/export/distance_exporter.py +++ b/semantica/export/distance_exporter.py @@ -16,19 +16,19 @@ Python API: import csv import io import json -from typing import Any, Dict, Iterable, List, Optional +from typing import Any, Dict, List, Optional from ..utils.helpers import classify_path_distance from ..utils.logging import get_logger +logger = get_logger(__name__) + _KG_AVAILABLE = False try: from ..kg import PathFinder, SimilarityCalculator, CentralityCalculator _KG_AVAILABLE = True -except ImportError: - pass - -logger = get_logger(__name__) +except ImportError as exc: + logger.debug("KG components not available; distance exporter will run in reduced mode: %s", exc) _ALL_COLUMNS = [ "source_id", "source_type", "target_id", "target_type", diff --git a/tests/_smoke_review_fixes.py b/tests/_smoke_review_fixes.py index 303b5c29..29fb7323 100644 --- a/tests/_smoke_review_fixes.py +++ b/tests/_smoke_review_fixes.py @@ -85,9 +85,8 @@ def test_bug004_causal_distance_report_schema_validates(): # ── qual_003: _distance_band static methods removed; classify_path_distance used ─ def test_qual003_distance_band_removed_from_causal_analyzer(): - from semantica.context.causal_analyzer import CausalChainAnalyzer import semantica.context.causal_analyzer as ca_mod - assert not hasattr(CausalChainAnalyzer, "_distance_band") + assert not hasattr(ca_mod.CausalChainAnalyzer, "_distance_band") ca_src = inspect.getsource(ca_mod) assert "def _distance_band" not in ca_src assert "classify_path_distance" in ca_src