Repository: Textualize/textual

Issue:
`Option` indexing appears to break after an `OptionList.clear_options` and `OptionList.add_option(s)`
This looks to be a bug introduced in v0.48 of Textual. Consider this code:

```python
from textual import on
from textual.app import App, ComposeResult
from textual.reactive import var
from textual.widgets import Button, OptionList
from textual.widgets.option_list import Option

class OptionListReAdd(App[None]):

    count: var[int] = var(0)

    def compose(self) -> ComposeResult:
        yield Button("Add again")
        yield OptionList(Option(f"This is the option we'll keep adding {self.count}", id="0"))

    @on(Button.Pressed)
    def readd(self) -> None:
        self.count += 1
        _ = self.query_one(OptionList).get_option("0")
        self.query_one(OptionList).clear_options().add_option(
            Option(f"This is the option we'll keep adding {self.count}", id="0")
        )

if __name__ == "__main__":
    OptionListReAdd().run()
```

With v0.47.1, you can press the button many times and the code does what you'd expect; the option keeps being replaced, and it can always be acquired back with a `get_option` on its ID.

With v0.48.0/1 you get the following error the second time you press the button:

```python
╭────────────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────────────╮
│ /Users/davep/develop/python/textual-sandbox/option_list_readd.py:18 in readd                                                                   │
│                                                                                                                                                │
│   15 │   @on(Button.Pressed)                                                                                                                   │
│   16 │   def readd(self) -> None:                                                                                                              │
│   17 │   │   self.count += 1                                                                                                                   │
│ ❱ 18 │   │   _ = self.query_one(OptionList).get_option("0")                                                                                    │
│   19 │   │   self.query_one(OptionList).clear_options().add_option(                                                                            │
│   20 │   │   │   Option(f"This is the option we'll keep adding {self.count}", id="0")                                                          │
│   21 │   │   )                                                                                                                                 │
│                                                                                                                                                │
│ ╭──────────────────────────────── locals ─────────────────────────────────╮                                                                    │
│ │ self = OptionListReAdd(title='OptionListReAdd', classes={'-dark-mode'}) │                                                                    │
│ ╰─────────────────────────────────────────────────────────────────────────╯                                                                    │
│                                                                                                                                                │
│ /Users/davep/develop/python/textual-sandbox/.venv/lib/python3.10/site-packages/textual/widgets/_option_list.py:861 in get_option               │
│                                                                                                                                                │
│    858 │   │   Raises:                                                                          ╭───────── locals ─────────╮                   │
│    859 │   │   │   OptionDoesNotExist: If no option has the given ID.                           │ option_id = '0'          │                   │
│    860 │   │   """                                                                              │      self = OptionList() │                   │
│ ❱  861 │   │   return self.get_option_at_index(self.get_option_index(option_id))                ╰──────────────────────────╯                   │
│    862 │                                                                                                                                       │
│    863 │   def get_option_index(self, option_id: str) -> int:                                                                                  │
│    864 │   │   """Get the index of the option with the given ID.                                                                               │
│                                                                                                                                                │
│ /Users/davep/develop/python/textual-sandbox/.venv/lib/python3.10/site-packages/textual/widgets/_option_list.py:845 in get_option_at_index      │
│                                                                                                                                                │
│    842 │   │   try:                                                                             ╭─────── locals ───────╮                       │
│    843 │   │   │   return self._options[index]                                                  │ index = 1            │                       │
│    844 │   │   except IndexError:                                                               │  self = OptionList() │                       │
│ ❱  845 │   │   │   raise OptionDoesNotExist(                                                    ╰──────────────────────╯                       │
│    846 │   │   │   │   f"There is no option with an index of {index}"                                                                          │
│    847 │   │   │   ) from None                                                                                                                 │
│    848                                                                                                                                         │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
OptionDoesNotExist: There is no option with an index of 1
```

Tagging @willmcgugan for triage.

Perhaps related to b1aaea781297699e029e120a55ca49b1361d056e

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d3796faf7..fe0830770 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+### Fixed
+
+- Fixed broken `OptionList` `Option.id` mappings https://github.com/Textualize/textual/issues/4101
+
 ### Changed
 
 - Breaking change: keyboard navigation in `RadioSet`, `ListView`, `OptionList`, and `SelectionList`, no longer allows highlighting disabled items https://github.com/Textualize/textual/issues/3881
diff --git a/src/textual/widgets/_option_list.py b/src/textual/widgets/_option_list.py
index a9ba696bc..888140bbe 100644
--- a/src/textual/widgets/_option_list.py
+++ b/src/textual/widgets/_option_list.py
@@ -573,15 +573,16 @@ class OptionList(ScrollView, can_focus=True):
             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. Add them to the
-            # list of options and map option IDs to their new indices.
+            # Pull out the content that is genuine options, create any new
+            # ID mappings required, then add the new options to the option
+            # list.
             new_options = [item for item in content if isinstance(item, Option)]
-            self._options.extend(new_options)
             for new_option_index, new_option in enumerate(
                 new_options, start=len(self._options)
             ):
                 if new_option.id:
                     self._option_ids[new_option.id] = new_option_index
+            self._options.extend(new_options)
 
             self._refresh_content_tracking(force=True)
             self.refresh()


Test patch:
diff --git a/tests/option_list/test_option_list_id_stability.py b/tests/option_list/test_option_list_id_stability.py
new file mode 100644
index 000000000..bd746914b
--- /dev/null
+++ b/tests/option_list/test_option_list_id_stability.py
@@ -0,0 +1,22 @@
+"""Tests inspired by https://github.com/Textualize/textual/issues/4101"""
+
+from __future__ import annotations
+
+from textual.app import App, ComposeResult
+from textual.widgets import OptionList
+from textual.widgets.option_list import Option
+
+
+class OptionListApp(App[None]):
+    """Test option list application."""
+
+    def compose(self) -> ComposeResult:
+        yield OptionList()
+
+
+async def test_get_after_add() -> None:
+    """It should be possible to get an option by ID after adding."""
+    async with OptionListApp().run_test() as pilot:
+        option_list = pilot.app.query_one(OptionList)
+        option_list.add_option(Option("0", id="0"))
+        assert option_list.get_option("0").id == "0"