From 3bda8fab4d9193c53ac3988c506ea205c4ada180 Mon Sep 17 00:00:00 2001 From: Ben Topping Date: Thu, 11 Jun 2026 15:36:46 +0100 Subject: [PATCH 1/5] feat(nanoseq): add metadatable_type to metadata lookup to improve query speed --- .../pcr_cycles_binned_plate_for_t_nano_seq.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb b/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb index fce323a8ce..fdd4e2ee7a 100644 --- a/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb +++ b/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb @@ -193,7 +193,9 @@ def well_filter # @param request_id [Integer] the ID of the request associated with the metadata # @return [Sequencescape::Api::V2::PolyMetadatum, nil] the found metadata or nil if no matching metadata is found def find_existing_metadata(metadata_key, request_id) - Sequencescape::Api::V2::PolyMetadatum.find(key: metadata_key, metadatable_id: request_id).first + # Note metadatable_type is 'Request' as we are looking for metadata on the request specifically + # This also allows us to make use of the index on the table during the lookup for better performance + Sequencescape::Api::V2::PolyMetadatum.find(key: metadata_key, metadatable_id: request_id, metadatable_type: 'Request').first end # Updates the value of an existing metadata if it's different from the provided value. From b5be5f3a6f940b3b4912e09254216c9a8b6cb5a8 Mon Sep 17 00:00:00 2001 From: Ben Topping Date: Thu, 11 Jun 2026 15:38:16 +0100 Subject: [PATCH 2/5] style: linted --- .../labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb b/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb index fdd4e2ee7a..228f8d53a9 100644 --- a/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb +++ b/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb @@ -195,7 +195,8 @@ def well_filter def find_existing_metadata(metadata_key, request_id) # Note metadatable_type is 'Request' as we are looking for metadata on the request specifically # This also allows us to make use of the index on the table during the lookup for better performance - Sequencescape::Api::V2::PolyMetadatum.find(key: metadata_key, metadatable_id: request_id, metadatable_type: 'Request').first + Sequencescape::Api::V2::PolyMetadatum.find(key: metadata_key, metadatable_id: request_id, + metadatable_type: 'Request').first end # Updates the value of an existing metadata if it's different from the provided value. From f1d667245397c907adbfd10b806023bfd623e8f5 Mon Sep 17 00:00:00 2001 From: Ben Topping Date: Fri, 12 Jun 2026 10:28:19 +0100 Subject: [PATCH 3/5] tests: correctly stub polymetadata metadatable_type in ss v2 requests --- ..._cycles_binned_plate_for_t_nano_seq_spec.rb | 18 +++++++++--------- spec/support/api_url_helper.rb | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb b/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb index 80cfc5a062..4e7c5ee8e0 100644 --- a/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb +++ b/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb @@ -1461,15 +1461,15 @@ before do allow(pm_pcr_cycles).to receive(:update).and_return(true) - stub_polymetadata(pm_original_plate_barcode, loop_2_request.id) - stub_polymetadata(pm_original_well_id, loop_2_request.id) - stub_polymetadata(pm_concentration_nm, loop_2_request.id) - stub_polymetadata(pm_input_amount_available, loop_2_request.id) - stub_polymetadata(pm_input_amount_desired, loop_2_request.id) - stub_polymetadata(pm_sample_volume, loop_2_request.id) - stub_polymetadata(pm_diluent_volume, loop_2_request.id) - stub_polymetadata(pm_pcr_cycles, loop_2_request.id) - stub_polymetadata(pm_hyb_panel, loop_2_request.id) + stub_polymetadata(pm_original_plate_barcode, loop_2_request.id, 'Request') + stub_polymetadata(pm_original_well_id, loop_2_request.id, 'Request') + stub_polymetadata(pm_concentration_nm, loop_2_request.id, 'Request') + stub_polymetadata(pm_input_amount_available, loop_2_request.id, 'Request') + stub_polymetadata(pm_input_amount_desired, loop_2_request.id, 'Request') + stub_polymetadata(pm_sample_volume, loop_2_request.id, 'Request') + stub_polymetadata(pm_diluent_volume, loop_2_request.id, 'Request') + stub_polymetadata(pm_pcr_cycles, loop_2_request.id, 'Request') + stub_polymetadata(pm_hyb_panel, loop_2_request.id, 'Request') end it 'makes the expected method calls when creating the child plate' do diff --git a/spec/support/api_url_helper.rb b/spec/support/api_url_helper.rb index 07cbb1047a..392acd6242 100644 --- a/spec/support/api_url_helper.rb +++ b/spec/support/api_url_helper.rb @@ -252,8 +252,8 @@ def stub_plate(plate, stub_search: true, custom_query: nil, custom_includes: nil stub_labware(plate) end - def stub_polymetadata(polymetadata, metadatable_id) - arguments = [{ key: polymetadata.key, metadatable_id: metadatable_id }] + def stub_polymetadata(polymetadata, metadatable_id, metadatable_type) + arguments = [{ key: polymetadata.key, metadatable_id: metadatable_id, metadatable_type: metadatable_type }] allow(Sequencescape::Api::V2::PolyMetadatum).to receive(:find).with(*arguments).and_return([polymetadata]) end From 4e5dcdea1b30567810c96da0b432547b83036a42 Mon Sep 17 00:00:00 2001 From: Ben Topping Date: Fri, 12 Jun 2026 11:28:45 +0100 Subject: [PATCH 4/5] perf: remove N+1 query for poly metadata keys --- .../pcr_cycles_binned_plate_for_t_nano_seq.rb | 31 +++++++++---------- ...cycles_binned_plate_for_t_nano_seq_spec.rb | 17 +++++----- spec/support/api_url_helper.rb | 4 +-- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb b/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb index 228f8d53a9..92878bce74 100644 --- a/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb +++ b/app/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq.rb @@ -69,11 +69,14 @@ def after_transfer! # are metadata values # @param child_well_location [String] the location of the child well associated with the request def create_or_update_request_metadata(request, request_metadata, child_well_location) + # Fetch all existing metadata for this request + existing_metadata_by_key = fetch_existing_metadata(request.id) + request_metadata.each do |metadata_key, metadata_value| - existing_metadata_v2 = find_existing_metadata(metadata_key, request.id) + existing_metadata = existing_metadata_by_key[metadata_key] - if existing_metadata_v2.present? - update_existing_metadata(existing_metadata_v2, metadata_value, metadata_key, child_well_location) + if existing_metadata.present? + update_existing_metadata(existing_metadata, metadata_value, metadata_key, child_well_location) else create_new_metadata(metadata_key, metadata_value, request, child_well_location) end @@ -186,31 +189,27 @@ def well_filter @well_filter ||= WellFilterAllowingPartials.new(creator: self, request_state: 'pending') end - # Finds and returns the first existing metadata that matches the provided key and request ID. - # If no matching metadata is found, it returns nil. + # Fetches all existing metadata for a request returns a hash keyed by metadata key. # - # @param metadata_key [String] the key of the metadata to find # @param request_id [Integer] the ID of the request associated with the metadata - # @return [Sequencescape::Api::V2::PolyMetadatum, nil] the found metadata or nil if no matching metadata is found - def find_existing_metadata(metadata_key, request_id) - # Note metadatable_type is 'Request' as we are looking for metadata on the request specifically - # This also allows us to make use of the index on the table during the lookup for better performance - Sequencescape::Api::V2::PolyMetadatum.find(key: metadata_key, metadatable_id: request_id, - metadatable_type: 'Request').first + # @return [Hash] hash of metadata keyed by metadata key + def fetch_existing_metadata(request_id) + Sequencescape::Api::V2::PolyMetadatum.find(metadatable_id: request_id, + metadatable_type: 'Request').index_by(&:key) end # Updates the value of an existing metadata if it's different from the provided value. # If the metadata fails to update, it raises a StandardError with a descriptive message. # - # @param existing_metadata_v2 [Sequencescape::Api::V2::Metadatum] the existing metadata to update + # @param existing_metadata [Sequencescape::Api::V2::Metadatum] the existing metadata to update # @param metadata_value [String] the new value for the metadata # @param metadata_key [String] the key of the metadata # @param child_well_location [String] the location of the child well associated with the request # @raise [StandardError] if the existing metadata fails to update - def update_existing_metadata(existing_metadata_v2, metadata_value, metadata_key, child_well_location) - return if existing_metadata_v2.value == metadata_value + def update_existing_metadata(existing_metadata, metadata_value, metadata_key, child_well_location) + return if existing_metadata.value == metadata_value - return if existing_metadata_v2.update(value: metadata_value) + return if existing_metadata.update(value: metadata_value) raise StandardError, "Existing metadata for request (key: #{metadata_key}, value: #{metadata_value}) " \ diff --git a/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb b/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb index 4e7c5ee8e0..14571cc639 100644 --- a/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb +++ b/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb @@ -1461,15 +1461,12 @@ before do allow(pm_pcr_cycles).to receive(:update).and_return(true) - stub_polymetadata(pm_original_plate_barcode, loop_2_request.id, 'Request') - stub_polymetadata(pm_original_well_id, loop_2_request.id, 'Request') - stub_polymetadata(pm_concentration_nm, loop_2_request.id, 'Request') - stub_polymetadata(pm_input_amount_available, loop_2_request.id, 'Request') - stub_polymetadata(pm_input_amount_desired, loop_2_request.id, 'Request') - stub_polymetadata(pm_sample_volume, loop_2_request.id, 'Request') - stub_polymetadata(pm_diluent_volume, loop_2_request.id, 'Request') - stub_polymetadata(pm_pcr_cycles, loop_2_request.id, 'Request') - stub_polymetadata(pm_hyb_panel, loop_2_request.id, 'Request') + stub_polymetadata( + [pm_original_plate_barcode, pm_original_well_id, pm_concentration_nm, pm_input_amount_available, + pm_input_amount_desired, pm_sample_volume, pm_diluent_volume, pm_pcr_cycles, pm_hyb_panel], + loop_2_request.id, + 'Request' + ) end it 'makes the expected method calls when creating the child plate' do @@ -1548,6 +1545,7 @@ end it 'updates existing metadata when it exists' do + request_metadata['key1'] = 'new_value1' allow(Sequencescape::Api::V2::PolyMetadatum).to receive(:find).and_return([existing_metadata_1]) expect(existing_metadata_1).to receive(:update).once.and_return(true) subject.create_or_update_request_metadata(request, request_metadata, child_well_location) @@ -1561,6 +1559,7 @@ end it 'raises an error when existing metadata fails to update' do + request_metadata['key1'] = 'new_value1' allow(Sequencescape::Api::V2::PolyMetadatum).to receive(:find).and_return([existing_metadata_1]) allow(existing_metadata_1).to receive(:update).and_return(false) expect do diff --git a/spec/support/api_url_helper.rb b/spec/support/api_url_helper.rb index 392acd6242..52631591ed 100644 --- a/spec/support/api_url_helper.rb +++ b/spec/support/api_url_helper.rb @@ -253,8 +253,8 @@ def stub_plate(plate, stub_search: true, custom_query: nil, custom_includes: nil end def stub_polymetadata(polymetadata, metadatable_id, metadatable_type) - arguments = [{ key: polymetadata.key, metadatable_id: metadatable_id, metadatable_type: metadatable_type }] - allow(Sequencescape::Api::V2::PolyMetadatum).to receive(:find).with(*arguments).and_return([polymetadata]) + arguments = [{ metadatable_id:, metadatable_type: }] + allow(Sequencescape::Api::V2::PolyMetadatum).to receive(:find).with(*arguments).and_return(polymetadata) end def stub_project(project) From 95a6b099fa409685ea50851d87bf9ab1bca47f51 Mon Sep 17 00:00:00 2001 From: Ben Topping Date: Fri, 12 Jun 2026 11:31:53 +0100 Subject: [PATCH 5/5] tests: adds test covering existing metadata matching new metadata --- .../pcr_cycles_binned_plate_for_t_nano_seq_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb b/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb index 14571cc639..8e338d2322 100644 --- a/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb +++ b/spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb @@ -1551,6 +1551,14 @@ subject.create_or_update_request_metadata(request, request_metadata, child_well_location) end + it 'does not update existing metadata when the value is the same' do + allow(Sequencescape::Api::V2::PolyMetadatum).to receive(:find).and_return([existing_metadata_1, + existing_metadata_2]) + expect(existing_metadata_1).not_to receive(:update) + expect(existing_metadata_2).not_to receive(:update) + subject.create_or_update_request_metadata(request, request_metadata, child_well_location) + end + it 'raises an error when new metadata fails to save' do allow(new_metadata_1).to receive(:save).and_return(false) expect do