Skip to content

Update validation implementation/texts against specs#3203

Merged
jschuurk-kr merged 7 commits intomainfrom
3171-check-validation-implementation-and-texts-against-specs
Apr 22, 2026
Merged

Update validation implementation/texts against specs#3203
jschuurk-kr merged 7 commits intomainfrom
3171-check-validation-implementation-and-texts-against-specs

Conversation

@stacktraceghost
Copy link
Copy Markdown
Contributor

@stacktraceghost stacktraceghost commented Apr 21, 2026

Resolves #3171

Based on the changes made in kiesraad/abacus-documentatie#54

Changes in this PR:

  • Updated docstrings c162d8d
    • Includes model
    • Some title updates
  • Update feedback d5fdc20 / 938ff59
    • CSB feedback copy updated
    • GSB feedback
      • Title removed for W201 - W204, defaults to "Controleer je antwoorden" so warnings get merged in display
      • Minor action change for F305 to align with documentation
      • Updated tests accordingly
    • Feedback component
      • Only show default actions (=handelingsperspectieven) for typists (all feedback types, all categories) and coordinators (error, GSB only).
      • Story: added election selector so you can see feedback based on GSB or CSB
  • Trigger rules for the right model 859076b
    • election_fixture now accept committee_category so we can test other election types
    • Rules which are implemented in shared structs, now have explicit tests to check that they trigger in the correct committee category, see changelog below:

Changelog validation rules:

  • Removed W203 from GSBResults
Validation code Model/file Change
F101/F102 ExtraInvestigation Added if-statement to trigger for GSB only
Tests converted to matrix, included case to check it doesn't trigger for CSB
F111/F112 CountingDifferencesPollingStation Added if-statement to trigger for GSB only
Tests converted to matrix, included case to check it doesn't trigger for CSB
F201 VotersCounts Should trigger for all models
Added case to check it also triggers for CSB
F202/F203/F204 VotesCounts Should trigger for all models
Added case to check it also triggers for CSB
F301 - F310 DifferencesCounts Should only trigger for GSB CSO/DSO. This file is only included in CSO (and later DSO) models, so left as-is.
F312 GSBDifferencesCounts Should only trigger for CSB. This file is only included in the GSB model, so left as-is.
F401 - F403 CommonPollingStationResults and GSBResults Should trigger for all models
Each model has their own (identical) implementation, since these rules are placed at the model root. Left as-is
W001 domain/data_entry.rs Should trigger for all models, left as-is
W201/W202 VotesCounts Added if-statement to trigger for GSB only
Added test case for both rules to check it doesn't trigger for CSB
W203 CommonPollingStationResults Should only trigger for CSB. This file is only included in the GSB model, so left as-is.
W204 VotesCounts Should trigger for all models
Added case to check it also triggers for CSB
W205/W206 GSBResults Should only trigger for CSB. This file is only included in the GSB model, so left as-is.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

Sigrid maintainability feedback

⚠️ Your code did not improve towards your objective of 3.5 stars.

Show details

Sigrid compared your code against the baseline of 2026-04-21.

👍 What went well?

You fixed or improved 9 refactoring candidates.

Risk System property Location
🔴 Duplication
(Fixed)
backend/src/domain/results/cso_next_session_results.rs line 31-52
backend/src/domain/results/gsb_results.rs line 135-156
backend/src/domain/results/cso_first_session_results.rs line 71-92
🔴 Duplication
(Fixed)
backend/src/domain/results/common_polling_station_results.rs line 35-89
backend/src/domain/results/gsb_results.rs line 69-123
🔴 Duplication
(Fixed)
backend/src/domain/results/common_polling_station_results.rs line 143-154
backend/src/domain/results/gsb_results.rs line 209-220
🔴 Duplication
(Fixed)
backend/src/domain/results/extra_investigation.rs line 41-46
backend/src/domain/results/counting_differences_polling_station.rs line 26-31
🔴 Duplication
(Fixed)
backend/src/domain/results/votes_counts.rs line 231-236
backend/src/domain/results/differences_counts.rs line 333-338
backend/src/domain/results/common_polling_station_results.rs line 94-99
+ 9 occurrences
🟠 Unit Size
(Improved)
backend/src/domain/results/gsb_results.rs
GSBResults.validate(ElectionWithPoliticalGroups,ValidationResults,FieldPath)
🟡 Unit Size
(Fixed)
backend/src/domain/results/gsb_results.rs
GSBResults.validate_voters_and_votes_count_difference(ValidationResults,FieldPath)
🟡 Unit Size
(Fixed)
backend/src/domain/results/votes_counts.rs
VotesCounts.validate_votes_counts_warnings(ValidationResults,FieldPath)
⚫️ + 1 more

