Merge branch 'fix/discovery' into 'develop'

Discovery no longer injects default config classes into target version

See merge request genaiic-reusable-assets/engagement-artifacts/genaiic-idp-accelerator!570

Author: rstrahan

CHANGELOG.md

@@ -15,6 +15,8 @@ SPDX-License-Identifier: MIT-0
 
 - **CLI: Remove deprecated `--pattern` references** — Updated `idp-cli.md` and CLI code to reflect the unified pattern architecture. Removed `--pattern` from all deploy and config command examples/options.
 
+- **Discovery no longer injects default config classes into target version** — Previously, running Discovery on a configuration version would merge all classes from the `default` version into the target version alongside the newly discovered class. Now Discovery only adds/updates the discovered class within the target version's own class list, keeping the version's classes exactly as the user curated them.
+
 - **Documentation: Comprehensive review and cleanup** — Fixed outdated references, broken links, and missing content across documentation files.
 
 ## [0.5.1]

lib/idp_common_pkg/idp_common/discovery/classes_discovery.py

@@ -98,8 +98,7 @@ def discovery_classes_with_document(self, input_bucket: str, input_prefix: str):
             # No need to transform - it's already in the right format
             current_class = model_response
 
-            # Merge the new class with existing Default + Custom classes
-            # and save to Custom config
+            # Add the discovered class to the target version's config
             self._merge_and_save_class(current_class)
 
             return {"status": "SUCCESS"}
