Merge pull request #2269 from broadinstitute/development

Release 1.97.0

Author: bistline

app/controllers/api/v1/study_files_controller.rb

@@ -388,7 +388,7 @@ def perform_update!(study_file)
           end
         end
 
-        if ['Expression Matrix', 'MM Coordinate Matrix'].include?(study_file.file_type) && !safe_file_params[:y_axis_label].blank?
+        if ['Expression Matrix', 'MM Coordinate Matrix'].include?(study_file.file_type) && safe_file_params[:y_axis_label].present?
           # if user is supplying an expression axis label, update default options hash
           study.default_options[:expression_label] = safe_file_params[:y_axis_label]
           study.save
@@ -400,9 +400,9 @@ def perform_update!(study_file)
           end
         end
 
-        if safe_file_params[:upload].present? && !is_chunked ||
-          safe_file_params[:remote_location].present? ||
-          study_file.needs_raw_counts_extraction?
+        if (safe_file_params[:upload].present? && !is_chunked) ||
+           safe_file_params[:remote_location].present? ||
+           study_file.needs_raw_counts_extraction?
           complete_upload_process(study_file, parse_on_upload)
         end
       end

app/lib/cluster_cache_service.rb

@@ -74,4 +74,54 @@ def self.cache_study_defaults(study)
       Rails.logger.error "Error in caching defaults for #{study.accession}: (#{e.class.name}) #{e.message}"
     end
   end
+
+  # set the default annotation for a study to the most relevant annotation available,
+  # such as cell_type__ontology_label or seurat_clusters
+  def self.configure_default_annotation(study)
+    return false if default_annotation_configured?(study)
+
+    best_available = best_available_annotation(study)
+    return false if best_available.nil?
+
+    # use default_options[:annotation] as default_annotation has fallback logic and we want the 'configured' value
+    existing_default = study.default_options[:annotation]
+    return false if best_available == existing_default
+
+    study.default_options[:annotation] = best_available
+    Rails.logger.info "Changing default annotation in #{study.accession} from " \
+                        "#{existing_default.presence || 'unassigned'} to #{best_available}"
+    study.save(validate: false) # prevent validation errors for older studies
+    log_props = {
+      studyAccession: study.accession,
+      default_annotation: best_available,
+      previous_annotation: existing_default
+    }
+    MetricsService.log('study-default-annotation', log_props, study.user)
+  end
+
+  # find the most relevant annotation to display as the default
+  # will prioritize convention-based cell types, then other 'cell type'-like annotations,
+  # followed by clustering algorithms and anything label/categorical in nature
+  def self.best_available_annotation(study)
+    annotations = DifferentialExpressionService.find_eligible_annotations(study)
+    return nil if annotations.empty?
+
+    ontology = annotations.detect { |a| a[:annotation_name] == 'cell_type__ontology_label' }
+    author_cell_type = annotations.detect { |a| a[:annotation_name] =~ /author.*cell.*type/i }
+    cell_type = annotations.detect { |a| a[:annotation_name] =~ DifferentialExpressionService::CELL_TYPE_MATCHER }
+    clustering = annotations.detect { |a| a[:annotation_name] =~ DifferentialExpressionService::CLUSTERING_MATCHER }
+    category = annotations.detect { |a| a[:annotation_name] =~ DifferentialExpressionService::CATEGORY_MATCHER }
+    best_avail = ontology || author_cell_type || cell_type || clustering || category
+
+    best_avail.present? ? [best_avail[:annotation_name], 'group', best_avail[:annotation_scope]].join('--') : nil
+  end
+
+  # helper to determine if a user set the default annotation manually by checking HistoryTracker for events
+  def self.default_annotation_configured?(study)
+    study.history_tracks.detect do |track|
+      track.original.dig('default_options', 'annotation').present? &&
+        track.original.dig('default_options', 'annotation') != '' &&
+        track.modified.dig('default_options', 'annotation') != track.original.dig('default_options', 'annotation')
+    end.present?
+  end
 end

app/lib/differential_expression_service.rb

@@ -5,8 +5,10 @@ class DifferentialExpressionService
   CELL_TYPE_MATCHER = /cell.*type/i
   # possible clustering algorithm results
   CLUSTERING_MATCHER = /(clust|seurat|leiden|louvain|snn_res)/i
+  # any categorical-type labels
+  CATEGORY_MATCHER = /(categor|labels)/i
   # union of all allowed annotations
-  ALLOWED_ANNOTS = Regexp.union(CELL_TYPE_MATCHER, CLUSTERING_MATCHER)
+  ALLOWED_ANNOTS = Regexp.union(CELL_TYPE_MATCHER, CLUSTERING_MATCHER, CATEGORY_MATCHER)
   # specific annotations to exclude from automation
   EXCLUDED_ANNOTS = /(enrichment__cell_type)/i
   # default quota for user-requested DE jobs
