feat(submission): offload background operations from finalise transaction

[adi-herwana-nus]

The third mitigation following the duplicate course_personal_times incident, after
#8559 (client fix: debounce the lesson plan
editor so it stops firing overlapping PATCHes) and
#8560 (unique index on
(course_user_id, lesson_plan_item_id)).

Background

Two Course::LessonPlan::CoursewidePersonalizedTimelineUpdateJob runs executing concurrently each
inserted a Course::PersonalTime row for the same (course_user_id, lesson_plan_item_id) pair.
Neither failed: the uniqueness check was a model-level validation with no backing index, so neither
transaction saw the other's uncommitted row.

Once a duplicate pair existed, every later personalisation run for that course user failed
deterministically — for an existing record the validation excludes only itself, so it always found the
twin. Finalising a submission recomputes the submitter's timeline, and did so inside the same
transaction that commits the student's work. So the exception unwound the whole submission: affected
students could not finalise anything in that course, on every retry, until we intervened.

#8560 stops the duplicates being created. This PR stops a failure in derived, recomputable state from
being able to destroy student work in the first place.

What moves out of the transaction

The transaction is opened in Course::Assessment::Submission::UpdateService#update_submission and
covers both the answer writes and the workflow transition — the finalise handler's save! only
opens a joined nested transaction, so every after_save callback is part of it too.

1. Personalised timeline recomputation

update_personalized_timeline_for_user is replaced by a new
Course::LessonPlan::PersonalizedTimelineUpdateJob, enqueued from
ActiveRecord.after_all_transactions_commit.

This is by far the largest unit of work in the transaction: precompute_data loads every lesson plan
item in the course with its reference and personal times, builds the user's submission-time hash and
computes a learning-rate EMA over it; execute then walks that list and inserts or updates a personal
time per shiftable item. It is also the only part that contends with the coursewide job for the same
rows.

Enqueuing after commit has an incidental benefit: the job now reads a committed submission, which is
what submitted_items was always meant to include.

Course::LessonPlan::PersonalizationConcern is no longer included in the submission model.

2. Cikgo task completion

publish_task_completion is a synchronous Excon PATCH with a five second timeout. It moves into a
new Course::Assessment::Submission::PublishTaskCompletionJob, enqueued after commit, so the request
neither waits on Cikgo nor fails because of it.

Two things worth knowing when reviewing this one:

• On attempting → submitted it is semantically a no-op. WORKFLOW_STATE_TO_TASK_COMPLETION_STATUS
  maps both attempting and submitted to :ongoing, so finalising pushes the status Cikgo already
  has. Only → published carries new information.
• It was not rolling back finalise in production — the rescue is raise e unless Rails.env.production?, so production logs and swallows. The production cost was up to five seconds
  of an open transaction holding locks on the submission and its answers while waiting on a third
  party. Outside production the behaviour was the inverted one, where a Cikgo hiccup did roll a
  finalise back.

The method is split so the two call sites get the error handling each needs, rather than one swallowing
compromise:

• publish_task_completion! pushes and lets failures propagate. The job uses it, so a failure is
  retried by Sidekiq and reported to Rollbar (ApplicationJob includes Rollbar::ActiveJob, which
  reports and re-raises) instead of vanishing into the log.
• publish_task_completion keeps the existing log-and-swallow-in-production behaviour for
  SubmissionsController#publish_cikgo_task_completion, the after_action on edit that repairs a
  missed push. That path is unchanged by this PR.

The job re-reads should_publish_task_completion? at run time rather than trusting the state at
enqueue, so a superseded push is dropped and two pushes racing each other converge on the current
status.

3. Submission notification

Email delivery was already deferred to after commit by Notifier::Base::ActivityWrapper. The queries
behind it were not: building the activity walks the submitter's groups to find managers, falls back to
course managers, subtracts the unsubscribed ones, and writes the activity and its notification rows.
That now happens after commit too. The guards stay inline, since they read
workflow_state_before_last_save.

This one stays on the request rather than becoming a job of its own, since its real work is enqueuing
mail delivery jobs.

What deliberately stays

