From 62b44519c68b403ea44d9a85ba8ee74cddee66e6 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Fri, 28 Aug 2026 09:57:59 -0400 Subject: [PATCH] fix(confidence): stop an unreadable confidence from reading as 0.0 strtod answers 0.0 for text it cannot read, and 0.0 is a real confidence in both places that call it. So a property blob carrying a malformed value, such as "confidence":null, came back as a recorded confidence of zero. Two things went wrong with that. In src/graph_buffer/graph_buffer.c, edge_props_confidence answers CBM_EDGE_CONF_ABSENT (-1) when an edge carries no confidence, so any real confidence outranks it. A malformed value answered 0.0, which beats -1 in the merge comparison, so the malformed blob displaced a clean stored one. The function's own comment already promised that "absent/unparseable reads as -1". Only the absent half was true. In src/mcp/mcp.c, bfs_edge_evidence_for_hop sets the confidence to -1 and the emitter publishes any value of 0 or more as a recorded number. A malformed value printed as 0.00, which reads as "the resolver was certain this is wrong" rather than "nobody wrote a number here". Both sites now pass an end pointer to strtod and keep the absent sentinel when the pointer never moved, which is the shape src/store/store.c:402 already uses. Two tests come with the change, and both were seen failing before the fix and passing after: gbuf_edge_props_unreadable_confidence_does_not_displace_absent tool_trace_path_unreadable_confidence_reports_not_recorded Red: 258 passed, 2 failed. Green: 260 passed, 0 failed. The full suite reports 7633 passed, 2 failed. Both failures are in tests/test_cli.c (lines 1749 and 6725) and reproduce on a clean tree without this change. They depend on the coding agents installed on the machine, not on this change. make -f Makefile.cbm lint-ci passes. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- src/graph_buffer/graph_buffer.c | 13 ++++++- src/mcp/mcp.c | 11 +++++- tests/test_graph_buffer.c | 29 +++++++++++++++ tests/test_mcp.c | 64 +++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 2 deletions(-) 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);