Skip to content
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.

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

Server endpoint for surfacing discrepancies #2026

Merged
merged 4 commits into from
Oct 31, 2024

Conversation

nikhilb4a
Copy link
Contributor

@nikhilb4a nikhilb4a commented Oct 30, 2024

This PR adds a server endpoint for getting discrepancies, and for now only adds the implementation for batch comparison audits.

Next, I plan to wire together the rest of the flow to the client, and then add implementation for ballot comparison audits. My plan is to build a bit more incrementally, so the PR doesn't become too much for me to reason about as I go.

For now, thought it might be helpful to agree upon the API and models I set up, of course subject to some minor changes if needed when the rest of the feature is built out.

Sample response received by the Client from this endpoint -
Screenshot 2024-10-31 at 9 43 38 AM

@nikhilb4a nikhilb4a requested a review from jonahkagan October 30, 2024 17:04
@nikhilb4a nikhilb4a force-pushed the nikhil/1912-discrepancy-report-in-app branch from da73188 to 05527ec Compare October 30, 2024 17:04
@nikhilb4a
Copy link
Contributor Author

nikhilb4a commented Oct 30, 2024

Note - Tests will be added soon, but a review of the endpoint would still be appreciated to evaluate the design decisions

server/api/jurisdictions.py Show resolved Hide resolved
server/api/jurisdictions.py Show resolved Hide resolved
contest.id, jurisdiction_id, batch_name, discrepancies
)
discrepancies_by_jurisdiction[jurisdiction_id].append(contest_discrepancies)
return discrepancies_by_jurisdiction
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, this logic will need to take into account combined batches. See the discrepancy counting endpoint above and/or sampled_batch_rows function in reports.py (which computes the discrepancy report). May be best to add this as a follow up once we get the rest of the feature working.

@nikhilb4a nikhilb4a force-pushed the nikhil/1912-discrepancy-report-in-app branch from 83c2b75 to a83c83c Compare October 31, 2024 18:39
@nikhilb4a nikhilb4a force-pushed the nikhil/1912-discrepancy-report-in-app branch from a83c83c to 0678197 Compare October 31, 2024 18:42
@nikhilb4a nikhilb4a requested a review from jonahkagan October 31, 2024 18:42
Copy link
Contributor

@jonahkagan jonahkagan left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the thorough testing!

def create_nested_discrepancies_dict():
return defaultdict(
lambda: defaultdict(lambda: defaultdict(lambda: defaultdict(int)))
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the way you're using this, I don't think you need the last two levels of defaultdict. I think this will suffice:

defaultdict(lambda: defaultdict(dict))

Also maybe just inline it since it's only called in one place?

Copy link
Contributor Author

@nikhilb4a nikhilb4a Oct 31, 2024

Choose a reason for hiding this comment

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

Ah yeah, thanks. I initially separated it out anticipating I would also need to use it for the future ballot comparison discrepancy function, but with the shorter init then inline is fine

server/api/jurisdictions.py Outdated Show resolved Hide resolved
@nikhilb4a nikhilb4a enabled auto-merge (squash) October 31, 2024 20:46
@nikhilb4a nikhilb4a merged commit ef9ae00 into main Oct 31, 2024
5 checks passed
@nikhilb4a nikhilb4a deleted the nikhil/1912-discrepancy-report-in-app branch October 31, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants