Merge pull request #1877 from CaptainMittens/fix/confidence-parse-sentinel

fix(confidence): stop an unreadable confidence from reading as 0.0

Author: DeusData

src/graph_buffer/graph_buffer.c

@@ -1045,7 +1045,18 @@ static double edge_props_confidence(const char *props_json) {
     if (!p) {
         return CBM_EDGE_CONF_ABSENT;
     }
-    return strtod(p + sizeof(conf_key) - SKIP_ONE, NULL);
+    /* strtod answers 0.0 for text it cannot read, and 0.0 is a real
+     * confidence that beats CBM_EDGE_CONF_ABSENT. So a blob carrying
+     * "confidence":null used to outrank a clean blob that carries no
+     * confidence at all, and displace it. Ask strtod where it stopped: an
+     * end pointer that never moved means it read nothing. */
+    const char *value = p + sizeof(conf_key) - SKIP_ONE;
+    char *end = NULL;
+    double conf = strtod(value, &end);
+    if (end == value) {
+        return CBM_EDGE_CONF_ABSENT;
+    }
+    return conf;
 }
 
 /* Decide whether an incoming property blob replaces the stored one on a

src/mcp/mcp.c

@@ -6498,7 +6498,16 @@ static bool bfs_edge_evidence_for_hop(cbm_traverse_result_t *tr, int64_t hop_nod
         if (conf) {
             const char *colon = strchr(conf, ':');
             if (colon) {
-                *confidence_out = strtod(colon + 1, NULL);
+                /* strtod answers 0.0 for text it cannot read, and the caller
+                 * publishes any value >= 0 as a recorded confidence. So a
+                 * malformed value used to print as 0.00 — the one number the
+                 * surrounding code works to keep meaningful. Keep the -1 when
+                 * the end pointer never moved: nothing was read. */
+                char *end = NULL;
+                double parsed = strtod(colon + 1, &end);
+                if (end != colon + 1) {
+                    *confidence_out = parsed;
+                }
             }
         }
         return true;

tests/test_graph_buffer.c

@@ -236,6 +236,34 @@ TEST(gbuf_edge_props_merge_prefers_higher_confidence) {
     PASS();
 }
 
