Skip to content

Commit 2fc8740

Browse files
Replace ListLike Protocol with list[T] type annotations (#562)
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 344ec8a commit 2fc8740

File tree

10 files changed

+55
-60
lines changed

10 files changed

+55
-60
lines changed

openhands/sdk/context/view.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from collections.abc import Sequence
12
from logging import getLogger
23
from typing import overload
34

@@ -12,7 +13,6 @@
1213
from openhands.sdk.event.base import EventBase, EventID
1314
from openhands.sdk.event.llm_convertible import ActionEvent, ObservationBaseEvent
1415
from openhands.sdk.event.types import ToolCallID
15-
from openhands.sdk.utils.protocol import ListLike
1616

1717

1818
logger = getLogger(__name__)
@@ -143,7 +143,7 @@ def _should_keep_event(
143143
return True
144144

145145
@staticmethod
146-
def from_events(events: ListLike[EventBase]) -> "View":
146+
def from_events(events: Sequence[EventBase]) -> "View":
147147
"""Create a view from a list of events, respecting the semantics of any
148148
condensation events.
149149
"""

openhands/sdk/conversation/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from openhands.sdk.conversation.base import BaseConversation
22
from openhands.sdk.conversation.conversation import Conversation
3-
from openhands.sdk.conversation.event_store import EventLog, ListLike
3+
from openhands.sdk.conversation.event_store import EventLog
4+
from openhands.sdk.conversation.events_list_base import EventsListBase
45
from openhands.sdk.conversation.secrets_manager import SecretsManager
56
from openhands.sdk.conversation.state import ConversationState
67
from openhands.sdk.conversation.stuck_detector import StuckDetector
@@ -17,5 +18,5 @@
1718
"SecretsManager",
1819
"StuckDetector",
1920
"EventLog",
20-
"ListLike",
21+
"EventsListBase",
2122
]

openhands/sdk/conversation/base.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,18 @@
44
from typing import TYPE_CHECKING, Protocol
55

66
from openhands.sdk.conversation.conversation_stats import ConversationStats
7+
from openhands.sdk.conversation.events_list_base import EventsListBase
78
from openhands.sdk.conversation.secrets_manager import SecretValue
89
from openhands.sdk.conversation.types import ConversationCallbackType, ConversationID
910
from openhands.sdk.llm.message import Message
1011
from openhands.sdk.security.confirmation_policy import (
1112
ConfirmationPolicyBase,
1213
NeverConfirm,
1314
)
14-
from openhands.sdk.utils.protocol import ListLike
1515

1616

1717
if TYPE_CHECKING:
1818
from openhands.sdk.conversation.state import AgentExecutionStatus
19-
from openhands.sdk.event.base import EventBase
2019

2120

2221
class ConversationStateProtocol(Protocol):
@@ -28,7 +27,7 @@ def id(self) -> ConversationID:
2827
...
2928

3029
@property
31-
def events(self) -> ListLike["EventBase"]:
30+
def events(self) -> EventsListBase:
3231
"""Access to the events list."""
3332
...
3433

openhands/sdk/conversation/event_store.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from collections.abc import Iterator
44
from typing import SupportsIndex, overload
55

6+
from openhands.sdk.conversation.events_list_base import EventsListBase
67
from openhands.sdk.conversation.persistence_const import (
78
EVENT_FILE_PATTERN,
89
EVENT_NAME_RE,
@@ -11,13 +12,12 @@
1112
from openhands.sdk.event import EventBase, EventID
1213
from openhands.sdk.io import FileStore
1314
from openhands.sdk.logger import get_logger
14-
from openhands.sdk.utils.protocol import ListLike
1515

1616

1717
logger = get_logger(__name__)
1818

1919

20-
class EventLog(ListLike[EventBase]):
20+
class EventLog(EventsListBase):
2121
def __init__(self, fs: FileStore, dir_path: str = EVENTS_DIR) -> None:
2222
self._fs = fs
2323
self._dir = dir_path
@@ -41,15 +41,19 @@ def get_id(self, idx: int) -> EventID:
4141
return self._idx_to_id[idx]
4242

4343
@overload
44-
def __getitem__(self, idx: SupportsIndex, /) -> EventBase: ...
44+
def __getitem__(self, idx: int) -> EventBase: ...
45+
4546
@overload
46-
def __getitem__(self, idx: slice, /) -> list[EventBase]: ...
47+
def __getitem__(self, idx: slice) -> list[EventBase]: ...
4748

48-
def __getitem__(self, idx: SupportsIndex | slice, /) -> EventBase | list[EventBase]:
49+
def __getitem__(self, idx: SupportsIndex | slice) -> EventBase | list[EventBase]:
4950
if isinstance(idx, slice):
5051
start, stop, step = idx.indices(self._length)
51-
return [self[i] for i in range(start, stop, step)]
52+
return [self._get_single_item(i) for i in range(start, stop, step)]
5253
# idx is int-like (SupportsIndex)
54+
return self._get_single_item(idx)
55+
56+
def _get_single_item(self, idx: SupportsIndex) -> EventBase:
5357
i = operator.index(idx)
5458
if i < 0:
5559
i += self._length
@@ -73,8 +77,8 @@ def __iter__(self) -> Iterator[EventBase]:
7377
self._id_to_idx.setdefault(evt_id, i)
7478
yield evt
7579

76-
def append(self, item: EventBase) -> None:
77-
evt_id = item.id
80+
def append(self, event: EventBase) -> None:
81+
evt_id = event.id
7882
# Check for duplicate ID
7983
if evt_id in self._id_to_idx:
8084
existing_idx = self._id_to_idx[evt_id]
@@ -83,7 +87,7 @@ def append(self, item: EventBase) -> None:
8387
)
8488

8589
path = self._path(self._length, event_id=evt_id)
86-
self._fs.write(path, item.model_dump_json(exclude_none=True))
90+
self._fs.write(path, event.model_dump_json(exclude_none=True))
8791
self._idx_to_id[self._length] = evt_id
8892
self._id_to_idx[evt_id] = self._length
8993
self._length += 1
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
from abc import ABC, abstractmethod
2+
from collections.abc import Sequence
3+
4+
from openhands.sdk.event import EventBase
5+
6+
7+
class EventsListBase(Sequence[EventBase], ABC):
8+
"""Abstract base class for event lists that can be appended to.
9+
10+
This provides a common interface for both local EventLog and remote
11+
RemoteEventsList implementations, avoiding circular imports in protocols.
12+
"""
13+
14+
@abstractmethod
15+
def append(self, event: EventBase) -> None:
16+
"""Add a new event to the list."""
17+
...

openhands/sdk/conversation/impl/remote_conversation.py

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from openhands.sdk.agent.base import AgentBase
1212
from openhands.sdk.conversation.base import BaseConversation, ConversationStateProtocol
1313
from openhands.sdk.conversation.conversation_stats import ConversationStats
14+
from openhands.sdk.conversation.events_list_base import EventsListBase
1415
from openhands.sdk.conversation.secrets_manager import SecretValue
1516
from openhands.sdk.conversation.state import AgentExecutionStatus
1617
from openhands.sdk.conversation.types import ConversationCallbackType, ConversationID
@@ -21,7 +22,6 @@
2122
from openhands.sdk.security.confirmation_policy import (
2223
ConfirmationPolicyBase,
2324
)
24-
from openhands.sdk.utils.protocol import ListLike
2525

2626

2727
logger = get_logger(__name__)
@@ -101,7 +101,7 @@ async def _client_loop(self) -> None:
101101
delay = min(delay * 2, 30.0)
102102

103103

104-
class RemoteEventsList(ListLike[EventBase]):
104+
class RemoteEventsList(EventsListBase):
105105
"""A list-like, read-only view of remote conversation events.
106106
107107
On first access it fetches existing events from the server. Afterwards,
@@ -155,6 +155,10 @@ def add_event(self, event: EventBase) -> None:
155155
self._cached_event_ids.add(event.id)
156156
logger.debug(f"Added event {event.id} to local cache")
157157

158+
def append(self, event: EventBase) -> None:
159+
"""Add a new event to the list (for compatibility with EventLog interface)."""
160+
self.add_event(event)
161+
158162
def create_default_callback(self) -> ConversationCallbackType:
159163
"""Create a default callback that adds events to this list."""
160164

@@ -167,30 +171,19 @@ def __len__(self) -> int:
167171
return len(self._cached_events)
168172

169173
@overload
170-
def __getitem__(self, index: SupportsIndex, /) -> EventBase: ...
174+
def __getitem__(self, index: int) -> EventBase: ...
171175

172176
@overload
173-
def __getitem__(self, index: slice, /) -> list[EventBase]: ...
177+
def __getitem__(self, index: slice) -> list[EventBase]: ...
174178

175-
def __getitem__(
176-
self,
177-
index: SupportsIndex | slice,
178-
/,
179-
) -> EventBase | list[EventBase]:
179+
def __getitem__(self, index: SupportsIndex | slice) -> EventBase | list[EventBase]:
180180
with self._lock:
181181
return self._cached_events[index]
182182

183183
def __iter__(self):
184184
with self._lock:
185185
return iter(self._cached_events)
186186

187-
def append(self, event: EventBase) -> None:
188-
# For remote conversations, events are added via API calls
189-
# This method is here for interface compatibility but shouldn't be used directly
190-
raise NotImplementedError(
191-
"Cannot directly append events to remote conversation"
192-
)
193-
194187

195188
class RemoteState(ConversationStateProtocol):
196189
"""A state-like interface for accessing remote conversation state."""

openhands/sdk/conversation/state.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# state.py
22
import json
3+
from collections.abc import Sequence
34
from enum import Enum
45
from typing import TYPE_CHECKING
56

@@ -21,7 +22,6 @@
2122
NeverConfirm,
2223
)
2324
from openhands.sdk.utils.models import OpenHandsModel
24-
from openhands.sdk.utils.protocol import ListLike
2525

2626

2727
logger = get_logger(__name__)
@@ -107,9 +107,9 @@ def model_post_init(self, __context) -> None:
107107
# Initialize FIFOLock
108108
FIFOLock.__init__(self)
109109

110-
# ===== Public "events" facade (ListLike[Event]) =====
110+
# ===== Public "events" facade (Sequence[Event]) =====
111111
@property
112-
def events(self) -> ListLike[EventBase]:
112+
def events(self) -> EventLog:
113113
return self._events
114114

115115
@property
@@ -231,7 +231,7 @@ def __setattr__(self, name, value):
231231
raise e
232232

233233
@staticmethod
234-
def get_unmatched_actions(events: ListLike[EventBase]) -> list[ActionEvent]:
234+
def get_unmatched_actions(events: Sequence[EventBase]) -> list[ActionEvent]:
235235
"""Find actions in the event history that don't have matching observations.
236236
237237
This method identifies ActionEvents that don't have corresponding

openhands/sdk/utils/__init__.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
"""Utility functions for the OpenHands SDK."""
22

3-
from .protocol import ListLike
43
from .truncate import (
54
DEFAULT_TEXT_CONTENT_LIMIT,
65
DEFAULT_TRUNCATE_NOTICE,
@@ -12,5 +11,4 @@
1211
"DEFAULT_TEXT_CONTENT_LIMIT",
1312
"DEFAULT_TRUNCATE_NOTICE",
1413
"maybe_truncate",
15-
"ListLike",
1614
]

openhands/sdk/utils/protocol.py

Lines changed: 0 additions & 16 deletions
This file was deleted.

tests/sdk/context/condenser/test_llm_summarizing_condenser.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
MetricsSnapshot,
1919
TextContent,
2020
)
21-
from openhands.sdk.utils.protocol import ListLike
2221

