Skip to content

fix(text-response): fix null handling of old solution evaluation breakdowns#8473

Merged
adi-herwana-nus merged 1 commit into
masterfrom
adi/null-evaluation-result-fix
Jul 7, 2026
Merged

fix(text-response): fix null handling of old solution evaluation breakdowns#8473
adi-herwana-nus merged 1 commit into
masterfrom
adi/null-evaluation-result-fix

Conversation

@adi-herwana-nus

Copy link
Copy Markdown
Contributor

Bug

Rollbar flagged a NoMethodError: undefined method 'index_by' for nil when rendering
the text response answer partial
(_text_response.json.jbuilder).

The solutionResults block reads the persisted auto-grading result:

last_attempt.auto_grading.result['evaluation_results'].index_by { |result| result['solution_id'] }

The evaluation_results key was only introduced in commit b6da95a4ad
("feat(text-response): implement spreadsheet formula autograding"). Answers graded
before that commit have an auto-grading result containing messages but no
evaluation_results key, so the lookup returns nil and .index_by raises, 500-ing the
answer view.

The guard on the surrounding condition only checked that result existed, not that it
contained evaluation_results:

) && last_attempt&.auto_grading&.result

Remediation

Tighten the guard so the entire solutionResults block is skipped when
evaluation_results is absent.

We are not backfilling existing DB records. Legacy answers simply render without a
grading breakdown; a user can regrade the answer to repopulate evaluation_results
and restore the breakdown on demand.

The recovery path above depends on a grader being able to re-run autograding, but the
Re-evaluate Answer button in
QuestionGrade.tsx
was only rendered for programming and autogradable rubric-based-response questions. We extended the button's render condition to also cover autogradable text-response questions.

Scope check

Audited every evaluation_results reference across app, lib, and spec. The jbuilder
is the only read-site of the persisted auto_grading.result['evaluation_results']
hash. All other references are producer-side (building the results in the grading
services) and cannot be nil in the same way. The sibling result['messages'] read is
safe — messages has existed since the original grading service, so every record has it.

Tests

Added regression coverage to
answers_controller_spec.rb
(the #show action renders this jbuilder end-to-end via render_views)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a runtime error when rendering text-response answer JSON for legacy autograding results that predate the introduction of evaluation_results, and ensures instructors can re-run autograding for affected text-response questions to repopulate the missing breakdown.

Changes:

  • Guard solutionResults rendering in the text-response Jbuilder partial on presence of evaluation_results to avoid nil.index_by crashes for legacy records.
  • Extend the grading UI to show the Re-evaluate Answer button for autogradable text-response questions (in addition to programming questions).
  • Add controller-spec regression coverage for both legacy (no evaluation_results) and current (has evaluation_results) autograding result shapes when rendering #show.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
app/views/course/assessment/answer/text_responses/_text_response.json.jbuilder Skips solutionResults block unless evaluation_results exists, preventing nil.index_by 500s for older persisted results.
client/app/bundles/course/assessment/submission/containers/QuestionGrade.tsx Enables Re-evaluate Answer for autogradable text-response questions to allow regeneration of missing breakdown data.
spec/controllers/course/assessment/submission/answer/answers_controller_spec.rb Adds end-to-end #show regression tests covering both legacy and current autograding result structures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cysjonathan cysjonathan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@adi-herwana-nus adi-herwana-nus enabled auto-merge (rebase) July 7, 2026 09:01
@adi-herwana-nus adi-herwana-nus merged commit f3a9d20 into master Jul 7, 2026
11 checks passed
@adi-herwana-nus adi-herwana-nus deleted the adi/null-evaluation-result-fix branch July 7, 2026 09:02
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.

3 participants