diff --git a/src/graph_buffer/graph_buffer.c b/src/graph_buffer/graph_buffer.c index b173267ff..939cc8cdb 100644 --- a/src/graph_buffer/graph_buffer.c +++ b/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 diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 73534404a..39607d85c 100644 --- a/src/mcp/mcp.c +++ b/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; diff --git a/tests/test_graph_buffer.c b/tests/test_graph_buffer.c index c4aa5c270..232286045 100644 --- a/tests/test_graph_buffer.c +++ b/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 */ diff --git a/tests/test_mcp.c b/tests/test_mcp.c index e8722ffc1..64f128820 100644 --- a/tests/test_mcp.c +++ b/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 @@ -13870,6 +13933,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);