Address review: deduplicate scraper/entity IDs and add type hints

- Add _deduplicate_id() for scraper names that collide (appends _2, _3, etc.)
- Add _deduplicate_entity_key() for entity names that collide within a scraper
- Use enumerate index for unnamed scrapers without a resource URL
- Add proper TYPE_CHECKING type hints for ScraperInstance fields
- Add contains() method to ScraperRegistry
- Add tests for duplicate scraper names and duplicate entity names

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: danieldotnl

custom_components/multiscrape/__init__.py

@@ -79,20 +79,22 @@ async def _async_process_config(hass: HomeAssistant, config) -> bool:
     load_tasks = []
     registry: ScraperRegistry = hass.data[DOMAIN]
 
-    for conf in config[DOMAIN]:
+    for scraper_idx, conf in enumerate(config[DOMAIN]):
         config_name = conf.get(CONF_NAME)
         if config_name is None:
             resource = conf.get(CONF_RESOURCE) or ""
-            config_name = f"scraper_{slugify(resource)}" if resource else "scraper_unnamed"
+            config_name = (
+                f"scraper_{slugify(resource)}" if resource else f"scraper_unnamed_{scraper_idx}"
+            )
             _LOGGER.debug(
-                "# Found no name for scraper, generated a unique name: %s", config_name
+                "# Found no name for scraper, generated name: %s", config_name
             )
 
         _LOGGER.debug(
             "%s # Setting up multiscrape with config:\n %s", config_name, conf
         )
 
-        scraper_id = config_name
+        scraper_id = _deduplicate_id(registry, config_name)
 
         file_manager = await create_file_manager(hass, config_name, conf.get(CONF_LOG_RESPONSE))
         session = create_http_session(config_name, conf, hass, file_manager)
@@ -130,7 +132,9 @@ async def _shutdown_session(_event, _session=session):
                 entity_name = platform_conf.get(CONF_NAME, "")
                 entity_key = slugify(entity_name) if entity_name else f"entity_{id(platform_conf)}"
 
