Skip to content

Fix bug in discrepancy calculation for ES&S overvotes #2162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jonahkagan wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Fix bug in discrepancy calculation for ES&S overvotes #2162

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
836eb8f
Fix bug in discrepancy calculation for ES&S overvotes
jonahkagan Feb 3, 2025
8918128
Refactor to more closely match the audit math
jonahkagan Feb 3, 2025
bef3648
Update snapshots
jonahkagan Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ disable=raw-checker-failed,
logging-fstring-interpolation,
broad-exception-raised,
use-dict-literal,
use-implicit-booleaness-not-comparison
use-implicit-booleaness-not-comparison,
too-many-return-statements



Expand Down
1 change: 0 additions & 1 deletion server/activity_log/slack_worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from ..sentry import configure_sentry


# pylint: disable=too-many-return-statements
def slack_message(activity: activity_log.Activity):
base = activity.base
org_link = urljoin(config.HTTP_ORIGIN, f"/support/orgs/{base.organization_id}")
Expand Down
35 changes: 26 additions & 9 deletions server/api/shared.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,15 +526,32 @@ def ballot_vote_deltas(
if reported is None:
reported = {choice.id: "0" for choice in contest.choices}

deltas = {}
for choice in contest.choices:
reported_vote = (
0 if reported[choice.id] in ["o", "u"] else int(reported[choice.id])
)
audited_vote = (
0 if audited[choice.id] in ["o", "u"] else int(audited[choice.id])
)
deltas[choice.id] = reported_vote - audited_vote
# Special case for ES&S overvotes/undervotes.
has_overvote = "o" in reported.values()
has_undervote = "u" in reported.values()
audited_votes = sum(map(int, (audited.values())))
# If the audited result correctly identified overvote/undervote, return no
# delta. Otherwise, return discrepancies as usual, but substituting in
# overvotes/undervotes.
if has_overvote:
if audited_votes > 1:

@arsalansufi arsalansufi Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I know this is just pulled from the audit math but do we need to account for the case that a contest allows for more than 1 vote?

@jonahkagan jonahkagan Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES&S CVRs only suport vote-for-1 so far: https://github.com/votingworks/arlo/blob/jonah/fix-ess-cvr-error/server/api/cvrs.py#L851-L853

We might want to add an assertion here and in the audit math about that assumption in case it changes

return None
else:
deltas = {
choice.id: 1 - int(audited[choice.id]) for choice in contest.choices

@jonahkagan jonahkagan Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little weird, though it's closer to what the math does. The math treats an overvote as a vote for the winner and a vote for the loser, whereas this logic treats an overvote as a vote for every candidate.

}
elif has_undervote:
if audited_votes < 1:

@arsalansufi arsalansufi Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming undervote here refers to a completely undervoted, i.e., blank contest. And not a vote for N where someone didn't vote for 0 but just some number less than N

return None
else:
deltas = {
choice.id: 0 - int(audited[choice.id]) for choice in contest.choices
}
else:
deltas = {
choice.id: int(reported[choice.id]) - int(audited[choice.id])
for choice in contest.choices
}

@jonahkagan jonahkagan Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on https://github.com/votingworks/arlo/blob/jonah/fix-ess-cvr-error/server/audit_math/supersimple.py#L135-L151


if all(delta == 0 for delta in deltas.values()):
return None
Expand Down