diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index 4e18a0a73cb..d96e9d28fb2 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -1,57 +1,108 @@ +import html +import os +from collections.abc import Iterator + from syft_client.sync.peers.peer import Peer, PeerState -from syft_client.sync.reprs.peer_repr import get_peer_list_table +# Emoji + friendly label shown per peer state in the summary view. +_STATE_DISPLAY = { + PeerState.ACCEPTED: ("✅", "connected"), + PeerState.REQUESTED_BY_ME: ("⏳", "requested_by_us (waiting for approval)"), + PeerState.REQUESTED_BY_PEER: ( + "📩", + "requested_by_peer (waiting for your approval)", + ), + PeerState.REJECTED: ("❌", "rejected"), +} + +# Display order: accepted first, then outgoing requests, then incoming requests. +_STATE_ORDER = { + PeerState.ACCEPTED: 0, + PeerState.REQUESTED_BY_ME: 1, + PeerState.REQUESTED_BY_PEER: 2, +} + + +def _peer_sort_key(peer: Peer) -> int: + return _STATE_ORDER.get(peer.state, 3) -class PeerList(list): - def __init__(self, *args: Peer, **kwargs): + +class PeerList: + """A list-like container for Peer objects with a friendly summary display.""" + + def __init__(self, peers: list[Peer] | None = None) -> None: """ - PeerList is a list specifically for Peer objects. - Validates that all items are Peer objects and that they are sorted correctly. + Validates that all items are Peer objects, then sorts them for display: + accepted first, then requested_by_me, then requested_by_peer. """ - super().__init__(*args, **kwargs) - # Validate all items are Peer objects - for item in self: + peers = list(peers) if peers else [] + for item in peers: if not isinstance(item, Peer): raise TypeError( f"All items in PeerList must be Peer objects, but got {type(item)}" ) - # Validate sorting (approved before pending) - self._validate_sorting() - - def _validate_sorting(self): - """Ensure peers are sorted: accepted, then requested_by_me, then requested_by_peer""" - order = { - PeerState.ACCEPTED: 0, - PeerState.REQUESTED_BY_ME: 1, - PeerState.REQUESTED_BY_PEER: 2, - } - last_order = -1 - for peer in self: - peer_order = order.get(peer.state, 3) - if peer_order < last_order: - raise ValueError( - "PeerList must be sorted: accepted first, then requested_by_me, then requested_by_peer" - ) - last_order = peer_order + # ensure consistent ordering for display + self._peers: list[Peer] = sorted(peers, key=_peer_sort_key) + + def __len__(self) -> int: + return len(self._peers) + + def __iter__(self) -> Iterator[Peer]: + return iter(self._peers) def __getitem__(self, index: str | int) -> Peer: if isinstance(index, int): - return super().__getitem__(index) + return self._peers[index] elif isinstance(index, str): - key = index - for peer in self: - if peer.email == key: - return peer - raise ValueError(f"Peer with email {index} not found") + try: + return next(peer for peer in self._peers if peer.email == index) + except StopIteration: + raise ValueError(f"Peer with email {index} not found") from None else: - raise ValueError(f"Invalid index type: {type(index)}") + raise TypeError(f"Invalid index type: {type(index)}") - def _repr_html_(self) -> str: - """Used by Jupyter to display Rich HTML.""" - peers = [p for p in self] - return get_peer_list_table(peers) + def _summary_text(self) -> str: + """Clean, human-friendly summary of the peers and what to do next.""" + if not self: + return "👥 You have no peers yet." + + lines = [f"👥 Your peers ({len(self)}):", ""] + pad = max(len(peer.email) for peer in self) + connected = [] + has_pending = False + for peer in self: + emoji, label = _STATE_DISPLAY.get(peer.state, ("❓", str(peer.state.value))) + lines.append(f" {emoji} {peer.email.ljust(pad)} — {label}") + if peer.state == PeerState.ACCEPTED: + connected.append(peer) + elif peer.state in (PeerState.REQUESTED_BY_ME, PeerState.REQUESTED_BY_PEER): + has_pending = True + + if connected: + lines += [ + "", + "💡 Tip: Once connected, you can access their datasets with:", + f' client.datasets.get("dataset_name", datasite="{connected[0].email}")', + ] + if has_pending: + lines += [ + "", + "⏳ You have pending requests waiting for approval — " + "follow up with the peer or check back later.", + ] + return "\n".join(lines) + + def __str__(self) -> str: + """Clean summary shown by print() and outside notebooks.""" + return self._summary_text() + + def _repr_html_(self) -> str | None: + """Used by Jupyter to display the summary; falls back to text elsewhere.""" + if os.environ.get("SYFT_NO_REPR_HTML", "").lower() in {"1", "true", "yes"}: + return None + return f"
{html.escape(self._summary_text())}
" def __repr__(self): - """Fallback for normal REPL""" + """Technical repr for debugging / normal REPL.""" peers = [p for p in self] return f"PeerList({peers!r})" diff --git a/tests/unit/test_datasets_jobs_repr.py b/tests/unit/test_datasets_jobs_repr.py index c77026cf121..42dd28b8f9d 100644 --- a/tests/unit/test_datasets_jobs_repr.py +++ b/tests/unit/test_datasets_jobs_repr.py @@ -1,9 +1,9 @@ """Tests for SyftDatasetManager and JobsList repr and indexing.""" import pytest - from syft_client.sync.syftbox_manager import SyftboxManager from syft_job.job import JobInfo, JobsList + from tests.unit.utils import create_tmp_dataset_files @@ -141,7 +141,7 @@ def _make_job_info(name: str, status: str = "pending") -> JobInfo: from syft_job.client import JobClient from syft_job.config import SyftJobConfig - from syft_job.manager import JobRef + from syft_job.job_ref import JobRef from syft_job.models import JobState, JobStatus, JobSubmissionMetadata config = SyftJobConfig( diff --git a/tests/unit/test_peer_list_repr.py b/tests/unit/test_peer_list_repr.py new file mode 100644 index 00000000000..eab825d5b53 --- /dev/null +++ b/tests/unit/test_peer_list_repr.py @@ -0,0 +1,147 @@ +"""Tests for PeerList's human-friendly summary rendering (__str__/_repr_html_/__repr__).""" + +import pytest +from syft_client.sync.peers.peer import Peer, PeerState +from syft_client.sync.peers.peer_list import PeerList + + +def _mixed_peer_list() -> PeerList: + """A PeerList with one peer in each visible state, in required sort order.""" + return PeerList( + [ + Peer(email="do@org.com", state=PeerState.ACCEPTED), + Peer(email="other@org.com", state=PeerState.REQUESTED_BY_ME), + Peer(email="incoming@org.com", state=PeerState.REQUESTED_BY_PEER), + ] + ) + + +def test_str_header_shows_count(): + pl = _mixed_peer_list() + assert str(pl).startswith("👥 Your peers (3):") + + +def test_str_renders_emoji_and_friendly_labels(): + text = str(_mixed_peer_list()) + assert "✅ do@org.com" in text + assert "— connected" in text + assert "⏳ other@org.com" in text + assert "requested_by_us (waiting for approval)" in text + assert "📩 incoming@org.com" in text + assert "requested_by_peer (waiting for your approval)" in text + + +def test_connected_tip_present_when_connected_peer_exists(): + text = str(_mixed_peer_list()) + assert "client.datasets.get" in text + assert 'datasite="do@org.com"' in text + + +def test_connected_tip_absent_when_no_connected_peer(): + pl = PeerList( + [ + Peer(email="other@org.com", state=PeerState.REQUESTED_BY_ME), + Peer(email="incoming@org.com", state=PeerState.REQUESTED_BY_PEER), + ] + ) + assert "client.datasets.get" not in str(pl) + + +def test_pending_tip_present_when_pending_peer_exists(): + text = str(_mixed_peer_list()) + assert "pending requests" in text + + +def test_pending_tip_absent_when_only_connected(): + pl = PeerList([Peer(email="do@org.com", state=PeerState.ACCEPTED)]) + assert "pending requests" not in str(pl) + + +def test_empty_peer_list_message(): + assert str(PeerList([])) == "👥 You have no peers yet." + + +def test_repr_html_wraps_and_escapes_summary(): + html = _mixed_peer_list()._repr_html_() + assert html is not None + assert html.startswith("
")
+    assert html.endswith("
") + assert "👥 Your peers (3):" in html + + +def test_repr_html_escapes_html_in_email(): + pl = PeerList([Peer(email="a@org.com", state=PeerState.ACCEPTED)]) + html = pl._repr_html_() + assert html is not None + assert "a@org.com" not in html + assert "a<b>@org.com" in html + + +def test_repr_html_returns_none_when_disabled(monkeypatch): + monkeypatch.setenv("SYFT_NO_REPR_HTML", "1") + assert _mixed_peer_list()._repr_html_() is None + + +def test_repr_preserves_technical_string(): + assert repr(_mixed_peer_list()).startswith("PeerList(") + + +def test_peer_list_len_and_iter(): + pl = _mixed_peer_list() + assert len(pl) == 3 + assert [p.email for p in pl] == [ + "do@org.com", + "other@org.com", + "incoming@org.com", + ] + + +def test_peer_list_getitem_int(): + assert _mixed_peer_list()[0].email == "do@org.com" + + +def test_peer_list_getitem_str(): + assert _mixed_peer_list()["other@org.com"].state == PeerState.REQUESTED_BY_ME + + +def test_peer_list_getitem_str_not_found(): + with pytest.raises(ValueError, match="not found"): + _mixed_peer_list()["nobody@org.com"] + + +def test_peer_list_getitem_invalid_type(): + with pytest.raises(TypeError, match="Invalid index type"): + _mixed_peer_list()[3.14] # type: ignore[index] + + +def test_peer_list_rejects_non_peer_items(): + with pytest.raises(TypeError): + PeerList(["not-a-peer"]) # type: ignore[list-item] + + +def test_peer_list_sorts_unsorted_input(): + # Construction sorts by state (accepted → requested_by_me → requested_by_peer) + # rather than requiring the caller to pre-sort. + pl = PeerList( + [ + Peer(email="incoming@org.com", state=PeerState.REQUESTED_BY_PEER), + Peer(email="connected@org.com", state=PeerState.ACCEPTED), + Peer(email="outgoing@org.com", state=PeerState.REQUESTED_BY_ME), + ] + ) + assert [p.state for p in pl] == [ + PeerState.ACCEPTED, + PeerState.REQUESTED_BY_ME, + PeerState.REQUESTED_BY_PEER, + ] + + +def test_peer_list_sort_is_stable_within_a_state(): + # Peers sharing a state keep their input order (stable sort). + pl = PeerList( + [ + Peer(email="second@org.com", state=PeerState.ACCEPTED), + Peer(email="first@org.com", state=PeerState.ACCEPTED), + ] + ) + assert [p.email for p in pl] == ["second@org.com", "first@org.com"]