Improve PreviewQuery error detection and update MCP descriptions

Expand PreviewQuery to detect additional alphanumeric identifier
format violations: case-mismatch, underscores, leading digits, and
purely numerical identifiers. Restructure error classification with
FORMAT_RULES for clarity and add corresponding locale strings.

Update MCP tool descriptions for search_projects, search_programs,
and search_portfolios to reflect case-insensitive identifier
matching, with updated test expectations.

Author: akabiru

app/services/mcp_tools/search_portfolios.rb

@@ -48,7 +48,7 @@ class SearchPortfolios < Base
       type: :object,
       properties: {
         name: { type: "string", description: "Name of the portfolio. Accepts partial names, not case-sensitive." },
-        identifier: { type: "string", description: "Portfolio identifier. Case-sensitive, matching exactly." },
+        identifier: { type: "string", description: "Portfolio identifier. Case-insensitive, matching exactly." },
         status_code: { type: "string", enum: Project.status_codes.keys, description: "The portfolio status." }
       }
     )

app/services/mcp_tools/search_programs.rb

@@ -48,7 +48,7 @@ class SearchPrograms < Base
       type: :object,
       properties: {
         name: { type: "string", description: "Name of the program. Accepts partial names, not case-sensitive." },
-        identifier: { type: "string", description: "Program identifier. Case-sensitive, matching exactly." },
+        identifier: { type: "string", description: "Program identifier. Case-insensitive, matching exactly." },
         status_code: { type: "string", enum: Project.status_codes.keys, description: "The program status." }
       }
     )

app/services/mcp_tools/search_projects.rb

@@ -48,7 +48,7 @@ class SearchProjects < Base
       type: :object,
       properties: {
         name: { type: "string", description: "Name of the project. Accepts partial project names, not case-sensitive." },
-        identifier: { type: "string", description: "Project identifier. Case-sensitive, matching exactly." },
+        identifier: { type: "string", description: "Project identifier. Case-insensitive, matching exactly." },
         status_code: { type: "string", enum: Project.status_codes.keys, description: "The project status." }
       }
     )

app/services/work_packages/identifier_autofix/preview_query.rb

@@ -34,42 +34,71 @@ class PreviewQuery
       Result = Data.define(:projects_data, :total_count)
       DISPLAY_COUNT = 5
 
+      # Priority-ordered format rules for identifier classification.
+      FORMAT_RULES = [
+        [:too_long, ->(id, max) { id.length > max }],
+        [:numerical, ->(id, _) { id.match?(/\A\d+\z/) }],
+        [:starts_with_number, ->(id, _) { id.match?(/\A\d/) }],
+        [:special_characters, ->(id, _) { id.match?(/[^a-zA-Z0-9]/) }],
+        [:not_uppercase, ->(id, _) { id != id.upcase }]
+      ].freeze
+
       def call
-        total   = problematic_scope.count
-        preview = problematic_scope
-                    .select(:id, :name, :identifier)
-                    .limit(DISPLAY_COUNT)
-                    .to_a
-
-        suggestions = WorkPackages::IdentifierAutofix::ProjectIdentifierSuggestionGenerator.call(
-          preview,
-          in_use_identifiers:,
-          reserved_identifiers:
-        )
+        Result.new(projects_data: build_projects_data, total_count: problematic_scope.count)
+      end
 
-        projects_data = suggestions.map do |entry|
+      private
+
+      def build_projects_data
+        generate_suggestions.map do |entry|
           entry.merge(error_reason: error_reason(entry[:current_identifier]))
         end
+      end
 
-        Result.new(projects_data:, total_count: total)
+      def generate_suggestions
+        ProjectIdentifierSuggestionGenerator.call(
+          preview_projects,
+          in_use_identifiers:,
+          reserved_identifiers:
+        )
       end
 
-      private
+      def preview_projects
+        problematic_scope
+          .select(:id, :name, :identifier)
+          .limit(DISPLAY_COUNT)
+          .to_a
+      end
 
+      # Scope conditions must cover all identifiers classifiable by #error_reason.
       def problematic_scope
-        @problematic_scope ||= Project.where(
-          "length(identifier) > ? OR identifier ~ ?",
-          ProjectIdentifierSuggestionGenerator::IDENTIFIER_LENGTH[:max],
-          "[^a-zA-Z0-9_]"
-        )
+        @problematic_scope ||= exceeds_max_length
+                                 .or(contains_non_alphanumeric)
+                                 .or(starts_with_digit)
+                                 .or(not_fully_uppercased)
       end
 
+      def exceeds_max_length        = Project.where("length(identifier) > ?", max_identifier_length)
+      def contains_non_alphanumeric = Project.where("identifier ~ ?", "[^a-zA-Z0-9]")
+      def starts_with_digit         = Project.where("identifier ~ ?", "^[0-9]")
+      def not_fully_uppercased      = Project.where("identifier != UPPER(identifier)")
+
+      def max_identifier_length = ProjectIdentifierSuggestionGenerator::IDENTIFIER_LENGTH[:max]
+
+      # Must handle all identifiers matched by #problematic_scope.
       def error_reason(identifier)
-        if identifier.length > ProjectIdentifierSuggestionGenerator::IDENTIFIER_LENGTH[:max]
-          :too_long
-        elsif identifier.match?(/[^a-zA-Z0-9_]/)
-          :special_characters
-        elsif in_use_identifiers.include?(identifier)
+        format_error_reason(identifier) || collision_error_reason(identifier) || :unknown
+      end
+
+      def format_error_reason(identifier)
+        FORMAT_RULES.each do |reason, check|
+          return reason if check.call(identifier, max_identifier_length)
+        end
+        nil
+      end
+
+      def collision_error_reason(identifier)
+        if in_use_identifiers.include?(identifier)
           :in_use
         elsif reserved_identifiers.include?(identifier)
           :reserved

