OptionList safe duplicate check

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `Pilot.click`/`Pilot.hover` can't use `Screen` as a selector https://github.com/Textualize/textual/issues/3395
 - App exception when a `Tree` is initialized/mounted with `disabled=True` https://github.com/Textualize/textual/issues/3407
 - Fixed application freeze when pasting an emoji into an application on Windows https://github.com/Textualize/textual/issues/3178
+- Fixed duplicate option ID handling in the `OptionList` https://github.com/Textualize/textual/issues/3455
 
 ### Added
 

src/textual/widgets/_option_list.py

@@ -540,10 +540,6 @@ def _refresh_content_tracking(self, force: bool = False) -> None:
                 if content.id is not None:
                     # The option has an ID set, create a mapping from that
                     # ID to the option so we can use it later.
-                    if content.id in option_ids:
-                        raise DuplicateID(
-                            f"The option list already has an option with id '{content.id}'"
-                        )
                     option_ids[content.id] = option
                 option += 1
             else:
@@ -558,6 +554,30 @@ def _refresh_content_tracking(self, force: bool = False) -> None:
         # list, set the virtual size.
         self.virtual_size = Size(self.scrollable_content_region.width, len(self._lines))
 
+    def _duplicate_id_check(self, candidate_items: list[OptionListContent]) -> None:
+        """Check the items to be added for any duplicates.
+
+        Args:
+            candidate_items: The items that are going be added.
+
+        Raises:
+            DuplicateID: If there is an attempt to use a duplicate ID.
+        """
+        # We're only interested in options, and only those that have IDs.
+        new_options = [
+            item
+            for item in candidate_items
+            if isinstance(item, Option) and item.id is not None
+        ]
+        # Get the set of new IDs that we're being given.
+        new_option_ids = set(option.id for option in new_options)
+        # Now check for duplicates, both internally amongst the new items
+        # incoming, and also against all the current known IDs.
+        if len(new_options) != len(new_option_ids) or not new_option_ids.isdisjoint(
+            self._option_ids
+        ):
+            raise DuplicateID("Attempt made to add options with duplicate IDs.")
+
     def add_options(self, items: Iterable[NewOptionListContent]) -> Self:
         """Add new options to the end of the option list.
 
@@ -569,12 +589,18 @@ def add_options(self, items: Iterable[NewOptionListContent]) -> Self:
 
         Raises:
             DuplicateID: If there is an attempt to use a duplicate ID.
+
+        Note:
+            All options are checked for duplicate IDs *before* any option is
+            added. A duplicate ID will cause none of the passed items to be
+            added to the option list.
         """
         # Only work if we have items to add; but don't make a fuss out of
         # zero items to add, just carry on like nothing happened.
         if items:
             # Turn any incoming values into valid content for the list.
             content = [self._make_content(item) for item in items]
+            self._duplicate_id_check(content)
             self._contents.extend(content)
             # Pull out the content that is genuine options and add them to the
             # list of options.

tests/option_list/test_option_list_create.py

@@ -119,5 +119,37 @@ async def test_add_later() -> None:
 async def test_create_with_duplicate_id() -> None:
     """Adding an option with a duplicate ID should be an error."""
     async with OptionListApp().run_test() as pilot:
+        option_list = pilot.app.query_one(OptionList)
+        assert option_list.option_count == 5
+        with pytest.raises(DuplicateID):
+            option_list.add_option(Option("dupe", id="3"))
+        assert option_list.option_count == 5
+
+
+async def test_create_with_duplicate_id_and_subsequent_non_dupes() -> None:
+    """Adding an option with a duplicate ID should be an error."""
+    async with OptionListApp().run_test() as pilot:
+        option_list = pilot.app.query_one(OptionList)
+        assert option_list.option_count == 5
         with pytest.raises(DuplicateID):
-            pilot.app.query_one(OptionList).add_option(Option("dupe", id="3"))
+            option_list.add_option(Option("dupe", id="3"))
+        assert option_list.option_count == 5
+        option_list.add_option(Option("Not a dupe", id="6"))
+        assert option_list.option_count == 6
+        option_list.add_option(Option("Not a dupe", id="7"))
+        assert option_list.option_count == 7
+
+
+async def test_adding_multiple_duplicates_at_once() -> None:
+    """Adding duplicates together than aren't existing duplicates should be an error."""
+    async with OptionListApp().run_test() as pilot:
+        option_list = pilot.app.query_one(OptionList)
+        assert option_list.option_count == 5
+        with pytest.raises(DuplicateID):
+            option_list.add_options(
+                [
+                    Option("dupe", id="42"),
+                    Option("dupe", id="42"),
+                ]
+            )
+        assert option_list.option_count == 5