mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-07 08:43:35 +08:00
fix(pagerank): reject NaN damping/epsilon in the range clamp
cbm_pagerank_compute's clamp used `damping < 0 || damping > 1` and `epsilon <= 0`, both of which IEEE-754 NaN bypasses (every NaN comparison is false). Since damping/epsilon became config-tunable and cbm_config_get_double uses strtod (which parses "nan"), a `config set pagerank_damping nan` would let NaN propagate through the power iteration: every rank becomes NaN, the convergence check `delta < epsilon` is always false (runs full max_iter), and the pagerank table is silently corrupted, poisoning the search relevance sort. Switch to the inverted-range form `!(damping >= 0 && damping <= 1)` and `!(epsilon > 0)`, which reject NaN because the inner comparison is false. Found by the silent-failure review agent; verified independently (strtod parses "nan"; NaN bypassed the old clamp). test_pagerank.c pagerank_nan_damping_is_clamped: a NaN damping must yield finite ranks summing to ~1. Green: pagerank 44 passed. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
@@ -150,9 +150,15 @@ int cbm_pagerank_compute(cbm_store_t *store, const char *project,
|
||||
cbm_rank_scope_t scope) {
|
||||
if (!store || !project || !project[0]) return -1;
|
||||
if (!weights) weights = &CBM_DEFAULT_EDGE_WEIGHTS;
|
||||
if (damping < 0.0 || damping > 1.0) damping = CBM_PAGERANK_DAMPING;
|
||||
/* Reject out-of-range AND NaN. IEEE-754 makes every NaN comparison false,
|
||||
* so the naive `damping < 0 || damping > 1` form lets NaN through (it then
|
||||
* poisons every rank and prevents convergence). The inverted-range form
|
||||
* `!(x >= lo && x <= hi)` rejects NaN because the inner >= is false.
|
||||
* NaN is reachable via config (strtod parses "nan") since damping/epsilon
|
||||
* became user-tunable. */
|
||||
if (!(damping >= 0.0 && damping <= 1.0)) damping = CBM_PAGERANK_DAMPING;
|
||||
if (max_iter <= 0) max_iter = CBM_PAGERANK_MAX_ITER;
|
||||
if (epsilon <= 0.0) epsilon = CBM_PAGERANK_EPSILON;
|
||||
if (!(epsilon > 0.0)) epsilon = CBM_PAGERANK_EPSILON;
|
||||
|
||||
sqlite3 *db = cbm_store_get_db(store);
|
||||
if (!db) return -1;
|
||||
|
||||
@@ -840,6 +840,33 @@ TEST(pagerank_damping_epsilon_config_tunable) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Robustness (review finding #44): a NaN damping must NOT corrupt PageRank.
|
||||
* Since #21 made damping/epsilon config-tunable and cbm_config_get_double uses
|
||||
* strtod (which parses "nan"), a user can `config set pagerank_damping nan`.
|
||||
* The range clamp must reject NaN — IEEE-754 makes all NaN comparisons false,
|
||||
* so the naive `damping < 0 || damping > 1` form lets NaN through, poisoning
|
||||
* every rank and preventing convergence. */
|
||||
TEST(pagerank_nan_damping_is_clamped) {
|
||||
cbm_store_t *s = cbm_store_open_memory();
|
||||
cbm_store_upsert_project(s, "nan", "/tmp/nan");
|
||||
int64_t a = add_node(s, "nan", "a");
|
||||
int64_t b = add_node(s, "nan", "b");
|
||||
add_edge(s, "nan", a, b, "CALLS");
|
||||
add_edge(s, "nan", b, a, "CALLS");
|
||||
|
||||
int rc = cbm_pagerank_compute(s, "nan", NAN, CBM_PAGERANK_EPSILON,
|
||||
CBM_PAGERANK_MAX_ITER, NULL, CBM_DEFAULT_RANK_SCOPE);
|
||||
ASSERT_EQ(rc, 2);
|
||||
double ra = get_pr(s, a);
|
||||
double rb = get_pr(s, b);
|
||||
ASSERT_TRUE(isfinite(ra)); /* NaN damping must be clamped, not propagated */
|
||||
ASSERT_TRUE(isfinite(rb));
|
||||
ASSERT_TRUE(fabs((ra + rb) - 1.0) < 0.05);
|
||||
|
||||
cbm_store_close(s);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── Suite registration ──────────────────────────────────── */
|
||||
|
||||
SUITE(pagerank) {
|
||||
@@ -852,6 +879,7 @@ SUITE(pagerank) {
|
||||
RUN_TEST(pagerank_edge_weights);
|
||||
RUN_TEST(pagerank_convergence);
|
||||
RUN_TEST(pagerank_damping_epsilon_config_tunable);
|
||||
RUN_TEST(pagerank_nan_damping_is_clamped);
|
||||
RUN_TEST(pagerank_sum_to_one);
|
||||
RUN_TEST(pagerank_stored_in_db);
|
||||
RUN_TEST(pagerank_recompute_replaces);
|
||||
|
||||
Reference in New Issue
Block a user