2322

2423
def message_event(content: str) -> MessageEvent:
@@ -84,13 +83,13 @@ def test_should_condense(mock_llm: LLM) -> None:
8483

8584
# Create events below the threshold
8685
small_events = [message_event(f"Event {i}") for i in range(max_size)]
87-
small_view = View.from_events(cast(ListLike[EventBase], small_events))
86+
small_view = View.from_events(small_events)
8887

8988
assert not condenser.should_condense(small_view)
9089

9190
# Create events above the threshold
9291
large_events = [message_event(f"Event {i}") for i in range(max_size + 1)]
93-
large_view = View.from_events(cast(ListLike[EventBase], large_events))
92+
large_view = View.from_events(large_events)
9493

9594
assert condenser.should_condense(large_view)
9695

@@ -101,7 +100,7 @@ def test_condense_returns_view_when_no_condensation_needed(mock_llm: LLM) -> Non
101100
condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=max_size)
102101

103102
events: list[EventBase] = [message_event(f"Event {i}") for i in range(max_size)]
104-
view = View.from_events(cast(ListLike[EventBase], events))
103+
view = View.from_events(events)
105104

106105
result = condenser.condense(view)
107106

@@ -123,7 +122,7 @@ def test_condense_returns_condensation_when_needed(mock_llm: LLM) -> None:
123122
cast(Any, mock_llm).set_mock_response_content("Summary of forgotten events")
124123

125124
events: list[EventBase] = [message_event(f"Event {i}") for i in range(max_size + 1)]
126-
view = View.from_events(cast(ListLike[EventBase], events))
125+
view = View.from_events(events)
127126

128127
result = condenser.condense(view)
129128

@@ -160,7 +159,7 @@ def test_get_condensation_with_previous_summary(mock_llm: LLM) -> None:
160159
events[:keep_first] + [condensation] + events[keep_first:]
161160
)
162161

163-
view = View.from_events(cast(ListLike[EventBase], events_with_condensation))
162+
view = View.from_events(events_with_condensation)
164163

165164
result = condenser.get_condensation(view)
166165

0 commit comments

Comments
 (0)