@@ -151,8 +150,7 @@ def discovery_classes_with_document_and_ground_truth(
             # No need to transform - it's already in the right format
             current_class = model_response
 
-            # Merge the new class with existing Default + Custom classes
-            # and save to Custom config
+            # Add the discovered class to the target version's config
             self._merge_and_save_class(current_class)
 
             return {"status": "SUCCESS"}
@@ -166,18 +164,17 @@ def discovery_classes_with_document_and_ground_truth(
 
     def _merge_and_save_class(self, new_class: Dict[str, Any]) -> None:
         """
-        Merge a new discovered class with existing Default + Custom classes and save to Custom.
+        Merge a new discovered class into the target version's configuration.
 
-        This method ensures that discovered classes are ADDITIVE to existing classes:
-        1. Read Default classes (base classes from deployment)
-        2. Read existing Custom classes (previous user customizations)
-        3. Build a merged list starting from Default, overriding with Custom
-        4. Add/update the new discovered class
-        5. Save the complete merged list to Custom
+        This method only adds/updates the discovered class within the target version's
+        own class list. It does NOT pull in classes from the default configuration version,
+        keeping the target version's classes exactly as the user curated them, plus the
+        newly discovered class.
 
-        This is necessary because the Default+Custom merge uses array replacement,
-        not array concatenation. By saving the complete class list to Custom,
-        we ensure no classes are lost during the merge.
+        Steps:
+        1. Read existing classes from the target version
+        2. Add/update the new discovered class (deduplicate by $id)
+        3. Save back to the target version
 
         Args:
             new_class: The newly discovered class schema to add/update
@@ -186,65 +183,36 @@ def _merge_and_save_class(self, new_class: Dict[str, Any]) -> None:
         new_class_id = new_class.get("$id") or new_class.get("x-aws-idp-document-type")
         logger.info(f"Merging discovered class: {new_class_id}")
 
-        # Step 1: Read Default classes (base classes from deployment)
-        default_config = self.config_manager.get_configuration("Config", "default")
-        default_classes: list = []
-        if (
-            default_config
-            and isinstance(default_config, IDPConfig)
-            and default_config.classes
-        ):
-            # Convert to list of dicts for easier manipulation
-            default_classes = [
-                cls
-                if isinstance(cls, dict)
-                else cls.model_dump()
-                if hasattr(cls, "model_dump")
-                else dict(cls)
-                for cls in default_config.classes
-            ]
-        logger.info(f"Found {len(default_classes)} classes in Default config")
-
-        # Step 2: Read existing Custom config (raw, no Pydantic defaults)
+        # Read existing config for the target version (raw, no Pydantic defaults)
         existing_custom = (
             self.config_manager.get_raw_configuration("Config", version=self.version)
             or {}
         )
         custom_classes = list(existing_custom.get("classes", []))
-        logger.info(f"Found {len(custom_classes)} classes in Custom config")
-
-        # Step 3: Build merged class list - start with Default, override with Custom
-        # Use a dict keyed by class ID for efficient deduplication
-        merged_classes_by_id: Dict[str, Dict[str, Any]] = {}
-
-        # Add Default classes first
-        for cls in default_classes:
-            cls_id = cls.get("$id") or cls.get("x-aws-idp-document-type")
-            if cls_id:
-                merged_classes_by_id[cls_id] = cls
+        logger.info(
+            f"Found {len(custom_classes)} existing classes in version '{self.version}'"
+        )
 
-        # Override/add Custom classes
+        # Build class list from existing version classes, deduplicating by ID
+        classes_by_id: Dict[str, Dict[str, Any]] = {}
         for cls in custom_classes:
             cls_id = cls.get("$id") or cls.get("x-aws-idp-document-type")
             if cls_id:
-                merged_classes_by_id[cls_id] = cls
+                classes_by_id[cls_id] = cls
 
-        # Step 4: Add/update the new discovered class
+        # Add/update the new discovered class
         if new_class_id:
-            merged_classes_by_id[new_class_id] = new_class
+            classes_by_id[new_class_id] = new_class
 
         # Convert back to list
-        merged_classes = list(merged_classes_by_id.values())
-        logger.info(f"Merged class list has {len(merged_classes)} classes")
+        merged_classes = list(classes_by_id.values())
+        logger.info(f"Saving {len(merged_classes)} classes to version '{self.version}'")
 
-        # Step 5: Save to Custom config
-        # The merged list will replace Default.classes during runtime merge,
-        # ensuring all classes (Default + Custom + new) are preserved
+        # Save to version config
         existing_custom["classes"] = merged_classes
         self.config_manager.save_raw_configuration(
             "Config", existing_custom, version=self.version
         )
-        logger.info(f"Saved {len(merged_classes)} classes to Custom config")
 
     def _validate_json_schema(self, schema: Dict[str, Any]) -> tuple[bool, str]:
         """

lib/idp_common_pkg/tests/unit/discovery/test_classes_discovery.py

@@ -639,32 +639,7 @@ def test_sample_output_format(self, service):
     def test_discovery_classes_with_document_updates_existing_class(
         self, service, mock_configuration_item
     ):
-        """Test that discovery updates existing class configuration."""
-        # Mock existing Default configuration with classes in JSON Schema format
-        # Use actual IDPConfig so isinstance check passes
-        existing_default_config = IDPConfig(
-            classes=[
-                {
-                    "$schema": "http://json-schema.org/draft-07/schema#",
-                    "$id": "w4",
-                    "type": "object",
-                    "title": "W-4",
-                    "description": "Old description",
-                    "x-aws-idp-document-type": "W-4",
-                    "properties": {},
-                },
-                {
-                    "$schema": "http://json-schema.org/draft-07/schema#",
-                    "$id": "other_form",
-                    "type": "object",
-                    "title": "Other-Form",
-                    "description": "Other form",
-                    "x-aws-idp-document-type": "Other-Form",
-                    "properties": {},
-                },
-            ]
-        )
-
+        """Test that discovery updates existing class in the target version only."""
         with (
             patch("idp_common.utils.s3util.S3Util.get_bytes") as mock_get_bytes,
             patch("idp_common.bedrock.extract_text_from_response") as mock_extract_text,
@@ -685,12 +660,29 @@ def test_discovery_classes_with_document_updates_existing_class(
                 "response": {"output": {"message": {"content": [{"text": "{}"}]}}},
                 "metering": {"tokens": 500},
             }
-            # get_configuration returns Default config (for reading Default classes)
-            service.config_manager.get_configuration.return_value = (
-                existing_default_config
-            )
-            # get_raw_configuration returns existing Custom (no existing Custom classes in this case)
-            service.config_manager.get_raw_configuration.return_value = {}
+            # Target version already has an old W-4 and another class
+            service.config_manager.get_raw_configuration.return_value = {
+                "classes": [
+                    {
+                        "$schema": "http://json-schema.org/draft-07/schema#",
+                        "$id": "w4",
+                        "type": "object",
+                        "title": "W-4",
+                        "description": "Old description",
+                        "x-aws-idp-document-type": "W-4",
+                        "properties": {},
+                    },
+                    {
+                        "$schema": "http://json-schema.org/draft-07/schema#",
+                        "$id": "other_form",
+                        "type": "object",
+                        "title": "Other-Form",
+                        "description": "Other form",
+                        "x-aws-idp-document-type": "Other-Form",
+                        "properties": {},
+                    },
+                ]
+            }
 
             result = service.discovery_classes_with_document(
                 "test-bucket", "test-document.pdf"
@@ -704,7 +696,7 @@ def test_discovery_classes_with_document_updates_existing_class(
             assert call_args[0][0] == "Config"  # Config type
             updated_classes = call_args[0][1]["classes"]  # Classes from saved config
 
-            # Should have 2 classes (Other-Form from Default + updated W-4)
+            # Should have 2 classes (existing Other-Form + updated W-4 from version)
             assert len(updated_classes) == 2
 
             # Find the W-4 class and verify it was updated (by $id)
@@ -714,6 +706,60 @@ def test_discovery_classes_with_document_updates_existing_class(
             assert w4_class is not None
             assert w4_class["description"] == "Updated description"
 
+            # Verify Other-Form is preserved from the version
+            other_class = next(
+                (cls for cls in updated_classes if cls.get("$id") == "other_form"), None
+            )
+            assert other_class is not None
+            assert other_class["description"] == "Other form"
+
+    def test_discovery_does_not_pull_default_classes_into_version(self, service):
+        """Test that discovery does NOT inject default config classes into the target version.
+
+        When a user runs discovery on a version, only the discovered class should be
+        added. Classes from the 'default' config version should NOT be merged in.
+        """
+        with (
+            patch("idp_common.utils.s3util.S3Util.get_bytes") as mock_get_bytes,
+            patch("idp_common.bedrock.extract_text_from_response") as mock_extract_text,
+        ):
+            mock_get_bytes.return_value = b"fake_content"
+            mock_extract_text.return_value = json.dumps(
+                {
+                    "$schema": "http://json-schema.org/draft-07/schema#",
+                    "$id": "w4",
+                    "type": "object",
+                    "title": "W-4",
+                    "description": "Discovered W-4",
+                    "x-aws-idp-document-type": "W-4",
+                    "properties": {},
+                }
+            )
+            service._mock_bedrock_client.return_value = {
+                "response": {"output": {"message": {"content": [{"text": "{}"}]}}},
+                "metering": {"tokens": 500},
+            }
+            # Target version has NO classes yet (empty version)
+            service.config_manager.get_raw_configuration.return_value = {}
+
+            result = service.discovery_classes_with_document(
+                "test-bucket", "test-document.pdf"
+            )
+
+            assert result["status"] == "SUCCESS"
+
+            # Verify only the discovered class is saved — no default classes injected
+            service.config_manager.save_raw_configuration.assert_called_once()
+            call_args = service.config_manager.save_raw_configuration.call_args
+            updated_classes = call_args[0][1]["classes"]
+
+            assert len(updated_classes) == 1
+            assert updated_classes[0]["$id"] == "w4"
+            assert updated_classes[0]["description"] == "Discovered W-4"
+
+            # Verify get_configuration was NOT called (no default config reading)
+            service.config_manager.get_configuration.assert_not_called()
+
     def test_discovery_classes_with_document_no_existing_config(self, service):
         """Test discovery when no existing configuration exists."""
         with (

lib/idp_common_pkg/tests/unit/discovery/test_classes_discovery_integration.py

@@ -17,7 +17,6 @@
 
 # Import application modules
 from idp_common.discovery.classes_discovery import ClassesDiscovery
-from idp_common.config.models import IDPConfig
 
 
 @pytest.mark.unit
@@ -371,7 +370,11 @@ def test_discovery_updates_existing_configuration(
         service_with_mocks,
         mock_w4_bedrock_response,
     ):
-        """Test that discovery properly updates existing configuration."""
+        """Test that discovery properly updates existing version configuration.
+
+        Discovery should only add/update the discovered class within the target
+        version's own classes — it should NOT pull in classes from the default version.
+        """
         # Mock S3 file content
         mock_get_bytes.return_value = b"fake content"
 
@@ -381,35 +384,30 @@ def test_discovery_updates_existing_configuration(
         ]["content"][0]["text"]
         service_with_mocks._mock_bedrock_client.return_value = mock_w4_bedrock_response
 
-        # Mock existing Default configuration with different forms in JSON Schema format
-        existing_default = IDPConfig(
-            classes=[
-                {
-                    "$schema": "http://json-schema.org/draft-07/schema#",
-                    "$id": "i9",
-                    "type": "object",
-                    "title": "I-9",
-                    "description": "Employment Eligibility Verification",
-                    "x-aws-idp-document-type": "I-9",
-                    "properties": {},
-                },
-                {
-                    "$schema": "http://json-schema.org/draft-07/schema#",
-                    "$id": "w4",
-                    "type": "object",
-                    "title": "W-4",
-                    "description": "Old W-4 description",
-                    "x-aws-idp-document-type": "W-4",
-                    "properties": {},
-                },
-            ]
-        )
-        # Create mocks for config_manager methods
-        service_with_mocks.config_manager.get_configuration = MagicMock(
-            return_value=existing_default
-        )
+        # Target version already has an I-9 and an old W-4
         service_with_mocks.config_manager.get_raw_configuration = MagicMock(
-            return_value={}  # No existing Custom
+            return_value={
+                "classes": [
+                    {
+                        "$schema": "http://json-schema.org/draft-07/schema#",
+                        "$id": "i9",
+                        "type": "object",
+                        "title": "I-9",
+                        "description": "Employment Eligibility Verification",
+                        "x-aws-idp-document-type": "I-9",
+                        "properties": {},
+                    },
+                    {
+                        "$schema": "http://json-schema.org/draft-07/schema#",
+                        "$id": "w4",
+                        "type": "object",
+                        "title": "W-4",
+                        "description": "Old W-4 description",
+                        "x-aws-idp-document-type": "W-4",
+                        "properties": {},
+                    },
+                ]
+            }
         )
         service_with_mocks.config_manager.save_raw_configuration = MagicMock()
 
@@ -429,7 +427,7 @@ def test_discovery_updates_existing_configuration(
         assert save_config_args[0] == "Config"  # First arg is config type
         updated_classes = save_config_args[1]["classes"]  # Second arg is config dict
 
-        # Should have 2 classes: I-9 (from Default) + W-4 (updated)
+        # Should have 2 classes: I-9 (existing in version) + W-4 (updated by discovery)
         assert len(updated_classes) == 2
 
         # Find and verify the updated W-4 class (JSON Schema format)
@@ -440,7 +438,7 @@ def test_discovery_updates_existing_configuration(
         )
         assert len(w4_class["properties"]) == 3
 
-        # Verify I-9 class is still present (JSON Schema format)
+        # Verify I-9 class is still present from the version (JSON Schema format)
         i9_class = next(cls for cls in updated_classes if cls["$id"] == "i9")
         assert i9_class["description"] == "Employment Eligibility Verification"