@@ -250,7 +252,7 @@ def self.find_eligible_annotations(study, skip_existing: false)
     annotations += metadata.map { |meta| { annotation_name: meta.name, annotation_scope: 'study' } }
     # special gotcha to remove 'cell_type' metadata annotation if 'cell_type__ontology_label' is present
     if annotations.detect { |annot| annot[:annotation_name] == 'cell_type__ontology_label' }.present?
-      annotations.reject! { |annot| annot[:annotation_name] == 'cell_type'}
+      annotations.reject! { |annot| annot[:annotation_name] == 'cell_type' }
     end
     cell_annotations = []
     groups_to_process = study.cluster_groups.select { |cg| cg.cell_annotations.any? }

app/lib/metrics_service.rb

@@ -123,7 +123,6 @@ def self.log(name, props={}, user=nil, request: nil)
       headers: headers,
       payload: post_body
     }
-
     self.post_to_bard(params, user)
   end
 

app/models/ingest_job.rb

@@ -557,6 +557,8 @@ def cluster_name_by_file_type
 
   # set the default annotation for the study, if not already set
   def set_default_annotation
+    ClusterCacheService.configure_default_annotation(study)
+    study.reload
     return if study.default_options[:annotation].present?
 
     cell_metadatum = study.cell_metadata.keep_if(&:can_visualize?).first || study.cell_metadata.first
@@ -1062,9 +1064,29 @@ def anndata_summary_props
         pipeline_args.detect { |c| c == study_file.id.to_s } &&
         (pipeline_args & CORE_ACTIONS).any?
     end
+    if previous_jobs.empty?
+      run = get_ingest_run
+      perftime = (TimeDifference.between(
+        event_timestamp(run.create_time), event_timestamp(run.update_time)
+      ).in_seconds * 1000).to_i
+      status = run.status.state == "FAILED" ? 'failed' : 'success'
+      return {
+        perfTime: perftime,
+        fileName: study_file.name,
+        fileType: study_file.file_type,
+        fileSize: study_file.upload_file_size,
+        studyAccession: study.accession,
+        trigger: study_file.upload_trigger,
+        jobStatus: status,
+        numFilesExtracted: 0,
+        machineType: params_object.machine_type,
+        action: 'unknown',
+        exitCode: exit_code
+      }
+    end
     # get total runtime from initial extract to final parse
-    initial_extract = previous_jobs.last
-    final_parse = previous_jobs.first
+    initial_extract = previous_jobs.min_by(&:create_time)
+    final_parse = previous_jobs.max_by(&:create_time)
     start_time = event_timestamp(initial_extract.create_time)
     end_time = event_timestamp(final_parse.update_time) # update_time is a proxy for last event timestamp
     job_perftime = (TimeDifference.between(start_time, end_time).in_seconds * 1000).to_i
@@ -1079,7 +1101,7 @@ def anndata_summary_props
     if job_status == 'failed'
       first_failure = previous_jobs.reverse.detect { |job| client.job_error(job.name).present? }
       args = client.get_job_command_line(job: first_failure)
-      error_action = args.detect {|c| BatchApiClient::FILE_TYPES_BY_ACTION.keys.include?(c.to_sym) }
+      error_action = args.detect { |c| BatchApiClient::FILE_TYPES_BY_ACTION.keys.include?(c.to_sym) }
       code = client.exit_code_from_task(first_failure.name)
     end
     # count total number of files extracted

app/models/study.rb

@@ -752,10 +752,11 @@ def author
   before_validation :set_data_dir, :set_firecloud_workspace_name, on: :create
   after_validation  :assign_accession, on: :create
   # before_save       :verify_default_options
-  after_create      :make_data_dir, :set_default_participant, :check_bucket_read_access
+  after_create      :make_data_dir, :set_default_participant, :check_bucket_read_access, :log_study_creation
   before_destroy    :ensure_cascade_on_associations
   after_destroy     :remove_data_dir
   before_save       :set_readonly_access
+  after_update      :log_study_state
 
   # search definitions
   index({"name" => "text", "description" => "text"}, {background: true})
@@ -1836,12 +1837,61 @@ def delete_convention_data
     end
   end
 
+  ## State tracking methods
+
   def last_public_date
-    history_tracks.where('modified.public': true).order_by(created_at: :desc).first&.created_at
+    history_tracks.where('modified.public': true).order_by(created_at: :desc).last&.created_at
   end
 
   def last_initialized_date
