in reviewhumanPR #129Trust
fix: enforce reputation score property in validate_reputation_scoring
fix: enforce reputation score property in validate_reputation_scoring
Author
@Anurag17-2005
github profile →- Status
- In review
- Opened on
- Jul 9
- Branch
- hackathon/anurag-reputation-validator-fix
Description
The pitch.
fix: enforce reputation score property in validate_reputation_scoring
Fixes #98
Problem: validate_reputation_scoring documented the property "reputation scores
decrease for agents that cheat" but never enforced it. The validator accumulated
scores in a dict but had a dead loop (lines 811-816) with only `pass` in the body,
so score violations were never caught.
Solution: This PR fixes the dead loop bug by enforcing that agents who cheat must
receive bad reports.
Implementation:
- Removed dead loop code
- Single-pass parse into agents_who_cheated / agents_with_bad_reports
- Enforces property: "cheaters must be reported"
- Updated tests to cover the validator logic
---
## Submission Context
This PR fixes the dead loop bug in issue #98. It is a supplementary contribution
alongside my primary NandaHack submission (PR #133 - Structured Receipt Field).
---
## Reviewer Feedback Addressed
Both critical issues from the review have been resolved:
### 1. ✅ Property 2 Dropped (Issue #98 Option A)
**Problem**: Property 2 ("net-negative scores") failed on stock reputation scenario
seeds 1, 3, 7, 123 because probabilistic cheaters who cheat rarely can still end
with positive scores.
**Solution**: Took reviewer's Option A:
- Removed property 2 entirely (no score tracking)
- Kept property 1: "cheaters must be reported"
- Updated docstring to document what is actually checked
- Updated 6 tests to match the simplified property
**Justification**: The reference MaliciousAgent cheats with probability 0.5. A cheater
who cheats once but delivers honestly 3+ times ends at +1. The validator now ensures
cheaters are reported (scores decrease from bad reports), but doesn't require
net-negative final scores.
### 2. ✅ Determinism Fixed
**Problem**: Set iteration caused hash-order non-deterministic output.
**Solution**: Use `sorted(unreported_cheaters)` in output formatting for byte-determinism.
---
## CI Output
```
make ci-local
All checks passed!
===== 1153 passed, 1 skipped, 1 deselected, 1 warning in 101.15s (0:01:41) =====
ci-local: all 5 checks passed. Safe to push.
```
---
## Scenario-Level Verification
As requested in review, tested scenarios/reputation.yaml on the specific seeds that failed Property 2:
**Seeds that now pass (previously failed with old Property 2):**
Seed 1: ✅ Passed - checked 3 cheaters, 3 agents with bad reports
Seed 3: ✅ Passed - checked 4 cheaters, 4 agents with bad reports
Seed 7: ✅ Passed - checked 4 cheaters, 4 agents with bad reports
Seed 123: ✅ Passed - checked 4 cheaters, 4 agents with bad reports
**Seed 42 (pre-existing failure, unrelated to this PR):**
Seed 42: ❌ Failed - cheaters not reported: ['malicious-3']
(Also fails on main branch - malicious-3 only cheats malicious peers who never report)
The simplified Property 1 ("cheaters must be reported") now holds on all seeds where the scenario actually generates reports. Seed 42's failure is pre-existing and documented in the review.
**Note on determinism fix**: Output format is now deterministic — compare the sorted list above (['malicious-3']) to main's raw set repr ({'malicious-3'}) for the same seed. This confirms both fixes are working in practice.
Try it
Open PR on GitHubView diffCheckout locally
git fetch origin pull/129/head:pr-129
git checkout pr-129