From 884cfa1c19104d494923931b58e667eb2a31bb9c Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 25 Feb 2026 14:54:51 +0000 Subject: [PATCH 1/8] Prevent coord sig figs defaulting to 2 when they should be ignored We use null in this context to mean "ignore significant figures", but this was previously causing the required sig figs to default to NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES (2). --- .../ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 548972a21e..1451489682 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -251,7 +251,11 @@ private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final C String submittedValue = submittedItem.getCoordinates().get(dimension); String choiceValue = choiceItem.getCoordinates().get(dimension); - int sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); + Integer sigFigs = null; + if (null != sigFigsMin || null != sigFigsMax) { + sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); + } + if (!ValidationUtils.numericValuesMatch(choiceValue, submittedValue, sigFigs, log) || null != sigFigsMin && ValidationUtils.tooFewSignificantFigures(submittedValue, sigFigsMin, log) || null != sigFigsMax && ValidationUtils.tooManySignificantFigures(submittedValue, sigFigsMax, log)) { From 67114d2cfd7ca9e78b60738e0eee8577bc64dca5 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 11:59:56 +0000 Subject: [PATCH 2/8] Bring coord sig fig feedback in line with numeric questions --- .../isaac/quiz/IsaacCoordinateValidator.java | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 1451489682..567e147269 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -45,6 +45,9 @@ public final QuestionValidationResponse validateQuestionResponse(final Question IsaacCoordinateQuestion coordinateQuestion = (IsaacCoordinateQuestion) question; CoordinateChoice submittedChoice = (CoordinateChoice) answer; + Integer sigFigsMin = coordinateQuestion.getSignificantFiguresMin(); + Integer sigFigsMax = coordinateQuestion.getSignificantFiguresMax(); + // STEP 0: Is it even possible to answer this question? if (null == coordinateQuestion.getChoices() || coordinateQuestion.getChoices().isEmpty()) { @@ -127,10 +130,6 @@ public final QuestionValidationResponse validateQuestionResponse(final Question } List choiceItems = coordinateChoice.getItems().stream().map(i -> (CoordinateItem) i).collect(Collectors.toList()); - // Get significant figures to validate with - Integer sigFigsMin = coordinateQuestion.getSignificantFiguresMin(); - Integer sigFigsMax = coordinateQuestion.getSignificantFiguresMax(); - // Check that the items in the submitted answer match the items in the choice numerically boolean allItemsMatch = false; @@ -169,7 +168,8 @@ public final QuestionValidationResponse validateQuestionResponse(final Question break; } else if (allItemsMatchWithoutSigFigs) { feedback = new Content(DEFAULT_VALIDATION_RESPONSE); - feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs"))); + // Too many sig figs, or too few sig figs but the choice has trailing zeros; otherwise correct + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); break; } @@ -179,23 +179,35 @@ public final QuestionValidationResponse validateQuestionResponse(final Question // For correct choices, check if the submitted items are a proper subset of the choice if (coordinateChoice.isCorrect() && (choiceItems.size() > submittedItems.size())) { boolean allSubmittedItemsInChoiceItems = true; + boolean allItemsInChoiceWithoutSigFigs = true; for (CoordinateItem submittedItem : submittedItems) { boolean submittedItemInChoiceItem = false; + boolean itemInChoiceWithoutSigFigs = false; for (CoordinateItem choiceItem : choiceItems) { if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { submittedItemInChoiceItem = true; break; + } else if (coordinateItemsMatch(submittedItem, choiceItem, null, null)) { + itemInChoiceWithoutSigFigs = true; } } if (!submittedItemInChoiceItem) { allSubmittedItemsInChoiceItems = false; - break; + // Check if this is just a significant figures mismatch + if (!itemInChoiceWithoutSigFigs) { + allItemsInChoiceWithoutSigFigs = false; + break; + } } } if (allSubmittedItemsInChoiceItems) { feedback = new Content((submittedItems.size() == 1 ? "This is" : "These are") + " correct, but can you find more?"); break; + } else if (allItemsInChoiceWithoutSigFigs) { + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); + break; } } @@ -232,9 +244,18 @@ public final QuestionValidationResponse validateQuestionResponse(final Question } } - // STEP 3: If we still have no feedback to give, use the question's default feedback if any to use: - if (feedbackIsNullOrEmpty(feedback) && null != coordinateQuestion.getDefaultFeedback()) { - feedback = coordinateQuestion.getDefaultFeedback(); + // STEP 3: If incorrect and we still have no feedback to give, use the question's default feedback if any to use: + if (!responseCorrect && feedbackIsNullOrEmpty(feedback)) { + if (null != coordinateQuestion.getDefaultFeedback()) { + feedback = coordinateQuestion.getDefaultFeedback(); + } else { + // If there was no default feedback, check for too few significant figures + if (submittedItems.stream().anyMatch(i -> i.getCoordinates().stream() + .anyMatch(c -> ValidationUtils.tooFewSignificantFigures(c, sigFigsMin, log)))) { + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_few"))); + } + } } return new QuestionValidationResponse(question.getId(), answer, responseCorrect, feedback, new Date()); @@ -251,10 +272,8 @@ private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final C String submittedValue = submittedItem.getCoordinates().get(dimension); String choiceValue = choiceItem.getCoordinates().get(dimension); - Integer sigFigs = null; - if (null != sigFigsMin || null != sigFigsMax) { - sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); - } + // Defaults to 2 if min/max null + int sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); if (!ValidationUtils.numericValuesMatch(choiceValue, submittedValue, sigFigs, log) || null != sigFigsMin && ValidationUtils.tooFewSignificantFigures(submittedValue, sigFigsMin, log) From 12a8713022b09308380defd96dbf61998b92cb15 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 14:12:10 +0000 Subject: [PATCH 3/8] Fix subset sig figs logic Don't break early on finding an incorrect sig figs match, in case another choice is a correct sig figs match. --- .../ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 567e147269..dca05e7f8f 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -203,18 +203,19 @@ public final QuestionValidationResponse validateQuestionResponse(final Question if (allSubmittedItemsInChoiceItems) { feedback = new Content((submittedItems.size() == 1 ? "This is" : "These are") + " correct, but can you find more?"); + feedback.setTags(new HashSet<>()); // Clear tags in case we previously set sig figs tags break; } else if (allItemsInChoiceWithoutSigFigs) { feedback = new Content(DEFAULT_VALIDATION_RESPONSE); feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); - break; + // Don't break; we could find a better match } } // If subset matching is allowed for this choice, check if the choice is a proper subset of the // submitted items boolean allowSubsetMatch = (null != coordinateChoice.isAllowSubsetMatch() && coordinateChoice.isAllowSubsetMatch()); - if (allowSubsetMatch && (submittedItems.size() > choiceItems.size())) { + if (null == feedback && allowSubsetMatch && (submittedItems.size() > choiceItems.size())) { boolean allChoiceItemsInSubmittedItems = true; for (CoordinateItem choiceItem : choiceItems) { boolean choiceItemInSubmittedItems = false; From 276825cc95d0759795347023181f9ef18272c562 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 16:34:45 +0000 Subject: [PATCH 4/8] Prevent sig fig feedback on incorrect values Don't show sig fig feedback for an incorrect value with the correct number of sig figs that matches a choice value to 2s.f. --- .../isaac/quiz/IsaacCoordinateValidator.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index dca05e7f8f..8e8ceaa539 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -150,10 +150,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question CoordinateItem choiceItem = choiceItems.get(coordIndex); CoordinateItem submittedItem = submittedItems.get(coordIndex); // Check that the submitted item matches the choice item - if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { + if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { allItemsMatch = false; // Check if this is just a significant figures mismatch - if (!coordinateItemsMatch(submittedItem, choiceItem, null, null)) { + if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { allItemsMatchWithoutSigFigs = false; // Exit early on mismatch: break; @@ -184,10 +184,12 @@ public final QuestionValidationResponse validateQuestionResponse(final Question boolean submittedItemInChoiceItem = false; boolean itemInChoiceWithoutSigFigs = false; for (CoordinateItem choiceItem : choiceItems) { - if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { + if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, + false)) { submittedItemInChoiceItem = true; break; - } else if (coordinateItemsMatch(submittedItem, choiceItem, null, null)) { + } else if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, + true)) { itemInChoiceWithoutSigFigs = true; } } @@ -220,7 +222,7 @@ public final QuestionValidationResponse validateQuestionResponse(final Question for (CoordinateItem choiceItem : choiceItems) { boolean choiceItemInSubmittedItems = false; for (CoordinateItem submittedItem : submittedItems) { - if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { + if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { choiceItemInSubmittedItems = true; break; } @@ -263,7 +265,8 @@ public final QuestionValidationResponse validateQuestionResponse(final Question } private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final CoordinateItem choiceItem, - final Integer sigFigsMin, final Integer sigFigsMax) { + final Integer sigFigsMin, final Integer sigFigsMax, + final boolean allowTooManySigFigs) { if (submittedItem.getCoordinates().size() != choiceItem.getCoordinates().size()) { return false; @@ -273,9 +276,15 @@ private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final C String submittedValue = submittedItem.getCoordinates().get(dimension); String choiceValue = choiceItem.getCoordinates().get(dimension); - // Defaults to 2 if min/max null - int sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); + if (allowTooManySigFigs) { + // Check if the submission has more significant figures than the allowed maximum + if (null != sigFigsMax && ValidationUtils.tooManySignificantFigures(submittedValue, sigFigsMax, log)) { + // Check if the submission would match the choice if we ignore the excess significant figures + return ValidationUtils.numericValuesMatch(choiceValue, submittedValue, sigFigsMax, log); + } + } + int sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); if (!ValidationUtils.numericValuesMatch(choiceValue, submittedValue, sigFigs, log) || null != sigFigsMin && ValidationUtils.tooFewSignificantFigures(submittedValue, sigFigsMin, log) || null != sigFigsMax && ValidationUtils.tooManySignificantFigures(submittedValue, sigFigsMax, log)) { From 62f0e276add3e825f1e15f43a4cf496713579528 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 16:42:17 +0000 Subject: [PATCH 5/8] Show sig fig feedback for allowSubsetMatch choices --- .../cl/dtg/isaac/quiz/IsaacCoordinateValidator.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 8e8ceaa539..06c2f3626c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -219,23 +219,36 @@ public final QuestionValidationResponse validateQuestionResponse(final Question boolean allowSubsetMatch = (null != coordinateChoice.isAllowSubsetMatch() && coordinateChoice.isAllowSubsetMatch()); if (null == feedback && allowSubsetMatch && (submittedItems.size() > choiceItems.size())) { boolean allChoiceItemsInSubmittedItems = true; + boolean allItemsInSubmittedWithoutSigFigs = true; for (CoordinateItem choiceItem : choiceItems) { boolean choiceItemInSubmittedItems = false; + boolean itemInSubmittedWithoutSigFigs = false; for (CoordinateItem submittedItem : submittedItems) { if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { choiceItemInSubmittedItems = true; break; + } else if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { + itemInSubmittedWithoutSigFigs = true; } } if (!choiceItemInSubmittedItems) { allChoiceItemsInSubmittedItems = false; + // Check if this is just a significant figures mismatch + if (!itemInSubmittedWithoutSigFigs) { + allItemsInSubmittedWithoutSigFigs = false; + } break; } } if (allChoiceItemsInSubmittedItems) { responseCorrect = coordinateChoice.isCorrect(); feedback = (Content) coordinateChoice.getExplanation(); + feedback.setTags(new HashSet<>()); // Clear tags in case we previously set sig figs tags break; + } else if (allItemsInSubmittedWithoutSigFigs) { + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); + // Don't break; we could find a better match } } } From 33ddd137e19f6817c1d0740dfd436cd8194bbd05 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 16:56:04 +0000 Subject: [PATCH 6/8] Simplify validator logic Assume there will not be distinct choices that differ only by significant figures. --- .../ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 06c2f3626c..c1a828187f 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -205,12 +205,11 @@ public final QuestionValidationResponse validateQuestionResponse(final Question if (allSubmittedItemsInChoiceItems) { feedback = new Content((submittedItems.size() == 1 ? "This is" : "These are") + " correct, but can you find more?"); - feedback.setTags(new HashSet<>()); // Clear tags in case we previously set sig figs tags break; } else if (allItemsInChoiceWithoutSigFigs) { feedback = new Content(DEFAULT_VALIDATION_RESPONSE); feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); - // Don't break; we could find a better match + break; } } @@ -243,12 +242,11 @@ public final QuestionValidationResponse validateQuestionResponse(final Question if (allChoiceItemsInSubmittedItems) { responseCorrect = coordinateChoice.isCorrect(); feedback = (Content) coordinateChoice.getExplanation(); - feedback.setTags(new HashSet<>()); // Clear tags in case we previously set sig figs tags break; } else if (allItemsInSubmittedWithoutSigFigs) { feedback = new Content(DEFAULT_VALIDATION_RESPONSE); feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); - // Don't break; we could find a better match + break; } } } From fe5d03907296884f23d64a921a55f281055f8b46 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 16:58:32 +0000 Subject: [PATCH 7/8] Remove redundant null check --- .../uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index c1a828187f..291d923545 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -216,7 +216,7 @@ public final QuestionValidationResponse validateQuestionResponse(final Question // If subset matching is allowed for this choice, check if the choice is a proper subset of the // submitted items boolean allowSubsetMatch = (null != coordinateChoice.isAllowSubsetMatch() && coordinateChoice.isAllowSubsetMatch()); - if (null == feedback && allowSubsetMatch && (submittedItems.size() > choiceItems.size())) { + if (allowSubsetMatch && (submittedItems.size() > choiceItems.size())) { boolean allChoiceItemsInSubmittedItems = true; boolean allItemsInSubmittedWithoutSigFigs = true; for (CoordinateItem choiceItem : choiceItems) { From 97ec917e05cc340996910718f645f9c809c0db00 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 27 Feb 2026 17:03:54 +0000 Subject: [PATCH 8/8] Restore default feedback for correct choices --- .../isaac/quiz/IsaacCoordinateValidator.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 291d923545..7c13167e88 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -258,18 +258,15 @@ public final QuestionValidationResponse validateQuestionResponse(final Question } } - // STEP 3: If incorrect and we still have no feedback to give, use the question's default feedback if any to use: - if (!responseCorrect && feedbackIsNullOrEmpty(feedback)) { - if (null != coordinateQuestion.getDefaultFeedback()) { - feedback = coordinateQuestion.getDefaultFeedback(); - } else { - // If there was no default feedback, check for too few significant figures - if (submittedItems.stream().anyMatch(i -> i.getCoordinates().stream() + // STEP 3: If we still have no feedback to give, use the question's default feedback if any to use: + if (feedbackIsNullOrEmpty(feedback) && null != coordinateQuestion.getDefaultFeedback()) { + feedback = coordinateQuestion.getDefaultFeedback(); + } + // If there was no default feedback, check for too few significant figures + if (feedbackIsNullOrEmpty(feedback) && submittedItems.stream().anyMatch(i -> i.getCoordinates().stream() .anyMatch(c -> ValidationUtils.tooFewSignificantFigures(c, sigFigsMin, log)))) { - feedback = new Content(DEFAULT_VALIDATION_RESPONSE); - feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_few"))); - } - } + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_few"))); } return new QuestionValidationResponse(question.getId(), answer, responseCorrect, feedback, new Date());