-                instance.platform_configs.setdefault(platform_domain, {})[entity_key] = platform_conf
+                platform_dict = instance.platform_configs.setdefault(platform_domain, {})
+                entity_key = _deduplicate_entity_key(platform_dict, entity_key)
+                platform_dict[entity_key] = platform_conf
 
                 load = discovery.async_load_platform(
                     hass,
@@ -151,6 +155,34 @@ async def _shutdown_session(_event, _session=session):
     return True
 
 
+def _deduplicate_id(registry: ScraperRegistry, base_id: str) -> str:
+    """Return a unique scraper ID, appending a suffix if needed."""
+    if not registry.contains(base_id):
+        return base_id
+    suffix = 2
+    while registry.contains(f"{base_id}_{suffix}"):
+        suffix += 1
+    deduped = f"{base_id}_{suffix}"
+    _LOGGER.warning(
+        "Duplicate scraper name '%s', using '%s' instead", base_id, deduped
+    )
+    return deduped
+
+
+def _deduplicate_entity_key(platform_dict: dict, base_key: str) -> str:
+    """Return a unique entity key within a platform, appending a suffix if needed."""
+    if base_key not in platform_dict:
+        return base_key
+    suffix = 2
+    while f"{base_key}_{suffix}" in platform_dict:
+        suffix += 1
+    deduped = f"{base_key}_{suffix}"
+    _LOGGER.warning(
+        "Duplicate entity name '%s', using '%s' instead", base_key, deduped
+    )
+    return deduped
+
+
 async def async_get_config_and_coordinator(hass, platform_domain, discovery_info):
     """Get the config and coordinator for the platform from discovery."""
     registry: ScraperRegistry = hass.data[DOMAIN]

custom_components/multiscrape/registry.py

@@ -3,19 +3,24 @@
 
 import logging
 from dataclasses import dataclass, field
+from typing import TYPE_CHECKING
 
 from homeassistant.const import Platform
 
+if TYPE_CHECKING:
+    from .coordinator import MultiscrapeDataUpdateCoordinator
+    from .scraper import Scraper
+
 _LOGGER = logging.getLogger(__name__)
 
 
 @dataclass
 class ScraperInstance:
-    """Holds all data for a single scraper configuration."""
+    """Hold all data for a single scraper configuration."""
 
     scraper_id: str
-    scraper: object
-    coordinator: object
+    scraper: Scraper
+    coordinator: MultiscrapeDataUpdateCoordinator
     platform_configs: dict[Platform, dict[str, dict]] = field(default_factory=dict)
 
 
@@ -26,6 +31,10 @@ def __init__(self):
         """Initialize an empty registry."""
         self._scrapers: dict[str, ScraperInstance] = {}
 
+    def contains(self, scraper_id: str) -> bool:
+        """Check if a scraper ID is already registered."""
+        return scraper_id in self._scrapers
+
     def register(self, instance: ScraperInstance) -> None:
         """Register a scraper instance by its unique ID."""
         if instance.scraper_id in self._scrapers:

tests/test_init.py

@@ -437,3 +437,70 @@ async def test_async_process_config_skips_platforms_not_in_config(hass: HomeAssi
     assert len(instance.platform_configs.get(Platform.SENSOR, {})) == 1
     assert len(instance.platform_configs.get(Platform.BINARY_SENSOR, {})) == 0
     assert len(instance.platform_configs.get(Platform.BUTTON, {})) == 0
+
+
+@pytest.mark.integration
+@pytest.mark.async_test
+@pytest.mark.timeout(10)
+@pytest.mark.respx
+async def test_duplicate_scraper_names_are_deduplicated(hass: HomeAssistant):
+    """Test that duplicate scraper names get a suffix instead of crashing."""
+    # Arrange
+    _async_setup_shared_data(hass)
+    config = {
+        DOMAIN: [
+            {
+                CONF_NAME: "my_scraper",
+                CONF_RESOURCE: "https://example.com/1",
+                Platform.SENSOR: [{CONF_NAME: "sensor1", "select": ".v1"}],
+            },
+            {
+                CONF_NAME: "my_scraper",
+                CONF_RESOURCE: "https://example.com/2",
+                Platform.SENSOR: [{CONF_NAME: "sensor2", "select": ".v2"}],
+            },
+        ]
+    }
+
+    # Act
+    result = await _async_process_config(hass, config)
+
+    # Assert - both scrapers should be registered with distinct IDs
+    assert result is True
+    registry: ScraperRegistry = hass.data[DOMAIN]
+    assert len(registry.get_all()) == 2
+    assert registry.get("my_scraper") is not None
+    assert registry.get("my_scraper_2") is not None
+
+
+@pytest.mark.integration
+@pytest.mark.async_test
+@pytest.mark.timeout(10)
+@pytest.mark.respx
+async def test_duplicate_entity_names_are_deduplicated(hass: HomeAssistant):
+    """Test that duplicate entity names within a scraper get a suffix."""
+    # Arrange
+    _async_setup_shared_data(hass)
+    config = {
+        DOMAIN: [
+            {
+                CONF_NAME: "my_scraper",
+                CONF_RESOURCE: "https://example.com",
+                Platform.SENSOR: [
+                    {CONF_NAME: "temperature", "select": ".temp1"},
+                    {CONF_NAME: "temperature", "select": ".temp2"},
+                ],
+            }
+        ]
+    }
+
+    # Act
+    await _async_process_config(hass, config)
+
+    # Assert - both sensors should be stored with distinct keys
+    registry: ScraperRegistry = hass.data[DOMAIN]
+    instance = registry.get("my_scraper")
+    sensor_configs = instance.platform_configs[Platform.SENSOR]
+    assert len(sensor_configs) == 2
+    assert "temperature" in sensor_configs
+    assert "temperature_2" in sensor_configs

tests/test_registry.py

@@ -87,3 +87,10 @@ def test_platform_configs_access(registry, make_instance):
 
     retrieved = registry.get("my_scraper")
     assert retrieved.platform_configs[Platform.SENSOR]["temp_sensor"] is sensor_config
+
+
+def test_contains(registry, make_instance):
+    """Test contains checks for registered IDs."""
+    assert registry.contains("my_scraper") is False
+    registry.register(make_instance("my_scraper"))
+    assert registry.contains("my_scraper") is True