Show discrepancies in UI for ballot comparison audits

[nikhilb4a]

Recommend to review commit by commit

This PR adds discrepancies in UI for ballot comparison audits.

Issue

Screen.Recording.2024-11-04.at.4.52.37.PM.mov

[nikhilb4a]

Ballot comparison cvrs are different than their batch comparison equivalent in that the values are all strings vs ints, to support 'o' and 'u'. Though, the code did describe it as a somewhat special case, so I am not certain it is relevant and could use some validation.

In the case where it is relevant, I thought it would be better to be consistent in returning ints where it is a number, so that is the purpose of this function. It allows us to have a single return type for discrepancies that is relatively clear. Feel free to let me know if it doesn't seem right

[jonahkagan]

What happens if we just leave them as their internal representation (ints for batch comparison, strings for ballot comparison)? Might be simpler to just keep the same representation until there's somewhere we actually need to use the strings as ints, that way you don't need to think about whether it's an int in one place or a string in other places

[nikhilb4a]

Hm that's true. I guess I thought it might be a bit odd if the frontend is getting strings on some API calls and ints on others for the same field values, when they might both be representing a 1, so that is what motivated me to normalize it. But, leaving them as is will work for sure too.

[nikhilb4a]

Removed normalization here

[jonahkagan]

Thanks for the explanatory comments!

[jonahkagan]

Container should go at the beginning, if present, and not in parens (it's the highest level in the hierarchy - i.e. multiple tabulators' batches could be in a container)

[jonahkagan]

I think reported_cvr and audited_cvr are guaranteed to be non-null at this point due to the checks in ballot_vote_deltas, so these could probably be condensed to:

reported_votes = reported_cvr.get(contest.id, {})
audited_votes = audited_cvr.get(contest.id, {})

[nikhilb4a]

Since the type of audited_cvr and reported_cvr is CVR which is an optional, it can be None. So, mypy complains that Item "None" of "Optional[Dict[str, Dict[str, str]]]" has no attribute "get". I think that is why I originally had that check.

Added the use of .get, but left the null check here, unless there is a cleaner way to do this.

[jonahkagan]

I meant they are logically guaranteed to not be None. I know the type system can't figure that out, since the checks to handle the None cases are in a different function. Up to you whether you prefer handling an impossible case to satisfy the type-system or overriding with a type assertion (I usually go for the latter, but that's just my style). Totally fine to leave it as is

[nikhilb4a]

Oh interesting. Thanks for the clarification and my bad for the miss there. I'll add in the type assertion. Still getting used to use to using type assertions consistently, but I see the benefit

[jonahkagan]

What happens if we just leave them as their internal representation (ints for batch comparison, strings for ballot comparison)? Might be simpler to just keep the same representation until there's somewhere we actually need to use the strings as ints, that way you don't need to think about whether it's an int in one place or a string in other places

[jonahkagan]

Might be nice to add a .toLocaleString() for longer numbers