Skip to content

Pie template clean: PR for heroku review app#613

Open
artoonie wants to merge 21 commits intomainfrom
pie-template-clean
Open

Pie template clean: PR for heroku review app#613
artoonie wants to merge 21 commits intomainfrom
pie-template-clean

Conversation

@artoonie
Copy link
Owner

No description provided.

skaphan added 17 commits March 20, 2026 18:11
Add embedded pie chart web component, templates, static assets,
and pie_chart_no_cache setting. Store raw JSON on graph for pie
chart data access.
The pie chart was the only visualization reading graph._raw_JSON directly,
bypassing config filters like excludeFinalWinnerAndEliminatedCandidate.
New converter in visualizer/pie/graphToRCtab.py builds RCTab-compatible
JSON from the same processed Graph that bar chart and sankey use.

Co-Authored-By: Claude Opus 4.6
- Pass textForWinner and excludeFinalWinnerAndEliminatedCandidate
- Set currentRound to numRounds so pie loads on last round
- Rebuilt standalone component with new props

Co-Authored-By: Claude Opus 4.6
Following the hideSankey/hideTabular pattern: model field, config JS
variable, settings checkbox, and tab hiding in hideTabsBasedOnConfig().

Co-Authored-By: Claude Opus 4.6
Covers structure validation, tally correctness, eliminations, multi-winner
surplus, inactive ballots, no-threshold elections, excludeFinalWinner flag,
and the _stringify helper.

Co-Authored-By: Claude Opus 4.6
Co-Authored-By: Claude Opus 4.6
Use a coprime stride near N/2 to reorder the color palette so that
visually similar colors are spread apart around the pie chart.

Co-Authored-By: Claude Opus 4.6
Replace color scramble with tally interleaving: alternate largest and
smallest candidates so small slices always have a large neighbor,
reducing label overlap. Inactive Ballots always last.

Fix color generation to use actual tally entry count instead of
config.numCandidates, which excluded Inactive Ballots and caused
color wrapping on large elections.

Co-Authored-By: Claude Opus 4.6
Per-round reordering broke D3 pie animation because candidate positions
shifted between rounds. Now the interleave order is stable across all
rounds, just dropping eliminated candidates.

Co-Authored-By: Claude Opus 4.6
Tests pie vistype rendering, embedly redirect, hidePie config toggle,
and cloudflare cache purge including pie URLs. Tests graphToRCtab with
candidate names containing HTML, quotes, and other special characters.

Co-Authored-By: Claude Opus 4.6
Set firstRoundDeterminesPercentages=false so percentages use active
votes per round as denominator. Inactive Ballots shows count only.
Animation button cycles Eliminate/Transfer/Consolidate labels.

Co-Authored-By: Claude Opus 4.6
…hart

Tooltip now positions near mouse cursor with viewport clamping.
Pie chart centered (removed stale margin-right). Bubble visualization
references removed.

Co-Authored-By: Claude Opus 4.6
Set aspect-ratio and max-width on pie-body container so the
browser allocates space before the custom element renders.
Reduces Cumulative Layout Shift from 0.753 to 0.014.

Co-Authored-By: Claude Opus 4.6
skaphan added 4 commits March 24, 2026 18:03
- Make pie_chart_no_cache conditional on DEBUG
- Remove TODO comment from pie.css
- Renumber hidePie migration to 0034 (avoids conflict with caching PR)
- Move _stringify to common.stringify (public API)
- Improve testGraphToRCtab docstring explaining its purpose
- Remove scripts/reset-db.sh from .gitignore
- Add pie chart round-change Selenium test

Co-Authored-By: Claude Opus 4.6

# Wait for the pie chart to render
self._ensure_eventually_asserts(
lambda: self.assertTrue(len(self._get_pie_svg_text()) > 0))

Check notice

Code scanning / CodeQL

Imprecise assert Note test

assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.

Copilot Autofix

AI about 12 hours ago

In general, to fix this type of issue you should replace generic boolean assertions like assertTrue(a > b) with the corresponding specific assertion, such as assertGreater(a, b), so that failing tests report the compared values. Alternatively, you can keep assertTrue but provide a descriptive message.

For this specific case in visualizer/tests/testLiveBrowserHeadless.py, we want to preserve the existing behavior—assert that the SVG text is non-empty—while improving the message. The best direct change is to use assertGreater on the length of the SVG text compared to zero. We should keep the lambda structure expected by _ensure_eventually_asserts, only changing the assertion call inside the lambda. Concretely, on line 732, replace self.assertTrue(len(self._get_pie_svg_text()) > 0) with self.assertGreater(len(self._get_pie_svg_text()), 0). No new imports or helper methods are required, since assertGreater is provided by unittest.TestCase, which this class already inherits from.

Suggested changeset 1
visualizer/tests/testLiveBrowserHeadless.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/visualizer/tests/testLiveBrowserHeadless.py b/visualizer/tests/testLiveBrowserHeadless.py
--- a/visualizer/tests/testLiveBrowserHeadless.py
+++ b/visualizer/tests/testLiveBrowserHeadless.py
@@ -729,7 +729,7 @@
 
         # Wait for the pie chart to render
         self._ensure_eventually_asserts(
-            lambda: self.assertTrue(len(self._get_pie_svg_text()) > 0))
+            lambda: self.assertGreater(len(self._get_pie_svg_text()), 0))
 
         # Change to round 1 via the pie's round player select.
         # Round 1 should have all candidates including Banana,
EOF
@@ -729,7 +729,7 @@

# Wait for the pie chart to render
self._ensure_eventually_asserts(
lambda: self.assertTrue(len(self._get_pie_svg_text()) > 0))
lambda: self.assertGreater(len(self._get_pie_svg_text()), 0))

# Change to round 1 via the pie's round player select.
# Round 1 should have all candidates including Banana,
Copilot is powered by AI and may make mistakes. Always verify output.
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