feat: add Baldwin's method and Total Vote Runoff - #78
feat: add Baldwin's method and Total Vote Runoff#78devin-ai-integration[bot] wants to merge 1 commit into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds Baldwin and Total Vote Runoff ranked-ballot methods. The implementation uses Borda elimination, majority stopping, Condorcet-winner preservation, and configurable tie-breaking. The methods are exported, documented, and tested. ChangesBaldwin voting methods
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Election
participant baldwin
participant BordaScoring
participant TieBreaker
Election->>baldwin: ranked ballots
baldwin->>BordaScoring: calculate active-candidate scores
BordaScoring-->>baldwin: Borda scores
baldwin->>TieBreaker: resolve lowest-score candidates
TieBreaker-->>baldwin: candidate to eliminate
baldwin-->>Election: winner or None
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #78 +/- ##
==========================================
+ Coverage 96.37% 96.65% +0.28%
==========================================
Files 17 18 +1
Lines 496 538 +42
==========================================
+ Hits 478 520 +42
Misses 18 18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_baldwin.py (1)
72-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a fixture that proves Borda scores are recomputed.
Candidate
2is last on every ballot, so removing it shifts candidates0and1equally. Stale previous-round scores can therefore produce the same tie and pass this test. Use ballots where candidate2changes the relative scores before elimination.Proposed test fixture
election = np.array([ [0, 1, 2], [0, 1, 2], - [1, 0, 2], - [1, 0, 2], + [1, 2, 0], + [1, 2, 0], ])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_baldwin.py` around lines 72 - 79, Update the election fixture in the test asserting baldwin(..., tiebreaker='order') so candidate 2’s elimination changes the relative Borda scores of candidates 0 and 1, proving scores are recomputed each round rather than reused. Keep the assertion focused on the expected winner and replace the current ballots with a fixture where candidate 2 is not uniformly last.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_baldwin.py`:
- Line 71: Update the test docstring describing baldwin(..., tiebreaker='order')
to state that the final tie is resolved and the count returns the resulting last
candidate, while keeping the docstring line under Ruff’s 79-character E501
limit.
---
Nitpick comments:
In `@tests/test_baldwin.py`:
- Around line 72-79: Update the election fixture in the test asserting
baldwin(..., tiebreaker='order') so candidate 2’s elimination changes the
relative Borda scores of candidates 0 and 1, proving scores are recomputed each
round rather than reused. Keep the assertion focused on the expected winner and
replace the current ballots with a fixture where candidate 2 is not uniformly
last.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2cbad13-09ac-4635-8511-17d1212f366b
📒 Files selected for processing (1)
tests/test_baldwin.py
|
|
||
|
|
||
| def test_baldwin_reaches_a_final_candidate_without_a_majority(): | ||
| """The count must return the last candidate after an unresolved final tie.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the docstring and satisfy Ruff E501.
baldwin(..., tiebreaker='order') resolves the final tie. It does not leave the tie unresolved. Rewrite the docstring and keep the line within 79 characters.
Proposed fix
- """The count must return the last candidate after an unresolved final tie."""
+ """Return the last candidate after resolving the final tie."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """The count must return the last candidate after an unresolved final tie.""" | |
| """Return the last candidate after resolving the final tie.""" |
🧰 Tools
🪛 GitHub Check: lint
[failure] 71-71: ruff (E501)
tests/test_baldwin.py:71:80: E501 Line too long (81 > 79)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_baldwin.py` at line 71, Update the test docstring describing
baldwin(..., tiebreaker='order') to state that the final tie is resolved and the
count returns the resulting last candidate, while keeping the docstring line
under Ruff’s 79-character E501 limit.
Source: Linters/SAST tools
Add two ranked-ballot voting methods to elsim.methods: - baldwin(): Baldwin's method, which repeatedly eliminates the candidate with the lowest Borda score, recomputing scores among the remaining candidates each round, until one candidate remains or a candidate holds a first-choice majority. - total_vote_runoff(): Total Vote Runoff, the name Foley and Maskin (2022) give to the same lowest-Borda elimination count; it delegates to baldwin(). Both stop as soon as a candidate holds a first-choice majority. Because a Condorcet winner always has an above-average Borda score, it can never be the lowest-scoring candidate and is never eliminated, so both methods satisfy the Condorcet criterion. Tests cover Condorcet-winner election, first-choice-majority stopping, the Baldwin/TVR equivalence, an unbroken score tie returning None, and the 'order' tiebreak. The README method map lists both new entries. Co-authored-by: Endolith <endolith@gmail.com>
35e22cb to
6a65f52
Compare
Summary
Adds two standalone ranked-ballot winner functions to
elsim.methods. This is the first PR in the collapse_2d rewrite stack — the round-trace APIs (#71) and the 2D animations (#72) build on top.Baldwin's method (
baldwin) repeatedly eliminates the candidate with the lowest Borda score, recomputing scores among the remaining candidates each round, until one candidate remains or a candidate holds a first-choice majority:Total Vote Runoff (
total_vote_runoff) is the name Foley & Maskin (2022) give to this exact count, so it delegates tobaldwin.Both satisfy the Condorcet criterion: a Condorcet winner always has an above-average Borda score, so it can never be the lowest scorer and is never eliminated. The first-choice-majority short-circuit is consistent with this (a first-choice-majority candidate is the Condorcet winner).
Both return
Noneon an unbroken elimination tie (matching the other elimination methods), with the usualtiebreaker={'random', 'order', None}options.Tests
tests/test_baldwin.pycovers: electing an existing Condorcet winner, first-choice-majority stopping, Baldwin/TVR returning the same winner,Noneon an unbroken score tie, and the'order'tiebreak. Thebaldwindocstring example is a doctest. Full suite221 passed; pre-commit / Ruff clean.The README method map lists both new entries.
Link to Devin session: https://app.devin.ai/sessions/d2d2ac0338354d7ab08c4468ee3e1680
Requested by: @endolith
Summary by CodeRabbit