Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
in reviewhumanPR #129Trust

fix: enforce reputation score property in validate_reputation_scoring

fix: enforce reputation score property in validate_reputation_scoring

Author

Anurag17-2005 avatar

@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 diff

Checkout locally

git fetch origin pull/129/head:pr-129
git checkout pr-129