From 2bc561af488771409955dfcbe66af31ad8a95ace Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 13:43:06 -0300 Subject: [PATCH 1/8] Fix JobRef import path --- tests/unit/test_datasets_jobs_repr.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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( From 2ca5becc2bcb832abf20f7f588d9d55ed0cc04c6 Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 10:08:45 -0300 Subject: [PATCH 2/8] feat: improve peer list display - Add __str__ and _repr_html_ that print a clean summary when accessed in a notebook cell. - Preserve current __repr__ behavior. --- syft_client/sync/peers/peer_list.py | 89 ++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 22 deletions(-) diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index 4e18a0a73cb..583b5db3739 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -1,14 +1,27 @@ +import html +import os + 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"), +} class PeerList(list): - def __init__(self, *args: Peer, **kwargs): + def __init__(self, peers: list[Peer]): """ PeerList is a list specifically for Peer objects. Validates that all items are Peer objects and that they are sorted correctly. """ - super().__init__(*args, **kwargs) + super().__init__(peers) # Validate all items are Peer objects for item in self: if not isinstance(item, Peer): @@ -25,33 +38,65 @@ def _validate_sorting(self): 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 + sorted_peers = sorted(self, key=lambda p: order.get(p.state, 3)) + if list(self) != sorted_peers: + raise ValueError( + "PeerList must be sorted: accepted first, then requested_by_me, then requested_by_peer" + ) - def __getitem__(self, index: str | int) -> Peer: + def __getitem__(self, index: str | int) -> Peer: # ty:ignore[invalid-method-override] if isinstance(index, int): return super().__getitem__(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 if peer.email == index) + except StopIteration: + raise ValueError(f"Peer with email {index} not found") else: raise ValueError(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 "SYFT_NO_REPR_HTML" in os.environ: + 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})" From 2c49660f6cb52214c2392b83c76be3ee5ebfc80c Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 10:23:44 -0300 Subject: [PATCH 3/8] feat: add tests for PeerList summary rendering and HTML representation --- tests/unit/test_peer_list_repr.py | 85 +++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 tests/unit/test_peer_list_repr.py diff --git a/tests/unit/test_peer_list_repr.py b/tests/unit/test_peer_list_repr.py new file mode 100644 index 00000000000..06490349924 --- /dev/null +++ b/tests/unit/test_peer_list_repr.py @@ -0,0 +1,85 @@ +"""Tests for PeerList's human-friendly summary rendering (__str__/_repr_html_/__repr__).""" + +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(") From a601283c231214ec919316110f779d2a9327d42c Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 11:04:58 -0300 Subject: [PATCH 4/8] feat: modify PeerList to use composition There's no good reason for PeerList to inherit from list. It's a LSP violation and no code relies on it being a list. --- syft_client/sync/peers/peer_list.py | 57 ++++++++++++++------------ tests/unit/test_peer_list_repr.py | 62 +++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 26 deletions(-) diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index 583b5db3739..fd876c8f37a 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -1,5 +1,6 @@ import html import os +from collections.abc import Iterator from syft_client.sync.peers.peer import Peer, PeerState @@ -14,46 +15,50 @@ 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: + """A list-like container for Peer objects with a friendly summary display.""" -class PeerList(list): def __init__(self, peers: list[Peer]): """ - 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__(peers) - # Validate all items are Peer objects - for item in self: + 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, - } - sorted_peers = sorted(self, key=lambda p: order.get(p.state, 3)) - if list(self) != sorted_peers: - raise ValueError( - "PeerList must be sorted: accepted first, then requested_by_me, then requested_by_peer" - ) - - def __getitem__(self, index: str | int) -> Peer: # ty:ignore[invalid-method-override] + # 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): try: - return next(peer for peer in self if peer.email == index) + return next(peer for peer in self._peers if peer.email == index) except StopIteration: raise ValueError(f"Peer with email {index} not found") else: - raise ValueError(f"Invalid index type: {type(index)}") + raise TypeError(f"Invalid index type: {type(index)}") def _summary_text(self) -> str: """Clean, human-friendly summary of the peers and what to do next.""" diff --git a/tests/unit/test_peer_list_repr.py b/tests/unit/test_peer_list_repr.py index 06490349924..eab825d5b53 100644 --- a/tests/unit/test_peer_list_repr.py +++ b/tests/unit/test_peer_list_repr.py @@ -1,5 +1,6 @@ """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 @@ -83,3 +84,64 @@ def test_repr_html_returns_none_when_disabled(monkeypatch): 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"] From 6a7ab3ccd2f300f292044d83349b297255653420 Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 11:52:03 -0300 Subject: [PATCH 5/8] fix: ensure PeerList initialization is backwards compatible --- syft_client/sync/peers/peer_list.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index fd876c8f37a..17d24020fb0 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -30,11 +30,12 @@ def _peer_sort_key(peer: Peer) -> int: class PeerList: """A list-like container for Peer objects with a friendly summary display.""" - def __init__(self, peers: list[Peer]): + def __init__(self, peers: list[Peer] | None = None) -> None: """ Validates that all items are Peer objects, then sorts them for display: accepted first, then requested_by_me, then requested_by_peer. """ + peers = peers or [] for item in peers: if not isinstance(item, Peer): raise TypeError( From 856cb99da7d41a63cf98fca5486f98b254ea15c2 Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 11:52:35 -0300 Subject: [PATCH 6/8] fix: reduce traceback noise --- syft_client/sync/peers/peer_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index 17d24020fb0..fa25a308dfe 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -57,7 +57,7 @@ def __getitem__(self, index: str | int) -> Peer: try: return next(peer for peer in self._peers if peer.email == index) except StopIteration: - raise ValueError(f"Peer with email {index} not found") + raise ValueError(f"Peer with email {index} not found") from None else: raise TypeError(f"Invalid index type: {type(index)}") From 6a16cef8cf256205c849008df3e89f1977975090 Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 11:53:45 -0300 Subject: [PATCH 7/8] fix: env-var should be treated as boolean --- syft_client/sync/peers/peer_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index fa25a308dfe..cc2db6822b5 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -98,7 +98,7 @@ def __str__(self) -> str: def _repr_html_(self) -> str | None: """Used by Jupyter to display the summary; falls back to text elsewhere.""" - if "SYFT_NO_REPR_HTML" in os.environ: + if os.environ.get("SYFT_NO_REPR_HTML", "").lower() in {"1", "true", "yes"}: return None return f"
{html.escape(self._summary_text())}
" From a7c98eff3281b16daefe274a96f38b094d3839b7 Mon Sep 17 00:00:00 2001 From: Pedro Werneck Date: Tue, 14 Jul 2026 14:16:55 -0300 Subject: [PATCH 8/8] fix: fix PeerList initialization --- syft_client/sync/peers/peer_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syft_client/sync/peers/peer_list.py b/syft_client/sync/peers/peer_list.py index cc2db6822b5..d96e9d28fb2 100644 --- a/syft_client/sync/peers/peer_list.py +++ b/syft_client/sync/peers/peer_list.py @@ -35,7 +35,7 @@ def __init__(self, peers: list[Peer] | None = None) -> None: Validates that all items are Peer objects, then sorts them for display: accepted first, then requested_by_me, then requested_by_peer. """ - peers = peers or [] + peers = list(peers) if peers else [] for item in peers: if not isinstance(item, Peer): raise TypeError(