From e6b31fc7648cc12e10eee7715da49bc8aafc719a Mon Sep 17 00:00:00 2001 From: efiten Date: Fri, 4 Sep 2026 17:04:48 +0200 Subject: [PATCH] fix(#1927): stop plotting the usefulness composite on the traffic axis (#1960) Closes #1927. The scatter's **"Traffic share"** axis and the **"Traffic"** table column both fell back to `usefulness_score` when `traffic_share_score` was absent, with nothing marking the substitution. Two points on one axis could therefore measure different things. ## They really are different metrics Checked rather than assumed, in `cmd/server/usefulness_composite.go`: ```go node["traffic_share_score"] = trafficRaw // :147 the single traffic axis node["usefulness_score"] = composite // :152 0.30*bridge + 0.25*coverage + ... ``` `openapi.go:178,182` describes them the same way. So the fallback put a **composite** under a column and an axis that both promise share of non-advert traffic. Worth flagging, because it is easy to conclude the opposite: **#1456, which introduced the fallback, was a rename of the display label** from "Usefulness" to "Traffic share". That makes the two field names look interchangeable, and I nearly stopped there. They are not: #672 later gave `usefulness_score` its own composite meaning. ## Fix Your first preference in the issue: drop the fallback rather than mark it or relabel the axis. A node with no `traffic_share_score` now reads as unknown, so the table shows an em dash and the point is dropped from the plot by the existing `plottable` filter (`analytics.js:2728`), exactly the way a node with no bridge score already is. **No other change was needed** for that, which is what makes this the cheap option of the three. ## Tests The unit test that pinned the old behaviour is updated rather than deleted, so the expectation is now recorded the right way round: ```js assert(mapped[1].traffic === null && mapped[1].fav === false, 'a node with only usefulness_score has no traffic value; it must not be substituted'); ``` `test-repeater-metric-scatter.js`: 31 passed, 0 failed. Also corrected a comment above `_toScatterPoints` that still documented the removed fallback chain. Co-authored-by: Claude Opus 5 (1M context) --- public/analytics.js | 21 ++++++++++++++------- test-repeater-metric-scatter.js | 10 ++++++++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index 0875e37e..eb162e49 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -2487,8 +2487,13 @@ return age < th.degradedMs ? 'active' : age < th.silentMs ? 'degraded' : 'silent'; } const pct = v => (v != null ? (v * 100).toFixed(1) + '%' : '—'); - // #1456: prefer traffic_share_score, fall back to usefulness_score. - const trafficOf = n => (n.traffic_share_score != null ? n.traffic_share_score : (n.usefulness_score != null ? n.usefulness_score : null)); + // #1927: no fallback to usefulness_score. They are different metrics: + // traffic_share_score is the single traffic axis, usefulness_score is the + // #672 composite (0.30*bridge + 0.25*coverage + ...), set separately at + // cmd/server/usefulness_composite.go:147 and :152. Substituting one for the + // other put a composite under a column and an axis that both promise share + // of non-advert traffic. A node without the metric now reads as unknown. + const trafficOf = n => (n.traffic_share_score != null ? n.traffic_share_score : null); function roleBadge(role) { // Route the unknown-role fallback through ROLE_COLORS.unknown (backed by // --mc-role-unknown / the shared Wong palette) instead of a hard-coded @@ -2682,9 +2687,10 @@ } // Map /api/nodes rows to plottable points. Pure and factored out of - // renderRepeaterMetricsTab so the repeater/room filter and the fallback - // chains (traffic_share_score → usefulness_score → null; name → pubkey - // prefix → '?') are unit-testable (#1760 review). + // renderRepeaterMetricsTab so the repeater/room filter and the name fallback + // (name → pubkey prefix → '?') are unit-testable (#1760 review). The traffic + // fallback to usefulness_score that used to be documented here was removed in + // #1927; see the note above trafficOf. function _toScatterPoints(nodes, favs) { return nodes .filter(n => n.role === 'repeater' || n.role === 'room') @@ -2693,8 +2699,9 @@ name: n.name || (n.public_key ? n.public_key.slice(0, 12) : '?'), role: n.role, fav: favs.has(n.public_key), - // #1456: prefer traffic_share_score, fall back to usefulness_score. - traffic: n.traffic_share_score != null ? n.traffic_share_score : (n.usefulness_score != null ? n.usefulness_score : null), + // #1927: see trafficOf above. A null drops the point from the scatter via + // the plottable filter, the same way a node with no bridge score is dropped. + traffic: n.traffic_share_score != null ? n.traffic_share_score : null, bridge: n.bridge_score != null ? n.bridge_score : null, relay1h: n.relay_count_1h != null ? n.relay_count_1h : null, relay24h: n.relay_count_24h != null ? n.relay_count_24h : null, diff --git a/test-repeater-metric-scatter.js b/test-repeater-metric-scatter.js index dd81fc48..dbaa3c32 100644 --- a/test-repeater-metric-scatter.js +++ b/test-repeater-metric-scatter.js @@ -81,8 +81,14 @@ assert(mapped[0].traffic === 0.5 && mapped[0].fav === true, 'traffic_share_score is preferred and favorites are flagged'); assert(mapped[0].bridge === 0.2 && mapped[0].relay1h === 1 && mapped[0].relay24h === 2 && mapped[0].adverts === 3, 'bridge/relay/advert counts map onto their renamed point fields (advert_count → adverts)'); -assert(mapped[1].traffic === 0.07 && mapped[1].fav === false, - 'missing traffic_share_score falls back to usefulness_score'); +// #1927: no fallback. usefulness_score is the #672 composite +// (0.30*bridge + 0.25*coverage + ...), a different metric from the single +// traffic axis, and substituting it put a composite under a column and an axis +// that both promise share of non-advert traffic. A node carrying only the +// composite now reads as unknown on that axis and is dropped from the plot by +// the plottable filter, which is what #1927 asked for. +assert(mapped[1].traffic === null && mapped[1].fav === false, + 'a node with only usefulness_score has no traffic value; it must not be substituted'); assert(mapped[2].traffic === null && mapped[2].bridge === null && mapped[2].relay1h === null, 'rows without scores map to null (not 0/undefined) so plots can skip them'); assert(mapped[3].name === 'NAMELESS0000',