+/* A confidence the code cannot read is not evidence of anything, so it must
+ * not outrank an edge that simply carries no confidence at all.
+ *
+ * edge_props_confidence answers -1 for "absent" so that any real confidence
+ * beats it. strtod answers 0.0 for text it cannot read, so an unreadable
+ * value used to come back as a real confidence of zero -- which beats -1 and
+ * displaced the stored blob. The function's own comment already promised
+ * that "absent/unparseable reads as -1"; only the absent half was true. */
+TEST(gbuf_edge_props_unreadable_confidence_does_not_displace_absent) {
+    const char *no_conf = "{\"callee\":\"f\",\"strategy\":\"lsp\"}";
+    const char *bad_conf = "{\"callee\":\"f\",\"confidence\":null,\"strategy\":\"registry\"}";
+
+    cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp");
+    int64_t a = cbm_gbuf_upsert_node(gb, "Function", "a", "pkg.a", "f.go", 1, 5, "{}");
+    int64_t b = cbm_gbuf_upsert_node(gb, "Function", "b", "pkg.b", "f.go", 6, 10, "{}");
+    cbm_gbuf_insert_edge(gb, a, b, "CALLS", no_conf);
+    cbm_gbuf_insert_edge(gb, a, b, "CALLS", bad_conf); /* unreadable, arrives last */
+
+    const cbm_gbuf_edge_t **edges = NULL;
+    int count = 0;
+    cbm_gbuf_find_edges_by_type(gb, "CALLS", &edges, &count);
+    ASSERT_EQ(count, 1);
+    ASSERT_TRUE(strstr(edges[0]->properties_json, "\"strategy\":\"lsp\"") != NULL);
+
+    cbm_gbuf_free(gb);
+    PASS();
+}
+
 /* An empty incoming blob must never displace real stored properties. */
 TEST(gbuf_edge_props_merge_keeps_existing_on_empty) {
     const char *lsp = "{\"callee\":\"f\",\"confidence\":0.95,\"strategy\":\"lsp\"}";
@@ -1152,6 +1180,7 @@ SUITE(graph_buffer) {
     /* Edge property merge determinism */
     RUN_TEST(gbuf_edge_props_merge_is_order_independent);
     RUN_TEST(gbuf_edge_props_merge_prefers_higher_confidence);
+    RUN_TEST(gbuf_edge_props_unreadable_confidence_does_not_displace_absent);
     RUN_TEST(gbuf_edge_props_merge_keeps_existing_on_empty);
 
     /* Shared ID tests */

tests/test_mcp.c

@@ -3629,6 +3629,69 @@ TEST(trace_evidence_strategy_class_vocabulary_is_closed) {
     PASS();
 }
 
+/* A confidence the code cannot read must be reported as "not recorded", not
+ * as a recorded zero.
+ *
+ * The emitter reserves ev_conf < 0 for "no confidence on this edge" and
+ * prints "-" (text) or null (json). The reader set ev_conf with
+ * strtod(colon + 1, NULL), and strtod answers 0.0 for text it cannot read --
+ * so a malformed value passed the ev_conf >= 0.0 test and printed 0.00, the
+ * one value the surrounding code goes out of its way to keep meaningful.
+ * A caller then cannot tell "the resolver was certain this is wrong" from
+ * "nobody wrote a number here". */
+TEST(tool_trace_path_unreadable_confidence_reports_not_recorded) {
+    cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
+    cbm_store_t *st = cbm_mcp_server_store(srv);
+    const char *proj = "badconf-proj";
+    cbm_mcp_server_set_project(srv, proj);
+    cbm_store_upsert_project(st, proj, "/tmp/badconf");
+    cbm_node_t caller = {.project = proj,
+                         .label = "Function",
+                         .name = "caller",
+                         .qualified_name = "badconf-proj.src.caller",
+                         .file_path = "src/a.c",
+                         .start_line = 1,
+                         .end_line = 5};
+    cbm_node_t callee = {.project = proj,
+                         .label = "Function",
+                         .name = "target",
+                         .qualified_name = "badconf-proj.src.target",
+                         .file_path = "src/a.c",
+                         .start_line = 10,
+                         .end_line = 20};
+    int64_t id_caller = cbm_store_upsert_node(st, &caller);
+    int64_t id_callee = cbm_store_upsert_node(st, &callee);
+    ASSERT_GT(id_caller, 0);
+    ASSERT_GT(id_callee, 0);
+    /* The strategy reads fine; only the confidence is malformed. */
+    cbm_edge_t e = {.project = proj,
+                    .source_id = id_caller,
+                    .target_id = id_callee,
+                    .type = "CALLS",
+                    .properties_json = "{\"callee\":\"target\",\"confidence\":null,"
+                                       "\"strategy\":\"lsp_trait_dispatch\",\"candidates\":1}"};
+    ASSERT_GT(cbm_store_insert_edge(st, &e), 0);
+
+    char *ev = cbm_mcp_server_handle(
+        srv, "{\"jsonrpc\":\"2.0\",\"id\":93,\"method\":\"tools/call\","
+             "\"params\":{\"name\":\"trace_path\",\"arguments\":{\"function_name\":\"caller\","
+             "\"project\":\"badconf-proj\",\"direction\":\"outbound\",\"include_evidence\":true}}}");
+    ASSERT_NOT_NULL(ev);
+    char *ev_txt = extract_text_content(ev);
+    ASSERT_NOT_NULL(ev_txt);
+    /* Positive controls: the hop and its readable class still come through, so
+     * a failure below is about the confidence and not a broken request. */
+    ASSERT_NOT_NULL(strstr(ev_txt, "target"));
+    ASSERT_NOT_NULL(strstr(ev_txt, "lsp"));
+    /* The claim: an unreadable confidence is never published as 0.00. */
+    ASSERT_NULL(strstr(ev_txt, "0.00"));
+    free(ev_txt);
+    free(ev);
+
+    cbm_mcp_server_free(srv);
+    PASS();
+}
+
 /* Distilled from #559 (@vvenegasv). The indexer already records
  * {strategy, confidence} on every CALLS edge (pass_calls.c:355) and the store
  * reads it back, but no tool ever surfaced it — an agent could see THAT A->B
@@ -13972,6 +14035,7 @@ SUITE(mcp) {
     RUN_TEST(trace_evidence_strategy_class_vocabulary_is_closed);
     RUN_TEST(tool_trace_path_evidence_is_opt_in_and_class_mapped);
     RUN_TEST(tool_trace_path_evidence_columns_match_header_issue1542);
+    RUN_TEST(tool_trace_path_unreadable_confidence_reports_not_recorded);
     RUN_TEST(tool_trace_call_path_depth_clamped);
     RUN_TEST(tool_trace_call_path_distinct_defs_not_over_unioned);
     RUN_TEST(tool_trace_call_path_dts_stub_unions_with_impl);