fix: rating running average folded in the previous vote instead of the new one

[grunch]

Summary

User::update_rating computed the incremental running average using self.last_rating (the previous review) instead of the incoming rating argument, and last_rating was only assigned afterwards. As a result the newest vote was never folded into total_rating until the next vote arrived.

This inverted the effect of a bad review whenever the previous rating was above the current average:

• After ten 5-star reviews: total_rating = 4.75, last_rating = 5.
• A subsequent 1-star review raised the displayed average to ≈ 4.77 (the formula folded in the stale 5), when it should drop to ≈ 4.41.

Since total_rating is the only aggregate surfaced on the orderbook and in the Peer payload, this systematically inflated reputations and made genuine bad reviews ineffective (or counterproductive) as a trust signal.

Fix

Use the new vote in the incremental-average formula:

self.total_rating =
    old_rating + ((rating as f64) - old_rating) / (self.total_reviews as f64);

The documented 1/2 weighting of the first vote is preserved. One-line change; no public API, field, or DB schema changes.

Tests

Added a #[cfg(test)] module in src/user.rs (written first, reproducing the bug before the fix):

• First vote is weighted by 1/2
• Second vote is folded into the average (was failing: 2.5 stayed at 2.5 instead of 1.75)
• A 1-star review lowers a farmed 4.75 average to 48.5/11 ≈ 4.41 (was failing: it rose to ≈ 4.77)
• A 5-star review raises a low average
• min_rating/max_rating track extremes
• cargo test (97 lib tests + doctests), cargo clippy --all-targets --all-features, cargo fmt --check all pass

Follow-up (out of scope)

update_rating(rating: u8) does not enforce the documented MIN_RATING..=MAX_RATING range at this layer; callers validate it upstream (message.rs). Worth a separate issue to also validate at this boundary.

Summary by CodeRabbit

• Bug Fixes

  • Improved rating calculations so subsequent ratings are based on the newly submitted rating, producing more accurate averages and rating movement.
  • Improved tracking of minimum and maximum ratings.

• Tests

  • Added coverage for first-vote weighting, incremental averages, average changes, and minimum/maximum rating tracking.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 658f4e4f-17fb-4ad3-b09e-a86806423f31

📥 Commits

Reviewing files that changed from the base of the PR and between 109f26c and 438e9c2.

📒 Files selected for processing (1)

• src/user.rs

---

Walkthrough

update_rating now uses the incoming rating for subsequent running-average calculations. Unit tests cover weighting, averaging, average movement, and minimum and maximum ratings.

Changes

Rating calculation

Layer / File(s)	Summary
Rating update and validation src/user.rs	The running average uses the current incoming rating instead of last_rating. Unit tests validate first-vote weighting, subsequent averaging, average increases and decreases, and rating extremes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 438e9

The change makes each new review affect the running rating as intended, with regression coverage and passing checks reported. No actionable merge-blocking risk remains after normal review and validation.

Poem

I’m a rabbit with ratings to weigh,
New votes guide the average each day.
Min and max stand tall,
Tests check them all,
And old bugs now hop away.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the fix to use the new incoming rating instead of the previous vote in the running average.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rating-running-average-uses-new-vote

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 438e9c2df8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Migrate persisted averages before applying the new recurrence

When an existing user has ratings written by an earlier release, total_rating is not yet an average containing all total_reviews: the old recurrence omitted the latest rating and folded in the first rating twice. Applying the incoming rating directly therefore permanently skips that pending historical vote. For example, a persisted user rated 5 then 1 has total_rating = 3.75; receiving 5 after this upgrade produces 4.1667 here, whereas the documented weighting gives 2.8333. Existing rows need to be migrated/rebuilt (or explicitly transitioned using historical rating data) before this recurrence is used.

Useful? React with 👍 / 👎.

[grunch]

Good observation — the math checks out. The old recurrence telescopes to total_rating = (r1/2 + r1 + r2 + … + r(n-1))/n, so every persisted aggregate double-counts the first vote and omits the latest one, and your 3.75 → 4.1667 vs 2.8333 example is exactly right. A few clarifications on why this PR doesn't (and can't) address it here:

1. An exact migration is not possible with the persisted data. The per-user error is exactly (r1 − last_rating)/n. last_rating is stored on the row, but r1 (the first vote) is not — the DB only keeps aggregates (total_rating, total_reviews, last_rating, min_rating, max_rating), not the vote history. There is nothing to rebuild from at this layer.

2. The residual error is bounded and self-correcting. The absolute error in the implied sum is fixed (r1 − last_rating, at most ±4), so the average is off by at most 4/n at upgrade time and decays as 1/m as new votes are folded in correctly. The new recurrence is contractive over the historical error; it does not amplify it.

3. Scope: this crate has no storage. mostro-core is a library; the SQLite rows live in mostrod. Any transition logic belongs in a daemon migration, not here — and holding this fix back would keep corrupting new data while the old data is discussed.

A practical follow-up for mostrod would be a one-time migration that folds in the pending stored vote once:

UPDATE users
SET total_rating = total_rating + (last_rating - total_rating) / total_reviews
WHERE total_reviews > 1;

That removes the omitted-latest-vote half of the error, leaving only the double-counted first vote, which is unrecoverable from the persisted aggregates. I'll open an issue on mostrod proposing it.