-    history_tracks.where('modified.initialized': true).order_by(created_at: :desc).first&.created_at
+    history_tracks.where('modified.initialized': true).order_by(created_at: :desc).last&.created_at
+  end
+
+  # find the last time an attribute changed in the history
+  def last_change_for(field)
+    history_tracks.select { |h| h.modified.keys.include?(field.to_s) }.last
+  end
+
+  # determine if a field changed from a given value
+  def field_changed_from?(history_track, field, value)
+    return false if history_track.nil?
+
+    history_track.original[field] == value && history_track.modified[field] != value
+  end
+
+  # helper to determine if a study was just made public
+  def was_just_published?(cutoff: nil)
+    track = last_change_for(:public)
+    field_changed_from?(track, :public, false) && track.created_at >= (cutoff || 1.second.ago)
+  end
+
+  # helper to determine if a study was just initialized
+  def was_just_initialized?(cutoff: nil)
+    track = last_change_for(:initialized)
+    field_changed_from?(track, :initialized, false) && track.created_at >= (cutoff || 1.second.ago)
+  end
+
+  # basic Mixpanel props for study state tracking
+  def mixpanel_state_props
+    {
+      studyAccession: accession,
+      created_at:,
+      domain: user.email.split('@').last,
+      numCells: cell_count,
+      public:,
+      last_public_date:,
+      initialized:,
+      last_initialized_date:
+    }
+  end
+
+  def log_study_creation
+    MetricsService.log('study-creation', mixpanel_state_props, user)
+  end
+
+  # only log state when a study is published/initialized
+  def log_study_state
+    MetricsService.log('study-state', mixpanel_state_props, user) if was_just_published? || was_just_initialized?
   end
 
   private

bin/run_orch_smoke_test.sh

@@ -18,6 +18,8 @@ DOCKER_IMAGE_NAME="gcr.io/broad-singlecellportal-staging/single-cell-portal"
 DOCKER_IMAGE_VERSION="development"
 PASSENGER_APP_ENV="test"
 NON_DOCKERIZED="false"
+# disable warnings about frozen literals
+export RUBYOPT=--disable-frozen-string-literal
 
 while getopts "k:e:v:d" OPTION; do
 case $OPTION in

db/migrate/20250528160832_set_relevant_default_annotations.rb

@@ -0,0 +1,17 @@
+class SetRelevantDefaultAnnotations < Mongoid::Migration
+  def self.up
+    accessions = Study.where(queued_for_deletion: false).pluck(:accession)
+    accessions.each do |accession|
+      study = Study.find_by(accession:)
+      begin
+        ClusterCacheService.configure_default_annotation(study)
+      rescue => e
+        ErrorTracker.report_exception(e, nil,{ study: })
+      end
+    end
+  end
+
+  def self.down
+    # default annotations can be reset manually if needed
+  end
+end

test/integration/external/study_validation_test.rb

@@ -130,8 +130,6 @@ class StudyValidationTest < ActionDispatch::IntegrationTest
     end
     puts "After #{seconds_slept} seconds, " + (example_files.values.map { |e| "#{e[:name]} is #{e[:object].parse_status}"}).join(", ") + '.'
 
-    study.reload
-
     example_files.values.each do |e|
       e[:object].reload # address potential race condition between parse_status setting to 'failed' and DeleteQueueJob executing
       assert_equal 'failed', e[:object].parse_status, "Incorrect parse_status for #{e[:name]}"
@@ -140,6 +138,8 @@ class StudyValidationTest < ActionDispatch::IntegrationTest
       assert cached_file.present?, "Did not find cached file at #{e[:cache_location]} in #{study.bucket_id}"
     end
 
+    sleep(5)
+    study.reload
     assert_equal 0, study.cell_metadata.size
     assert_equal 0, study.genes.size
     assert_equal 0, study.cluster_groups.size

test/lib/summary_stats_utils_test.rb

@@ -157,18 +157,21 @@ class SummaryStatsUtilsTest < ActiveSupport::TestCase
   end
 
   test 'should get data retention policy report' do
+    expected_studies = Study.where(detached: false, firecloud_project: FireCloudClient::PORTAL_NAMESPACE)
     client_mock = Minitest::Mock.new
     bucket_mock = Minitest::Mock.new
-    created_at = DateTime.now.in_time_zone - 1.day
-    bucket_mock.expect :nil?, false
     file_mock = Minitest::Mock.new
-    2.times do
-      bucket_mock.expect :next?, false
-      file_mock.expect :next?, false
+    created_at = DateTime.now.in_time_zone - 1.day
+    expected_studies.each do |study|
+      bucket_mock.expect :nil?, false
+      2.times do
+        bucket_mock.expect :next?, false
+        file_mock.expect :next?, false
+      end
+      file_mock.expect :any?, true
+      bucket_mock.expect :files, file_mock
+      client_mock.expect :get_workspace_bucket, bucket_mock, [study.bucket_id]
     end
-    file_mock.expect :any?, true
-    bucket_mock.expect :files, file_mock
-    client_mock.expect :get_workspace_bucket, bucket_mock, [String]
     FireCloudClient.stub :new, client_mock do
       SummaryStatsUtils.stub :bytes_and_last_created_for,
                              [250.gigabytes, created_at],