feat: improve peer list display

packages/syft-rds/tests/test_datasets_jobs_repr.py

@@ -1,9 +1,9 @@
 """Tests for SyftDatasetManager and JobsList repr and indexing."""
 
 import pytest
-
 from syft_rds import SyftRDSClient
 from syft_job.job import JobInfo, JobsList
+
 from dataset_test_utils import create_tmp_dataset_files
 
 

syft/sync/peers/peer_list.py

@@ -1,57 +1,108 @@
+import html
+import os
+from collections.abc import Iterator
+
 from syft.sync.peers.peer import Peer, PeerState
-from syft.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"<pre>{html.escape(self._summary_text())}</pre>"
 
     def __repr__(self):
-        """Fallback for normal REPL"""
+        """Technical repr for debugging / normal REPL."""
         peers = [p for p in self]
         return f"PeerList({peers!r})"

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.sync.peers.peer import Peer, PeerState
+from syft.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("<pre>")
+    assert html.endswith("</pre>")
+    assert "👥 Your peers (3):" in html
+
+
+def test_repr_html_escapes_html_in_email():
+    pl = PeerList([Peer(email="a<b>@org.com", state=PeerState.ACCEPTED)])
+    html = pl._repr_html_()
+    assert html is not None
+    assert "a<b>@org.com" not in html
+    assert "a&lt;b&gt;@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"]