config/locales/en.yml

@@ -411,9 +411,13 @@ en:
           label_example_work_package_id: Example work package ID
         autofix_preview:
           error_too_long: Has to be fewer than 5 characters
+          error_numerical: Cannot be purely numerical
+          error_starts_with_number: Cannot start with a number
           error_special_characters: Special characters not allowed
+          error_not_uppercase: Must be uppercase
           error_in_use: Already in use as another project's active handle
           error_reserved: Reserved by another project's handle history
+          error_unknown: Needs manual review
           remaining_projects:
             one: "... 1 more project"
             other: "... %{count} more projects"

spec/requests/mcp/mcp_tools/search_portfolios_spec.rb

@@ -90,12 +90,12 @@
       end
     end
 
-    context "when passing a non-exact identifier" do
+    context "when passing a case-variant identifier" do
       let(:call_args) { { identifier: "Abc" } }
 
-      it "does not find the portfolio" do
+      it "finds the portfolio (case-insensitive)" do
         subject
-        expect(parsed_results.dig("structuredContent", "items")).to be_empty
+        expect(parsed_results.dig("structuredContent", "items")).to be_present
       end
     end
 

spec/requests/mcp/mcp_tools/search_programs_spec.rb

@@ -90,12 +90,12 @@
       end
     end
 
-    context "when passing a non-exact identifier" do
+    context "when passing a case-variant identifier" do
       let(:call_args) { { identifier: "Abc" } }
 
-      it "does not find the program" do
+      it "finds the program (case-insensitive)" do
         subject
-        expect(parsed_results.dig("structuredContent", "items")).to be_empty
+        expect(parsed_results.dig("structuredContent", "items")).to be_present
       end
     end
 

spec/requests/mcp/mcp_tools/search_projects_spec.rb

@@ -89,12 +89,12 @@
       end
     end
 
-    context "when passing a non-exact identifier" do
+    context "when passing a case-variant identifier" do
       let(:call_args) { { identifier: "Abc" } }
 
-      it "does not find the project" do
+      it "finds the project (case-insensitive)" do
         subject
-        expect(parsed_results.dig("structuredContent", "items")).to be_empty
+        expect(parsed_results.dig("structuredContent", "items")).to be_present
       end
     end
 

spec/services/work_packages/identifier_autofix/preview_query_spec.rb

@@ -35,16 +35,22 @@
 
   let(:display_count) { described_class::DISPLAY_COUNT }
 
+  # Store identifiers bypassing normalizes (which would downcase/upcase them)
+  def set_raw_identifier(project, identifier)
+    Project.where(id: project.id).update_all(Arel.sql("identifier = #{Project.connection.quote(identifier)}"))
+    project
+  end
+
   def create_problematic_project(name:, identifier:)
-    create(:project, name:, identifier:)
+    set_raw_identifier(create(:project, name:), identifier)
   end
 
   def create_valid_project(name:, identifier:)
-    create(:project, name:, identifier:)
+    set_raw_identifier(create(:project, name:), identifier)
   end
 
   context "when there are no problematic projects" do
-    before { create_valid_project(name: "Clean Project", identifier: "clean") }
+    before { create_valid_project(name: "Clean Project", identifier: "CLEAN") }
 
     it "returns total_count 0 and empty projects_data" do
       expect(result.total_count).to eq(0)
@@ -55,9 +61,9 @@ def create_valid_project(name:, identifier:)
   context "when a project has underscores in its identifier" do
     before { create_valid_project(name: "My Project", identifier: "my_proj") }
 
-    it "does not flag it as problematic" do
-      expect(result.total_count).to eq(0)
-      expect(result.projects_data).to be_empty
+    it "flags it as problematic (underscores are not valid in semantic identifiers)" do
+      expect(result.total_count).to eq(1)
+      expect(result.projects_data.first[:error_reason]).to eq(:special_characters)
     end
   end
 
@@ -126,5 +132,33 @@ def create_valid_project(name:, identifier:)
       create_problematic_project(name: "Test", identifier: "my-very-long-identifier")
       expect(result.projects_data.first[:error_reason]).to eq(:too_long)
     end
+
+    it "assigns :numerical when identifier is purely numeric" do
+      create_problematic_project(name: "Test", identifier: "12345")
+      expect(result.projects_data.first[:error_reason]).to eq(:numerical)
+    end
+
+    it "assigns :starts_with_number when identifier begins with a digit" do
+      create_problematic_project(name: "Test", identifier: "1abc")
+      expect(result.projects_data.first[:error_reason]).to eq(:starts_with_number)
+    end
+
+    it "assigns :not_uppercase when identifier is lowercase but otherwise valid" do
+      create_problematic_project(name: "Test", identifier: "proj")
+      expect(result.projects_data.first[:error_reason]).to eq(:not_uppercase)
+    end
+
+    it "assigns :unknown when an identifier in the scope matches no known classification" do
+      project = create_valid_project(name: "Oddball", identifier: "ODDBALL")
+
+      # Simulate a new problematic_scope condition that catches a project
+      # not covered by any error_reason branch (drift scenario).
+      query = described_class.new
+      forced_scope = Project.where(id: project.id)
+      allow(query).to receive(:problematic_scope).and_return(forced_scope)
+
+      result = query.call
+      expect(result.projects_data.first[:error_reason]).to eq(:unknown)
+    end
   end
 end