• Answer updates, finalise_current_answers, submitted_at — the student's work and the
  definition of the state transition.
• assign_zero_experience_points — not a side effect. An assessment with no questions finalises
  straight to published, skipping the states where grading would assign points, and published
  carries validate_awarded_attributes. It is three in-memory assignments on the record being saved,
  persisted by the outer update!'s own save!; there is no separate write to move, and deferring it
  would mean saving a record its own validation rejects.
• update_todo — one row in the same database. Deferring it buys no latency and would leave the
  student's landing page showing a submitted assessment as still in progress, with nothing to
  reconcile it.

Consequences if the offloaded work fails

The point of the change is that these are now recoverable rather than destructive, so they are worth
stating explicitly:

Offloaded work	If it fails after the finalise commits	Recovery
Timeline recomputation	The student's timeline is one submission stale but still internally valid — upcoming items keep the dates from the previous recomputation.	Sidekiq retries. Any later recomputation for that user (their next submission, "Recompute all times" on their personal times page, or a coursewide run) brings it forward. Note the strategies refuse to move dates that have already passed, so the repair is complete only for items that have not yet opened.
Course::LearningRateRecord row	A gap in a log. It is written after execute and never read back — compute_learning_rate_ema re-derives from submissions and personal times each run. Its only readers are the personal times and course user pages.	None needed.
Other submissions' force-submit schedule	force_submit_at derives from the timeline, so a stale timeline can leave a job scheduled against a deadline that would have shifted.	Self-corrects. ScheduleExpiringSubmissionsJob runs hourly and reschedules any submission whose force_submit_at no longer matches its recorded force_submit_scheduled_at, and ForceSubmitTimedSubmissionJob re-checks live before submitting.
Cikgo push	Cikgo's copy of the task shows a stale status — usually no observable difference on finalise, given the status mapping above.	Sidekiq retries, and the failure is reported to Rollbar. Beyond that it self-heals: SubmissionsController already re-pushes on edit.
Submission notification	Managers do not get the "new submission" email and the activity feed misses an entry.	None. Low impact — graders still see the submission in the submissions list.

Why this is safe without locking

Offloading widens the window in which the recomputation can overlap a coursewide run. That overlap was
always possible — the finalise transaction took no lock that excluded the job, which is exactly how the
duplicates were created — so this is a widening, not a new exposure. With #8560 already merged, what
changes is the outcome when two runs do collide:

	before #8560	after #8560, before this PR	after this PR
Two runs insert the same pair	both commit; a duplicate is created silently	one commits, the other raises inside the finalise transaction	one commits, the other raises inside a job
Cost to the loser	a poisoned row that fails every later run	the student's submission is rolled back	a Sidekiq retry, which finds the row and succeeds

Locking would reduce the collisions themselves, but nothing here depends on it: a collision is now
transient and self-correcting, and a run that fails permanently leaves a stale timeline rather than
lost work.

Behaviour changes

• Finalising no longer recomputes the timeline before the response returns. The student's own next
  deadlines may lag their submission by however long the job queue takes. The recomputation runs on
  :lowest, matching CoursewidePersonalizedTimelineUpdateJob.
• A failed recomputation is now a retried job reported to Rollbar, instead of a silently rolled back
  submission.
• A Cikgo outage no longer delays or fails a finalise in any environment, and — new — is now visible
  in Rollbar rather than only in the production log.
• Nothing that finalise defers can turn a committed submission into an error response. Only three
  perform_later/notifier calls remain on the request after the commit, and an enqueue failure there
  would already be failing auto_grade_submission on the same path.

Testing

New spec/jobs/course/lesson_plan/personalized_timeline_update_job_spec.rb: the job is queued,
enqueued by finalising, shifts only the unsubmitted item, no-ops on the fixed algorithm, and — the one
that matters here — running it twice neither duplicates rows nor drifts.

New spec/jobs/course/assessment/submission/publish_task_completion_job_spec.rb: the job pushes the
current status, drops a push whose submission no longer qualifies, and lets a failed push propagate,
which is what puts it in front of Sidekiq's retries and Rollbar.

New work deferred past the finalise transaction group in
spec/models/course/assessment/submission_spec.rb, asserting each of the three is invisible while the
transaction is open, absent entirely if it rolls back, and — for the Cikgo push — enqueued rather than
called during the request.

Checked against the old behaviour by restoring master's three concerns and re-running the group: 4
of its 6 examples fail, including the Cikgo rollback case, which errors on master because the
inline push raises inside the transaction. The two that pass beforehand are the rollback cases for the
timeline and the activity — vacuously, since on master that work was inside the transaction and rolled
back with it. They are kept as forward guards against the deferral being moved back out of the
after-commit hook.

The notification assertion goes through Activity.count rather than a message expectation, because
assessment_submitted is reached via Notifier::Base.method_missing and cannot be a verified double.

One spec-infrastructure note for reviewers. The offload makes the suite run these jobs concurrently
with example bodies, since the background thread adapter only joins them when the example ends. That
surfaced a real failure in personalization_concern_spec.rb — three per-user recomputations racing the
coursewide run under test, which is the incident in miniature. This PR adds a wait_for_enqueued_jobs
helper to spec/support/active_job.rb and settles the setup before that example. Other specs that
finalise a submission in setup and then assert on personal times may need the same.

Run locally: the submission model, controller, service, job, personalisation-concern and notifier specs
all pass. Feature specs run: spec/features/course/assessment/assessment_attempt_spec.rb and all of
spec/features/course/assessment/submission/ — 66 examples, one failure, which is a pre-existing local
Keycloak login problem (submissions_spec.rb:54 dies in login_as before it reaches a submission, and
fails identically with this branch's changes reverted).

Follow-ups, not in this PR

• Course::PersonalTimesController#recompute still runs the recomputation synchronously in the
  request, with the same "hundreds of items in one request" shape removed from finalise here. It should
  go through the same job.
• Course::LearningRateRecord's timestamp is effectively a "last successfully recomputed at" marker,
  and surfacing it on the personal times page would make a failed recomputation visible to
  instructors. Useful, but a feature rather than part of this fix.
• Locking around the personalisation entry points, as described above — optional given the index.
• The analogous transactions for other lesson plan item classes that recompute timelines on submission
  were not examined; only the assessment path is covered here.

[Copilot]

🟡 Changes recommended

The after-commit Cikgo push can still raise outside production, causing finalise/update calls to error after the DB commit, which is a correctness/UX regression relative to the stated intent.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Moves expensive and failure-prone side effects out of the submission finalise transaction so derived-state failures (timeline recomputation, notifications, Cikgo push) can’t roll back committed student work.

Changes:

• Enqueue per-user personalised timeline recomputation via a new Course::LessonPlan::PersonalizedTimelineUpdateJob after all transactions commit.
• Defer Cikgo task status push and submission activity/notification creation to after_all_transactions_commit.
• Add/adjust specs and test helpers to ensure deferred work is observable only after commit and to reduce background-thread job races in specs.

File summaries

File	Description
spec/support/active_job.rb	Adds wait_for_enqueued_jobs helper to block on background-thread adapter execution during spec setup.
spec/models/course/assessment/submission_spec.rb	Adds commit-boundary assertions for deferred timeline job, activity creation, and Cikgo push behavior.
spec/jobs/course/lesson_plan/personalized_timeline_update_job_spec.rb	New job spec covering enqueueing, shifting behavior, and idempotency.
spec/controllers/concerns/course/lesson_plan/personalization_concern_spec.rb	Settles per-user recomputation jobs before running coursewide recomputation assertions.
app/models/concerns/course/assessment/submission/workflow_event_concern.rb	Replaces inline timeline recompute with after-commit job enqueue.
app/models/concerns/course/assessment/submission/notification_concern.rb	Defers activity/notification creation until after commit.
app/models/concerns/course/assessment/submission/cikgo_task_completion_concern.rb	Defers Cikgo push until after commit via a wrapper callback.
app/jobs/course/lesson_plan/personalized_timeline_update_job.rb	New ActiveJob to recompute a single course user’s personalised timeline under the correct tenant.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.