👎 What could be better?

Unfortunately, 14 refactoring candidates were introduced or got worse.

Risk System property Location
🔴 Duplication
(Introduced)
backend/src/domain/results/common_polling_station_results.rs line 35-89
backend/src/domain/results/gsb_results.rs line 41-95
🔴 Duplication
(Introduced)
backend/src/domain/results/gsb_results.rs line 107-128
backend/src/domain/results/cso_next_session_results.rs line 31-52
backend/src/domain/results/cso_first_session_results.rs line 71-92
🔴 Duplication
(Introduced)
backend/src/domain/results/common_polling_station_results.rs line 143-154
backend/src/domain/results/gsb_results.rs line 179-190
🔴 Duplication
(Introduced)
backend/src/domain/results/counting_differences_polling_station.rs line 26-32
backend/src/domain/results/extra_investigation.rs line 41-47
🔴 Duplication
(Introduced)
backend/src/domain/data_entry.rs line 215-220
backend/src/domain/results/mod.rs line 262-267
backend/src/domain/results/counting_differences_polling_station.rs line 26-31
+ 11 occurrences
🟠 Unit Complexity
(Worsened)
frontend/src/components/ui/Feedback/Feedback.tsx
Feedback.tsx.Feedback(FeedbackProps)
🟡 Unit Size
(Introduced)
backend/src/domain/results/votes_counts.rs
VotesCounts.validate_votes_counts_warnings(ElectionWithPoliticalGroups,ValidationResults,FieldPath)
🟡 Unit Size
(Worsened)
backend/src/domain/results/extra_investigation.rs
ExtraInvestigation.validate(ElectionWithPoliticalGroups,ValidationResults,FieldPath)
⚫️ + 6 more

📚 Remaining technical debt

15 refactoring candidates didn't get better or worse, but are still present in the code you touched.

View this system in Sigrid to explore your technical debt

⭐️ Sigrid ratings

System property System on 2026-04-21 Before changes New/changed code
Volume 3.4 N/A N/A
Duplication 3.6 2.7 2.6
Unit Size 2.3 0.8 0.9
Unit Complexity 3.4 3.0 2.4
Unit Interfacing 2.8 1.0 1.0
Module Coupling 3.3 5.5 5.5
Component Independence 5.5 N/A N/A
Component Entanglement N/A N/A N/A
Maintainability 3.5 2.9 2.7

💬 Did you find this feedback helpful?

We would like to know your thoughts to make Sigrid better.
Your username will remain confidential throughout the process.


View this system in Sigrid

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.34%. Comparing base (b4bcb3d) to head (39d0c36).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3203      +/-   ##
==========================================
+ Coverage   90.32%   90.34%   +0.01%     
==========================================
  Files         457      457              
  Lines       21517    21522       +5     
  Branches     2324     2331       +7     
==========================================
+ Hits        19436    19444       +8     
+ Misses       1970     1967       -3     
  Partials      111      111              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stacktraceghost stacktraceghost force-pushed the 3171-check-validation-implementation-and-texts-against-specs branch from 859076b to 81579ce Compare April 21, 2026 11:46
@stacktraceghost stacktraceghost marked this pull request as ready for review April 21, 2026 11:47
@stacktraceghost stacktraceghost requested a review from a team as a code owner April 21, 2026 11:47
@stacktraceghost stacktraceghost requested review from Lionqueen94, jorisleker, jschuurk-kr and praseodym and removed request for a team April 21, 2026 11:47
Comment thread backend/src/domain/results/votes_counts.rs Outdated
Comment thread backend/src/domain/results/votes_counts.rs Outdated
Comment thread backend/src/domain/election.rs Outdated
@github-actions
Copy link
Copy Markdown

PDF Diff Summary

Comparing against base branch: main

File Status
model-n-10-2.pdf ✅ No changes
model-na-14-2-bijlage1.pdf ✅ No changes
model-na-14-2.pdf ✅ No changes
model-na-31-2-bijlage1.pdf ✅ No changes
model-na-31-2-inlegvel.pdf ✅ No changes
model-na-31-2.pdf ✅ No changes
model-p-22-2-bijlage1.pdf ✅ No changes
model-p-22-2.pdf ✅ No changes
model-p-2a.pdf ✅ No changes

@jschuurk-kr jschuurk-kr added this pull request to the merge queue Apr 22, 2026
Merged via the queue into main with commit b5f8b01 Apr 22, 2026
21 of 22 checks passed
@jschuurk-kr jschuurk-kr deleted the 3171-check-validation-implementation-and-texts-against-specs branch April 22, 2026 08:51
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.

Check validation implementation and texts against specs

4 participants