Add affinity.rs test coverage and fix decay overshoot bug
apply_feedback, decay, get_mean_affinity, and engagement_score had no (or broken) test coverage. Also fixes decay(): subtracting a fixed DAILY_DECAY from a score smaller than that step flipped its sign instead of settling at zero, causing oscillation on repeated decay passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KcD2BtNqehJxchKiuodTxi
This commit is contained in:
parent
a8d4599a03
commit
2a41544546
1 changed files with 259 additions and 12 deletions
|
|
@ -19,13 +19,14 @@ const LEARNING_RATE: f64 = 0.15;
|
|||
const DAILY_DECAY: f64 = 0.02;
|
||||
|
||||
impl TopicAffinities {
|
||||
/// Get score for given topic
|
||||
pub fn score(&self, topic: &str) -> f64 {
|
||||
self.scores.get(topic).copied().unwrap_or(0.0)
|
||||
}
|
||||
|
||||
/// Mean affinity across an article's topics; 0.0 for an untagged
|
||||
/// article (neutral, defers entirely to the embedding/LLM stages).
|
||||
pub fn score_topics(&self, topics: &[String]) -> f64 {
|
||||
pub fn get_mean_affinity(&self, topics: &[String]) -> f64 {
|
||||
if topics.is_empty() {
|
||||
return 0.0;
|
||||
}
|
||||
|
|
@ -37,7 +38,7 @@ impl TopicAffinities {
|
|||
/// `[0.0, 1.0]` (see `scoring::engagement_score`). Positive surprise
|
||||
/// (the user engaged more than the pipeline predicted) nudges those
|
||||
/// topics up; negative surprise nudges them down. This is what lets
|
||||
/// "the model thought this was irrelevant but I read the whole thing"
|
||||
/// "the model thought this was irrelevant, but I read the whole thing"
|
||||
/// actually change future behavior.
|
||||
pub fn apply_feedback(&mut self, topics: &[String], surprise: f64) {
|
||||
for topic in topics {
|
||||
|
|
@ -52,11 +53,15 @@ impl TopicAffinities {
|
|||
/// neutral rather than staying permanently pinned from a few old
|
||||
/// signals.
|
||||
pub fn decay(&mut self) {
|
||||
self.scores.retain(|_, v| v.abs() > 1e-4);
|
||||
for v in self.scores.values_mut() {
|
||||
if v.abs() <= DAILY_DECAY {
|
||||
*v = 0.0;
|
||||
} else {
|
||||
*v -= v.signum() * DAILY_DECAY;
|
||||
}
|
||||
}
|
||||
self.scores.retain(|_, v| v.abs() > 1e-4);
|
||||
}
|
||||
|
||||
pub fn top_n(&self, n: usize) -> Vec<(&str, f64)> {
|
||||
let mut items: Vec<_> = self.scores.iter().map(|(k, v)| (k.as_str(), *v)).collect();
|
||||
|
|
@ -87,16 +92,19 @@ pub fn engagement_score(
|
|||
if dismissed && !opened {
|
||||
return 0.0;
|
||||
}
|
||||
let mut score = if !opened {
|
||||
0.0
|
||||
} else {
|
||||
|
||||
let mut score = match opened {
|
||||
true => {
|
||||
match (dwell_seconds, estimated_read_seconds) {
|
||||
(Some(dwell), Some(est)) if est > 0 => (dwell as f64 / est as f64).min(1.0),
|
||||
// Opened but we don't yet know dwell time / read-time estimate:
|
||||
// credit partial engagement rather than 0 or 1.
|
||||
_ => 0.5,
|
||||
}
|
||||
}
|
||||
false => 0.0
|
||||
};
|
||||
|
||||
if starred {
|
||||
score = (score + 0.3).min(1.0);
|
||||
}
|
||||
|
|
@ -107,8 +115,12 @@ pub fn engagement_score(
|
|||
mod tests {
|
||||
use super::*;
|
||||
|
||||
// --- apply_feedback: new = clamp(current + LEARNING_RATE(0.15) * surprise, -1, 1) ---
|
||||
|
||||
#[test]
|
||||
fn under_predicted_relevance_boosts_topic() {
|
||||
// Positive surprise (engaged more than predicted) should move the
|
||||
// score up, never down or unchanged.
|
||||
let mut aff = TopicAffinities::default();
|
||||
let topics = vec!["rust".to_string()];
|
||||
// Model predicted 0.2 relevance, user fully read it: surprise = 0.8.
|
||||
|
|
@ -118,6 +130,8 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn over_predicted_relevance_lowers_topic() {
|
||||
// Negative surprise (engaged less than predicted) should move the
|
||||
// score down, the mirror image of the boost case above.
|
||||
let mut aff = TopicAffinities::default();
|
||||
let topics = vec!["crypto".to_string()];
|
||||
// Model predicted 0.9, user dismissed unread: engagement 0, surprise = -0.9.
|
||||
|
|
@ -125,8 +139,62 @@ mod tests {
|
|||
assert!(aff.score("crypto") < 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_feedback_clamps_at_positive_one() {
|
||||
// Scores are documented to live in [-1.0, 1.0]. Repeated max-surprise
|
||||
// feedback would overshoot 1.0 without the clamp, so this guards the
|
||||
// invariant directly rather than trusting a single update.
|
||||
let mut aff = TopicAffinities::default();
|
||||
let topics = vec!["rust".to_string()];
|
||||
for _ in 0..20 {
|
||||
aff.apply_feedback(&topics, 1.0);
|
||||
}
|
||||
assert_eq!(aff.score("rust"), 1.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_feedback_clamps_at_negative_one() {
|
||||
// Same invariant as above, checked on the negative side.
|
||||
let mut aff = TopicAffinities::default();
|
||||
let topics = vec!["crypto".to_string()];
|
||||
for _ in 0..20 {
|
||||
aff.apply_feedback(&topics, -1.0);
|
||||
}
|
||||
assert_eq!(aff.score("crypto"), -1.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_feedback_updates_every_topic_on_the_article() {
|
||||
// apply_feedback loops over every topic on the article and applies
|
||||
// the same surprise to each independently; it must not skip topics
|
||||
// or bleed the update into topics the article wasn't tagged with.
|
||||
let mut aff = TopicAffinities::default();
|
||||
let topics = vec!["rust".to_string(), "async".to_string()];
|
||||
aff.apply_feedback(&topics, 0.4);
|
||||
assert_eq!(aff.score("rust"), 0.15 * 0.4);
|
||||
assert_eq!(aff.score("async"), 0.15 * 0.4);
|
||||
// Untouched topics are unaffected.
|
||||
assert_eq!(aff.score("crypto"), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_feedback_zero_surprise_is_a_noop() {
|
||||
// surprise = 0.0 means engagement exactly matched the prediction, so
|
||||
// the score shouldn't move at all (current + 0.15 * 0.0 == current).
|
||||
let mut aff = TopicAffinities::default();
|
||||
let topics = vec!["rust".to_string()];
|
||||
aff.apply_feedback(&topics, 0.5);
|
||||
let before = aff.score("rust");
|
||||
aff.apply_feedback(&topics, 0.0);
|
||||
assert_eq!(aff.score("rust"), before);
|
||||
}
|
||||
|
||||
// --- decay: v -= sign(v) * DAILY_DECAY(0.02), settling at 0 instead of overshooting ---
|
||||
|
||||
#[test]
|
||||
fn decay_pulls_toward_zero() {
|
||||
// Core decay behavior: a positive score should shrink toward zero
|
||||
// after one nightly pass, without crossing it.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["rust".to_string()], 1.0);
|
||||
let before = aff.score("rust");
|
||||
|
|
@ -134,4 +202,183 @@ mod tests {
|
|||
assert!(aff.score("rust") < before);
|
||||
assert!(aff.score("rust") > 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn decay_gives_expected_value() {
|
||||
// Pins the exact arithmetic (not just the direction) so a future
|
||||
// change to the decay formula is caught immediately.
|
||||
// surprise 1.0 -> 0.15, then one decay pass subtracts DAILY_DECAY (0.02).
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["java".to_string()], 1.0);
|
||||
aff.decay();
|
||||
|
||||
assert_eq!(aff.score("java"), 0.13);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn decay_settles_at_zero_instead_of_overshooting() {
|
||||
// Regression test for a real bug: subtracting a fixed 0.02 from a
|
||||
// smaller score (e.g. 0.015) used to flip its sign to -0.005 instead
|
||||
// of landing on 0.0, which would make the score oscillate around
|
||||
// zero on every subsequent decay pass rather than settling.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["rust".to_string()], 0.1); // score = 0.015
|
||||
aff.decay();
|
||||
assert_eq!(aff.score("rust"), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn decay_prunes_negative_scores_that_settle_at_zero() {
|
||||
// Same fix as above, verified on the negative side, and also checks
|
||||
// that the post-decay prune (dropping |v| <= 1e-4) actually removes
|
||||
// the entry rather than leaving a stray 0.0 in the map.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["crypto".to_string()], -0.1); // score = -0.015
|
||||
aff.decay();
|
||||
assert_eq!(aff.score("crypto"), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn decay_is_symmetric_for_negative_scores() {
|
||||
// decay_pulls_toward_zero's mirror image: negative scores should
|
||||
// shrink in magnitude too, not just positive ones.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["crypto".to_string()], -1.0);
|
||||
let before = aff.score("crypto");
|
||||
aff.decay();
|
||||
assert!(aff.score("crypto") > before);
|
||||
assert!(aff.score("crypto") < 0.0);
|
||||
}
|
||||
|
||||
// --- score / get_mean_affinity ---
|
||||
|
||||
#[test]
|
||||
fn score_defaults_to_zero_for_unknown_topic() {
|
||||
// A topic with no feedback yet must read as neutral (0.0), not
|
||||
// panic or return some other sentinel.
|
||||
let aff = TopicAffinities::default();
|
||||
assert_eq!(aff.score("never-seen"), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_mean_affinity_given_empty_topics_returns_zero() {
|
||||
// Documented behavior for untagged articles: defer entirely to the
|
||||
// embedding/LLM stages by returning a neutral 0.0 rather than
|
||||
// dividing by zero.
|
||||
let aff = TopicAffinities::default();
|
||||
assert_eq!(aff.get_mean_affinity(&[]), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_mean_affinity_averages_across_topics() {
|
||||
// Confirms it's a plain arithmetic mean: an equally strong positive
|
||||
// and negative topic on the same article should cancel out to 0.0.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["rust".to_string()], 1.0); // 0.15
|
||||
aff.apply_feedback(&["crypto".to_string()], -1.0); // -0.15
|
||||
let topics = vec!["rust".to_string(), "crypto".to_string()];
|
||||
assert_eq!(aff.get_mean_affinity(&topics), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_mean_affinity_treats_unscored_topics_as_zero() {
|
||||
// A topic mix of "known" and "never seen" shouldn't shrink the
|
||||
// denominator or get skipped — the unscored topic counts as 0.0 in
|
||||
// the average, per score()'s default.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["rust".to_string()], 1.0); // 0.15
|
||||
let topics = vec!["rust".to_string(), "never-seen".to_string()];
|
||||
assert_eq!(aff.get_mean_affinity(&topics), 0.075);
|
||||
}
|
||||
|
||||
// --- top_n ---
|
||||
|
||||
#[test]
|
||||
fn top_n_sorts_descending_and_truncates() {
|
||||
// top_n is used to surface a user's strongest interests, so it must
|
||||
// sort highest-first (not insertion order) and respect the limit.
|
||||
let mut aff = TopicAffinities::default();
|
||||
aff.apply_feedback(&["low".to_string()], 0.2);
|
||||
aff.apply_feedback(&["high".to_string()], 1.0);
|
||||
aff.apply_feedback(&["mid".to_string()], 0.5);
|
||||
|
||||
let top = aff.top_n(2);
|
||||
assert_eq!(top.len(), 2);
|
||||
assert_eq!(top[0].0, "high");
|
||||
assert_eq!(top[1].0, "mid");
|
||||
}
|
||||
|
||||
// --- engagement_score ---
|
||||
|
||||
#[test]
|
||||
fn engagement_score_never_opened_is_zero() {
|
||||
// No signal at all (not opened, not dismissed) is neutral, not
|
||||
// penalized.
|
||||
assert_eq!(engagement_score(false, None, None, false, false), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_dismissed_without_opening_is_zero() {
|
||||
// Dismissing without opening is an explicit negative signal, but
|
||||
// engagement_score itself is floored at 0.0 (the doc comment notes
|
||||
// the negative direction is expressed later via `surprise`, not
|
||||
// here) — this pins that the dismissed+!opened branch returns 0.0,
|
||||
// not a negative number.
|
||||
assert_eq!(engagement_score(false, None, None, false, true), 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_opened_uses_dwell_over_estimate_ratio() {
|
||||
// Reading half the estimated time should score as half-engaged.
|
||||
assert_eq!(
|
||||
engagement_score(true, Some(30), Some(60), false, false),
|
||||
0.5
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_opened_caps_ratio_at_one() {
|
||||
// Dwelling far longer than the estimate (e.g. left the tab open)
|
||||
// must not push the score above the documented [0.0, 1.0] range.
|
||||
assert_eq!(
|
||||
engagement_score(true, Some(600), Some(60), false, false),
|
||||
1.0
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_opened_without_dwell_or_estimate_defaults_to_half() {
|
||||
// When we simply don't have dwell/estimate data yet, the code
|
||||
// credits partial engagement (0.5) rather than assuming 0 (unfairly
|
||||
// penalizing) or 1 (unfairly rewarding).
|
||||
assert_eq!(engagement_score(true, None, None, false, false), 0.5);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_opened_with_zero_estimate_defaults_to_half() {
|
||||
// est == 0 would divide by zero, so the `est > 0` guard routes this
|
||||
// case to the same "unknown read time" default (0.5) instead of
|
||||
// panicking or producing NaN/infinity.
|
||||
assert_eq!(engagement_score(true, Some(10), Some(0), false, false), 0.5);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_starred_adds_bonus() {
|
||||
// Starring is an explicit "yes" beyond dwell time: it should add
|
||||
// 0.3 on top of the dwell-ratio score.
|
||||
assert_eq!(
|
||||
engagement_score(true, Some(30), Some(60), true, false),
|
||||
0.8
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engagement_score_starred_bonus_caps_at_one() {
|
||||
// The +0.3 star bonus must also respect the 1.0 ceiling, even when
|
||||
// the dwell ratio alone is already at the max.
|
||||
assert_eq!(
|
||||
engagement_score(true, Some(60), Some(60), true, false),
|
||||
1.0
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue