diff --git a/app/mailers/project_notification_mailer.rb b/app/mailers/project_notification_mailer.rb index 4e209dc..4f163ea 100644 --- a/app/mailers/project_notification_mailer.rb +++ b/app/mailers/project_notification_mailer.rb @@ -8,4 +8,12 @@ def notify_subject_completion(user, context, completion_percentage) @email_to = user['email'] mail(to: @email_to, subject: "Your subjects are almost retired") end + + def notify_prediction_change(user, context, change_difference) + @change_difference = change_difference + @project_name = context.module_name + @workflow_name = context.extractor_name + @email_to = user['email'] + mail(to: @email_to, subject: "Significant change in Predictions") + end end diff --git a/app/services/prediction_results/process.rb b/app/services/prediction_results/process.rb index 4ea8fb0..cc43d70 100755 --- a/app/services/prediction_results/process.rb +++ b/app/services/prediction_results/process.rb @@ -6,6 +6,7 @@ module PredictionResults class Process SUBJECT_ACTION_API_BATCH_SIZE = ENV.fetch('SUBJECT_ACTION_API_BATCH_SIZE', '10').to_i COMPLETION_NOTIFICATION_THRESHOLD = ENV.fetch('COMPLETION_NOTIFICATION_THRESHOLD', '0.95').to_f + PREDICTION_CHANGE_THRESHOLD = ENV.fetch('COMPLETION_NOTIFICATION_THRESHOLD', '0.4').to_f attr_accessor :results_url, :subject_set_id, :probability_threshold, :over_threshold_subject_ids, :under_threshold_subject_ids, @@ -54,6 +55,7 @@ def partition_results @under_threshold_subject_ids << subject_id if probability < probability_threshold end check_completion_and_notify + check_prediction_change_and_notify # now add some 'spice' to the results by adding some random under threshold subject ids # but don't skew the prediction results by adding too many under threshold images # ensure we only use apply the randomisation factor to the count of over threshold subject ids @@ -90,12 +92,26 @@ def api_batch_bulk_job_args(subject_ids) end private - def check_completion_and_notify + def completion_rate total_under_threshold_subjects = @under_threshold_subject_ids.count - completion_rate = (total_under_threshold_subjects.to_f / @total_subjects.to_f) + @completion_rate ||= (total_under_threshold_subjects.to_f / @total_subjects.to_f) + end + + def check_completion_and_notify if completion_rate >= COMPLETION_NOTIFICATION_THRESHOLD - NotifyProjectOwnerJob.perform_async(subject_set_id, completion_rate) + NotifyProjectOwnerJob.perform_async(subject_set_id, @completion_rate) end end + + def check_prediction_change_and_notify + context = Context.find_by(active_subject_set_id: subject_set_id) + return unless context + difference = (completion_rate - context.last_completion_rate.to_f).abs + if (difference > PREDICTION_CHANGE_THRESHOLD) && context.last_completion_rate != 0 + NotifyProjectOwnerJob.perform_async(subject_set_id, difference, 'model_result_change') + end + + context.update(last_completion_rate: @completion_rate) + end end end diff --git a/app/sidekiq/notify_project_owner_job.rb b/app/sidekiq/notify_project_owner_job.rb index d0362a3..e3ff013 100644 --- a/app/sidekiq/notify_project_owner_job.rb +++ b/app/sidekiq/notify_project_owner_job.rb @@ -5,18 +5,31 @@ class NotifyProjectOwnerJob include Sidekiq::Job sidekiq_options retry: 5 - def perform(subject_set_id, completion_rate) + def perform(subject_set_id, completion_rate, action='subject_completion') @context = Context.find_by!(active_subject_set_id: subject_set_id) + @completion_rate = completion_rate + handle_notify(action) + end + private + def handle_notify(action) owner_link = fetch_project_owner owner_user = fetch_owner_user(owner_link['id']) - ProjectNotificationMailer - .notify_subject_completion(owner_user, @context, (completion_rate * 100)) - .deliver_now + case action + when 'subject_completion' + ProjectNotificationMailer + .notify_subject_completion(owner_user, @context, (@completion_rate * 100)) + .deliver_now + when 'model_result_change' + ProjectNotificationMailer + .notify_prediction_change(owner_user, @context, (@completion_rate * 100)) + .deliver_now + else + raise StandardError.new('No NotifyProjectOwnerJob action specified') + end end - private def fetch_project_owner(max_retries = 3) with_api_retry(max_retries) do resp = Panoptes::Api.client.project(@context.project_id) diff --git a/app/views/project_notification_mailer/notify_prediction_change.html.erb b/app/views/project_notification_mailer/notify_prediction_change.html.erb new file mode 100644 index 0000000..f7d6e86 --- /dev/null +++ b/app/views/project_notification_mailer/notify_prediction_change.html.erb @@ -0,0 +1,6 @@ + +

Prediction run for <%= @workflow_name %> workflow of project: <%= @project_name %> have a significant change of <%= @change_difference %>% from last week's run

+ +

Thanks,

+ +

The Zooniverse Team

diff --git a/db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb b/db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb new file mode 100644 index 0000000..3ef3948 --- /dev/null +++ b/db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb @@ -0,0 +1,5 @@ +class AddLastCompletionRateToContexts < ActiveRecord::Migration[7.0] + def change + add_column :contexts, :last_completion_rate, :float, default: 0.0 + end +end diff --git a/db/schema.rb b/db/schema.rb index b7d5d01..720ca7c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2025_04_27_195821) do +ActiveRecord::Schema[7.0].define(version: 2025_06_09_121625) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -52,6 +52,7 @@ t.string "module_name" t.string "extractor_name" t.jsonb "metadata" + t.float "last_completion_rate", default: 0.0 t.index ["workflow_id", "project_id"], name: "index_contexts_on_workflow_id_and_project_id", unique: true end diff --git a/spec/fixtures/contexts.yml b/spec/fixtures/contexts.yml index 8b74e77..719b5dd 100644 --- a/spec/fixtures/contexts.yml +++ b/spec/fixtures/contexts.yml @@ -7,6 +7,7 @@ galaxy_zoo_cosmic_active_learning_project: module_name: 'galaxy_zoo' extractor_name: 'cosmic_dawn' metadata: None + last_completion_rate: 0.1 galaxy_zoo_euclid_active_learning_project: id: 2 @@ -17,6 +18,7 @@ galaxy_zoo_euclid_active_learning_project: module_name: 'galaxy_zoo' extractor_name: 'euclid' metadata: None + last_completion_rate: 0.1 galaxy_zoo_cosmos_active_learning_project: id: 3 @@ -26,7 +28,9 @@ galaxy_zoo_cosmos_active_learning_project: pool_subject_set_id: 67 module_name: 'galaxy_zoo' extractor_name: 'jwst_cosmos' + last_completion_rate: 0.1 metadata: { + 'n_blocks': 2, 'fixed_crop':{ 'lower_left_x': 30, 'lower_left_y': 30, diff --git a/spec/mailers/project_notification_mailer_spec.rb b/spec/mailers/project_notification_mailer_spec.rb index 0f0719b..7cfc4b0 100644 --- a/spec/mailers/project_notification_mailer_spec.rb +++ b/spec/mailers/project_notification_mailer_spec.rb @@ -4,10 +4,9 @@ let(:user) { { 'email' => 'test@project@owner.com' } } let(:context) { Context.first } let(:completion_percentage) { 96 } - let(:mail) { ProjectNotificationMailer.notify_subject_completion(user, context, completion_percentage)} describe "#notify_subject_completion" do - + let(:mail) { ProjectNotificationMailer.notify_subject_completion(user, context, completion_percentage)} it "mails the correct user" do expect(mail.to).to include(user['email']) end @@ -32,4 +31,31 @@ expect(mail.body.encoded).to match("#{completion_percentage}%") end end + + describe "#notify_prediction_change" do + let(:mail) { ProjectNotificationMailer.notify_prediction_change(user, context, completion_percentage)} + it "mails the correct user" do + expect(mail.to).to include(user['email']) + end + + it 'comes from no-reply@zooniverse.org' do + expect(mail.from).to include('no-reply@zooniverse.org') + end + + it 'has the correct subject' do + expect(mail.subject).to eq("Significant change in Predictions") + end + + it 'has the project name in the body' do + expect(mail.body.encoded).to match("#{context.module_name}") + end + + it 'has the workflow name in the body' do + expect(mail.body.encoded).to match("#{context.extractor_name}") + end + + it 'has the completion percentage in the body' do + expect(mail.body.encoded).to match("#{completion_percentage}%") + end +end end diff --git a/spec/services/prediction_results/process_spec.rb b/spec/services/prediction_results/process_spec.rb index a65a2e3..ed34104 100755 --- a/spec/services/prediction_results/process_spec.rb +++ b/spec/services/prediction_results/process_spec.rb @@ -4,13 +4,16 @@ require 'remote_file/reader' RSpec.describe PredictionResults::Process do + fixtures :contexts + let(:confidence_threshold) { 0.8 } let(:remote_file) do # build a fake file we double as a result of the downloader Tempfile.new('remote-file-test') end let(:results_url) { 'https://fake.com/results.json' } - let(:active_subject_set_id) { 1 } + let(:context){ contexts(:galaxy_zoo_cosmic_active_learning_project) } + let(:active_subject_set_id) { context.active_subject_set_id } let(:process_results_service) { described_class.new(results_url: results_url, subject_set_id: active_subject_set_id) } let(:over_threshold_subject_id) { 1 } let(:under_threshold_subject_id) { 2 } @@ -94,15 +97,34 @@ expect(process_results_service.random_spice_subject_ids).to match_array([under_threshold_subject_id]) end - context 'when completion hits the notification threshold' do + context 'Notification triggers' do before do - stub_const("PredictionResults::Process::COMPLETION_NOTIFICATION_THRESHOLD", 0.5) allow(NotifyProjectOwnerJob).to receive(:perform_async) end - it 'calls NotifyProjectOwnerJob for almost retired subjects' do - process_results_service.partition_results - expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, 0.5) + context 'when completion hits the notification threshold' do + before do + stub_const("PredictionResults::Process::COMPLETION_NOTIFICATION_THRESHOLD", 0.5) + end + + it 'calls NotifyProjectOwnerJob for almost retired subjects' do + process_results_service.partition_results + expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, 0.5) + end + end + + context 'when model changes significantly' do + before do + stub_const("PredictionResults::Process::PREDICTION_CHANGE_THRESHOLD", 0.1) + end + + it 'calls NotifyProjectOwnerJob for model changes' do + process_results_service.partition_results + + completion_rate = (process_results_service.under_threshold_subject_ids.count.to_f / process_results_service.prediction_data.size.to_f).to_f + difference = (completion_rate - context.last_completion_rate.to_f).abs + expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, difference, 'model_result_change') + end end end end diff --git a/spec/sidekiq/notify_project_owner_job_spec.rb b/spec/sidekiq/notify_project_owner_job_spec.rb index 1033556..a5505eb 100755 --- a/spec/sidekiq/notify_project_owner_job_spec.rb +++ b/spec/sidekiq/notify_project_owner_job_spec.rb @@ -19,27 +19,55 @@ allow(panoptes_client_double).to receive(:project).and_return(fake_project_hash) allow(panoptes_client_double).to receive(:user).and_return(fake_user_hash) allow(Panoptes::Client).to receive(:new).and_return(panoptes_client_double) - allow(ProjectNotificationMailer) - .to receive(:notify_subject_completion) - .and_return(mailer_double) - job.perform(context.active_subject_set_id, completion_rate) end - it 'calls the api client to fetch project details' do - expect(panoptes_client_double).to have_received(:project).with(context.project_id) - end + context 'subject_completion' do + before do + allow(ProjectNotificationMailer) + .to receive(:notify_subject_completion) + .and_return(mailer_double) + job.perform(context.active_subject_set_id, completion_rate) + end + it 'calls the api client to fetch project details' do + expect(panoptes_client_double).to have_received(:project).with(context.project_id) + end - it 'calls the api client to fetch user details' do - expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) - end + it 'calls the api client to fetch user details' do + expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) + end + + it 'calls ProjectNotificationMailer notify_subject_completion' do + expect(ProjectNotificationMailer).to have_received(:notify_subject_completion).with(fake_user_hash, context, (completion_rate * 100)) + end + + it 'attempts to deliver the mail' do + expect(mailer_double).to have_received(:deliver_now) + end - it 'calls ProjectNotificationMailer notify_subject_completion' do - allow(ProjectNotificationMailer).to receive(:notify_subject_completion) - expect(ProjectNotificationMailer).to have_received(:notify_subject_completion).with(fake_user_hash, context, (completion_rate * 100)) end - it 'attempts to deliver the mail' do - expect(mailer_double).to have_received(:deliver_now) + context 'model_result_change' do + before do + allow(ProjectNotificationMailer) + .to receive(:notify_prediction_change) + .and_return(mailer_double) + job.perform(context.active_subject_set_id, completion_rate, 'model_result_change') + end + it 'calls the api client to fetch project details' do + expect(panoptes_client_double).to have_received(:project).with(context.project_id) + end + + it 'calls the api client to fetch user details' do + expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) + end + + it 'calls ProjectNotificationMailer notify_prediction_change' do + expect(ProjectNotificationMailer).to have_received(:notify_prediction_change).with(fake_user_hash, context, (completion_rate * 100)) + end + + it 'attempts to deliver the mail' do + expect(mailer_double).to have_received(:deliver_now) + end end end end