fix: NaN confidence filter silently rejected all graph_accel traversals#340
Merged
fix: NaN confidence filter silently rejected all graph_accel traversals#340
Conversation
The facade passed float('nan') as the min_confidence parameter when no
confidence filter was requested. In Rust's IEEE 754 semantics, the
comparison `edge.confidence >= NaN` is always false, so the confidence
filter silently rejected every edge that had a loaded confidence value.
This caused graph_accel_neighborhood, graph_accel_path, and
graph_accel_subgraph to return empty results whenever edges had
confidence properties — which is the common case for semantic edges
created during ingestion.
The fix passes None (SQL NULL) instead, which maps to Rust's
Option::None and bypasses the confidence filter entirely.
Verify that BFS and app_id resolution work correctly when nodes are added first (load_vertices) and edges added separately (load_edges), matching the pgrx extension's actual load sequence.
New graph-accel way covers build, deploy, debug, GUC testing, and the NULL-vs-NaN parameter pitfall. API way gets a GraphFacade section. Testing way adds Rust test commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
graph_facade.pypassedfloat('nan')as themin_confidenceparameter to graph_accel SQL functions when no confidence filter was requested. In IEEE 754,NaN >= xis alwaysfalse, so the Rust confidence filter rejected every edge with a loaded confidence value — silently returning empty results from BFS neighborhood, shortest path, and subgraph extraction.None(SQLNULL) instead, which maps to Rust'sOption::Noneand bypasses the confidence filter entirely.graph_accel_neighborhood,graph_accel_path,graph_accel_subgraph) were affected. Without this fix, any graph where edges have confidence properties (the common case for semantic edges from ingestion) returned zero results.How it was found
Searching "Negentropic Viability Model" in the web workstation returned only 1 concept with 1 adjacent at 4 hops deep. Investigation initially focused on the GUC edge-type filtering from PR #339, but the actual bug predates that PR — it was present from the original graph_accel facade integration. The GUC filtering fix (correctly reducing edge count from 19K to 3.9K) just made the empty results more noticeable since there was less data to mask the issue.
The IEEE 754 NaN trap
Test plan
concept relatedreturns 83 concepts at depth 2 (was 0)concept relatedat depth 4 returns extensive results (81K+ chars, was 0)NULLreturns 83 results,NaNreturns 0cargo test)