SAK-52184 Samigo fix Tally errors in some Total Scores/Question Pool/Item Analysis stats

[ottenhoff]

CC: @mylescarey2019 please test .... Code is untested and your Testing Plan is very detailed!

Summary by CodeRabbit

• Bug Fixes

  • More accurate scoring across various question types: correct/blank/incorrect answers are now tallied per submission to avoid double-counting and better reflect real responses.
  • Histograms and percent-correct displays now use per-submission aggregation, improving reliability of response counts and percentages.

• Refactor

  • Internal scoring and statistics workflows restructured for clearer, more maintainable tallying and aggregation logic.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Refactors tallying and scoring across TotalScoresBean, HistogramListener, and StatisticsService: introduces per-item tallying helpers, switches histogram and calculated-question logic to per-submission aggregation, and adjusts multiple-correct / fill-in scoring decisions and blank-handling.

Changes

Cohort / File(s)	Summary
Tallying and Scoring (TotalScoresBean) samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/bean/evaluation/TotalScoresBean.java	Added 7+ private helper methods (isTallyableItemType, getTallyableItems, getCorrectAnswerCountByItem, groupGradingsByItem, tallyItem, getFirstAnsweredGrading, getMatchingRequiredChoices). Refactored getResults() to per-item tallying and updated getIsOneSelectionType(). Added imports: LinkedHashMap, Objects, ItemTextIfc. Large internal logic change (+294/-19).
Histogram and Calculated Scores (HistogramListener) samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/listener/evaluation/HistogramListener.java	Expanded detail-inclusion to include MATRIX_CHOICES_SURVEY. Reworked per-question stats to use responded/blank counts, converted getCalculatedQuestionScores to per-submission aggregation (grouping by assessmentGradingId), and updated histogram bar height/percent calculations to use submission-aware totals. Adjusted EMI and survey paths and added NPE/zero safeguards (+114/-61).
Item Evaluation Rules (StatisticsService) samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/StatisticsService.java	Moved MULTIPLE_CORRECT_SINGLE_SELECTION_ID into single-correct handling path. Reworked fill-in logic to treat null/blank answers as blanks early. Enhanced multiple-correct evaluation with explicit blank/incorrect tracking and stricter correctness conditions to avoid double-counting (+23/-12).

Possibly related PRs

• PR #14341: Modifies HistogramListener to adopt submission-aware aggregation and adjusts histogram bar/percent calculations; closely related to the histogram and per-submission changes here.

Suggested reviewers

• ern
• jesusmmp

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies the main change: fixing tally errors in Samigo statistics components (Total Scores, Question Pool, Item Analysis).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.5)

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/listener/evaluation/HistogramListener.java

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/services/assessment/StatisticsService.java (1)

270-321: Blank FIB/FIN submissions can be double‑counted as incorrect.
When publishedAnswerId is null, blankResponses is incremented, but the post‑loop tally still falls through to the incorrect branch because blankAnswers stays 0. This can count one submission as both blank and incorrect. Consider tracking a per‑submission blank flag and short‑circuiting before the final tally.

Proposed fix

         for (Set<ItemGradingData> submissionItemGradingData : itemgradingDataByAssessmentGradingId.values()) {
             int presentAnswerCount = submissionItemGradingData.size();
             int correctAnswers = 0;
             int incorrectAnswers = 0;
             int blankAnswers = 0;
+            boolean hasNullAnswer = false;

             for (ItemGradingData itemGradingData : submissionItemGradingData) {
                 Long answerId = itemGradingData.getPublishedAnswerId();
                 if (answerId == null) {
                     // With a blank answer there should only one ItemGradingData per submission
                     // But to be safe, let's break out of the loop to avoid double counting
-                    blankResponses++;
-                    break;
+                    hasNullAnswer = true;
+                    break;
                 }
@@
-            if (blankAnswers == presentAnswerCount) {
+            if (hasNullAnswer) {
+                blankResponses++;
+                continue;
+            }
+            if (blankAnswers == presentAnswerCount) {
                 blankResponses++;
             } else if (correctAnswers == presentAnswerCount) {
                 correctResponses++;

samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/listener/evaluation/HistogramListener.java (1)

1851-1876: Histogram bar height should scale by submission count, not item‑gradings.
results now counts submissions, but bar heights divide by scores.size() (item gradings), which can under‑scale bars for multipart calculated questions. Use total submission responses instead.

Proposed fix

-        for (Map.Entry<String, Integer> resultEntry : results.entrySet()) {
+        int totalResponses = results.get(CORRECT) + results.get(INCORRECT);
+        for (Map.Entry<String, Integer> resultEntry : results.entrySet()) {
             HistogramBarBean bar = new HistogramBarBean();
             bar.setLabel(resultEntry.getKey());
             bar.setNumStudents(resultEntry.getValue());
             bar.setNumStudentsText(String.valueOf(resultEntry.getValue()));
             bar.setNumStudentsText(resultEntry.getValue() + " " + resultEntry.getKey());
             bar.setIsCorrect(resultEntry.getKey().equals(CORRECT));
             int height = 0;
-            if (!scores.isEmpty()) {
-                height = COLUMN_MAX_HEIGHT * resultEntry.getValue() / scores.size();
+            if (totalResponses > 0) {
+                height = COLUMN_MAX_HEIGHT * resultEntry.getValue() / totalResponses;
             }
             bar.setColumnHeight(Integer.toString(height));
             barList.add(bar);
         }
@@
-        int totalResponses = results.get(CORRECT) + results.get(INCORRECT);
         if (totalResponses > 0) {
             double percentCorrect = ((double) results.get(CORRECT) / (double) totalResponses) * 100;
             String percentCorrectStr = Integer.toString((int) percentCorrect);
             qbean.setPercentCorrect(percentCorrectStr);
         }

🤖 Fix all issues with AI agents

In
`@samigo/samigo-app/src/java/org/sakaiproject/tool/assessment/ui/bean/evaluation/TotalScoresBean.java`:
- Around line 544-562: In TotalScoresBean where TypeIfc.IMAGEMAP_QUESTION is
handled, change the blank-check logic for ItemGradingData answers so that
answerText values equal to the literal "undefined" are treated like blanks;
specifically update the condition that currently uses
StringUtils.isBlank(gradingData.getAnswerText()) to also consider
gradingData.getAnswerText() != null &&
gradingData.getAnswerText().trim().equals("undefined") (or the StringUtils
equivalent) when evaluating hasAnyResponse; keep the rest of the loop and return
semantics (gradingList, getPublishedAnswerId(), getAnswerText(), getIsCorrect())
unchanged.

[mylescarey2019]

@ottenhoff Thanks for working on this Sam. Have you finished work on this and applying any of the suggested AI changes? I.e. is this patch ready for me to begin testing?

Once this patch is ready for me to proceed I will be able do so, but do know that I will have to balance testing efforts against my current workload over the next couple days / week.

Thanks,
Myles Carey