feat: add live frontend API - #255
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds typed live event and view APIs, hub-backed actor projections, versioned snapshot validation and restoration, byte-based metrics, availability-aware scheduling, and listener-driven examples and tests. ChangesLive observation and persistence integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Completed the full self-review follow-up in 12 ordered commits. The update replaces strong frontend/actor cycles with weak back-references, makes torrent/peer/tracker terminal states irreversible, closes subscriptions correctly, uses opaque tracker IDs, unifies engine fanout around Validation on the pushed head ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/libtortillas/src/peer/actor.rs (1)
658-686: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winFrontend update fires on every peer wire message, not just state-relevant ones.
self.frontend.update(...)at line 685 runs unconditionally after the wholematch msg { ... }block, soKeepAlive,Request,Cancel, andExtendedmessages — none of which change anyPeerViewfield — still trigger a fullPeerViewrebuild plus a chain throughFrontendPublisher::peer_updated→torrent_view()(clones the wholeEngineView.torrentsVec) →publish_torrent→ the shared engine-wideLivePublisherlock/broadcast. During active piece exchange this is the highest-frequency code path per peer, and it now funnels through one shared lock per engine on every single message across every peer/torrent.Move the update into only the match arms that actually change
PeerViewfields (Choke,Unchoke,Interested,NotInterested,Have,Piece,Bitfield), and drop it forKeepAlive/Request/Cancel/Extended/unexpectedHandshake.♻️ Proposed fix
PeerMessages::Choke => { self.peer.set_am_choked(true); trace!("Peer choked us"); + self.frontend.update(PeerView::from_peer(&self.peer, true)); } PeerMessages::Unchoke => { self.peer.update_last_optimistic_unchoke(); self.peer.set_am_choked(false); self.flush_queue().await; self.flush_block_requests().await; self.notify_ready().await; trace!("Peer unchoked us"); + self.frontend.update(PeerView::from_peer(&self.peer, true)); } PeerMessages::Interested => { self.peer.set_interested(true); trace!("Peer is interested in our pieces"); + self.frontend.update(PeerView::from_peer(&self.peer, true)); } PeerMessages::NotInterested => { self.peer.set_interested(false); trace!("Peer is not interested in our pieces"); + self.frontend.update(PeerView::from_peer(&self.peer, true)); } ... PeerMessages::Piece(index, offset, data) => { ... + self.frontend.update(PeerView::from_peer(&self.peer, true)); } ... PeerMessages::Bitfield(bitfield) => { self.peer.pieces = bitfield; self.determine_interest().await; self.notify_ready().await; + self.frontend.update(PeerView::from_peer(&self.peer, true)); } PeerMessages::Handshake(_) => { warn!("Received unexpected handshake from peer"); } } - self.frontend.update(PeerView::from_peer(&self.peer, true)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/peer/actor.rs` around lines 658 - 686, Move the frontend update out of the unconditional post-match call in the peer message handler and invoke it only in the state-changing arms: Choke, Unchoke, Interested, NotInterested, Have, Piece, and Bitfield. Do not update the frontend for KeepAlive, Request, Cancel, Extended, or unexpected Handshake messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/libtortillas/src/frontend/publisher.rs`:
- Around line 124-131: The weak FrontendHub upgrade in FrontendPublisher::hub
can panic when TorrentActor accesses the frontend after hub shutdown. Update
TorrentActor’s frontend handling to use an Option<FrontendPublisher>, matching
PeerHandle and TrackerHandle, and guard startup and linked-child failure
frontend calls when the publisher is unavailable; alternatively provide
TorrentActor a strongly owned frontend handle or subscription.
---
Outside diff comments:
In `@crates/libtortillas/src/peer/actor.rs`:
- Around line 658-686: Move the frontend update out of the unconditional
post-match call in the peer message handler and invoke it only in the
state-changing arms: Choke, Unchoke, Interested, NotInterested, Have, Piece, and
Bitfield. Do not update the frontend for KeepAlive, Request, Cancel, Extended,
or unexpected Handshake messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c3bc4b5-9f92-4c6b-bec6-90fa5360b035
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
README.mdcrates/libtortillas/Cargo.tomlcrates/libtortillas/examples/live_frontend.rscrates/libtortillas/src/ARCHITECTURE.mdcrates/libtortillas/src/engine/messages.rscrates/libtortillas/src/engine/mod.rscrates/libtortillas/src/engine/snapshot.rscrates/libtortillas/src/errors.rscrates/libtortillas/src/facade.rscrates/libtortillas/src/frontend/event.rscrates/libtortillas/src/frontend/handle.rscrates/libtortillas/src/frontend/listener.rscrates/libtortillas/src/frontend/live.rscrates/libtortillas/src/frontend/mod.rscrates/libtortillas/src/frontend/publisher.rscrates/libtortillas/src/frontend/subscription.rscrates/libtortillas/src/frontend/view.rscrates/libtortillas/src/lib.rscrates/libtortillas/src/peer/actor.rscrates/libtortillas/src/torrent/actor.rscrates/libtortillas/src/torrent/handle.rscrates/libtortillas/src/torrent/messages.rscrates/libtortillas/src/torrent/mod.rscrates/libtortillas/src/torrent/snapshot.rscrates/libtortillas/src/torrent/storage.rscrates/libtortillas/src/torrent/swarm.rscrates/libtortillas/src/tracker/actor.rscrates/libtortillas/src/tracker/model.rscrates/libtortillas/tests/engine_lifecycle.rscrates/libtortillas/tests/facade.rscrates/libtortillas/tests/live_frontend.rscrates/libtortillas/tests/persistence.rsdocs/frontend-integration.md
🚧 Files skipped from review as they are similar to previous changes (7)
- crates/libtortillas/src/ARCHITECTURE.md
- crates/libtortillas/examples/live_frontend.rs
- crates/libtortillas/src/tracker/model.rs
- crates/libtortillas/src/lib.rs
- crates/libtortillas/tests/persistence.rs
- README.md
- crates/libtortillas/src/engine/messages.rs
WARNINGthis is one of the first fully vibe coded PRs on this repo, mostly a thought experiment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
crates/libtortillas/src/live/stream.rs (2)
76-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
EventSubscription::closed()here.Lines 79-84 duplicate the private constructor defined at Lines 222-227.
♻️ Proposed cleanup
let state = mutex_lock(&self.state); if state.closed { - return { - let (sender, receiver) = broadcast::channel(1); - let weak = sender.downgrade(); - drop(sender); - EventSubscription::from_receiver(receiver, weak) - }; + return EventSubscription::closed(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/live/stream.rs` around lines 76 - 92, Update the closed branch in subscribe to return the existing EventSubscription::closed() constructor instead of recreating the broadcast channel, downgrade, and receiver setup inline; leave the active subscription path unchanged.
129-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPublicly exported
LivePublishergives applications the write side of the live contract.
replace_view,emit_without_view_change, andclose_with_terminal_eventarepuband re-exported throughfacade.rs, so consumers can fabricate events or permanently close a stream they only obtained a handle to. If the intent is "observation only" for applications, consider making the mutation methodspub(crate)(or keeping the type crate-internal) and exposing only listeners/subscriptions publicly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/live/stream.rs` around lines 129 - 172, Restrict the mutation methods on the publicly exported LivePublisher so applications cannot alter or close live streams through an acquired handle. Change replace_view, replace_view_and_emit, emit_without_view_change, and close_with_terminal_event to crate-visible access (or make LivePublisher crate-internal), while preserving public listener/subscription APIs and internal call sites.crates/libtortillas/src/engine/messages.rs (1)
38-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
discard_restored_torrent— it now discards newly created torrents too.
GetLiveViewfailure (line 295-304) triggers this cleanup forCreateTorrentRequest::Newjust as much as for::Restore. The name implies restore-only scope, which could mislead future edits that add restore-specific behavior here.♻️ Suggested rename
- async fn discard_restored_torrent( + async fn discard_created_torrent( &mut self, info_hash: InfoHash, torrent: &ActorRef<TorrentActor>, ) {(and update the 4 call sites accordingly)
Also applies to: 238-258, 289-304
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/engine/messages.rs` around lines 38 - 53, Rename EngineActor::discard_restored_torrent to a creation/restoration-neutral name reflecting that it cleans up both newly created and restored torrents, then update all four call sites, including the GetLiveView failure path for CreateTorrentRequest::New. Preserve the existing unregister, graceful-stop, and scope-removal behavior.crates/libtortillas/src/torrent/actor.rs (2)
524-549: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTracker wire bytes are folded into the torrent's content transfer metrics.
metric_sourceschains tracker views into the same aggregate that producesmetrics.traffic.totals/rates, so announce overhead shows up as torrent download/upload speed and totals in every frontend (the test at Line 1926 encodes this:50_000peer +250tracker). Consider aggregating peers only forTransferMetrics, and exposing tracker overhead separately if it's needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/actor.rs` around lines 524 - 549, Update the metrics aggregation in the torrent metrics construction to use only peer sources for TransferMetrics totals and rates; remove trackers from the metric_sources chain used by TransferRates::aggregate and the totals fold. Preserve tracker metrics separately only if an existing dedicated field or API supports exposing their overhead.
220-232: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTransient storage errors permanently wedge the torrent.
pre_startfailures setTorrentState::Failed, andcan_start()only acceptsAdded | ResolvingMetadata | Ready | Paused, so a recoverable cause (missing mount, transient permission error) leaves no way to retrystart()after the user fixes it. Consider allowing recovery fromFailed(e.g. viaset_output_folder/set_piece_storageresetting toPaused) or documenting thatFailedis final for the session.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/actor.rs` around lines 220 - 232, Ensure recoverable pre_start failures do not permanently block restarting the torrent: update the failure recovery flow around TorrentState::Failed and can_start() so changing storage configuration through set_output_folder or set_piece_storage resets the torrent to Paused, or otherwise permits a subsequent start(). Preserve Failed for unrecoverable errors and keep the existing pre_start error reporting.crates/libtortillas/src/torrent/snapshot.rs (2)
182-202: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
block_mapduplicatepiece_indexentries pass validation.Nothing rejects two entries with the same
piece_index;restore_snapshotthen callsrestore_piece_blockstwice for that index and the last one silently wins. A cheap uniqueness check here keeps the validated type an actual invariant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/snapshot.rs` around lines 182 - 202, The block_map validation loop must reject duplicate piece_index values before accepting the snapshot. Update the validation surrounding block_map to track each validated index and return self.invalid(...) when an index appears more than once, while preserving the existing range, bitfield, metadata, and block-count checks.
318-342: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent
piece_lengtherror handling between storage arms.The
Disk+FileMetadataarm swallows the error withunwrap_or(usize::MAX)(making every piece look invalid), while theInFilearm propagates it with?. Sincepiece_lengthonly fails for a structurally broken snapshot, propagate in both places so the caller gets the typedInvalidSnapshotrather than a silent full demotion.♻️ Suggested change
RestoreVerification::FileMetadata => { + let expected = u64::try_from(piece_length(&info, index)?).unwrap_or(u64::MAX); fs::metadata(path).await.is_ok_and(|metadata| { - metadata.len() - >= u64::try_from(piece_length(&info, index).unwrap_or(usize::MAX)) - .unwrap_or(u64::MAX) + metadata.len() >= expected }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/snapshot.rs` around lines 318 - 342, Update the Disk branch’s RestoreVerification::FileMetadata handling to call piece_length with ? instead of converting errors via unwrap_or and u64::try_from fallbacks. Preserve the existing metadata length comparison while propagating the typed InvalidSnapshot error consistently with the InFile branch.crates/libtortillas/src/torrent/piece_flow.rs (1)
155-168: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
fill_peer_request_windowisn't state-gated, unlikefill_all_peer_request_windows.Callers such as
peer_rejected_request(torrent/messages.rs Line 98) andpiece_completedinvoke it without checkingstate == Downloading && is_ready(), so a torrent that was just paused can still emit new block requests. Consider moving the guard intofill_peer_request_window_toso every path shares it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/piece_flow.rs` around lines 155 - 168, Add the same state and readiness guard used by fill_all_peer_request_windows to fill_peer_request_window_to, returning without issuing requests unless the torrent is Downloading and is_ready(). Keep fill_peer_request_window and all callers unchanged so every request-window refill path enforces the guard centrally.crates/libtortillas/src/protocol/stream.rs (1)
234-249: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
split()panics instead of surfacing a recoverable error.The assertion turns a caller-sequencing mistake (any prior
PeerRecv::recv()leaves bytes inread_buffer) into a task abort in library code. Either returnResult<(PeerReader, PeerWriter), Self>or hand the buffered bytes toPeerReaderso the split is always safe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/protocol/stream.rs` around lines 234 - 249, Update PeerStream::split to avoid asserting on non-empty read_buffer and triggering a panic. Make split recoverable by returning Result<(PeerReader, PeerWriter), Self> and returning the original PeerStream when buffered data exists, or transfer the buffered data into PeerReader so splitting remains safe; preserve the existing transport-specific reader/writer construction for empty buffers.crates/libtortillas/src/pieces/piece_manager.rs (1)
127-128: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winApply the same safe conversion to the sibling
lengthcasts in this function.Good defensive fix for the multi-file
file.lengthconversion, butpiece_to_pathsstill has two rawas usizecasts of the same risk class: the single-file branch'slet file_len = *length as usize;(line 110) andlet piece_len = info.piece_length as usize;(line 86). On platforms whereusizeis narrower than the source type, these would silently truncate rather than error, producing an incorrect piece-to-file mapping instead of a clear failure.♻️ Suggested consistency fix
- let piece_len = info.piece_length as usize; + let piece_len = usize::try_from(info.piece_length) + .context("piece length cannot be represented on this platform")?; ... - let file_len = *length as usize; + let file_len = usize::try_from(*length) + .context("file length cannot be represented on this platform")?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/pieces/piece_manager.rs` around lines 127 - 128, In piece_to_paths, replace the raw usize casts for info.piece_length and the single-file length with checked usize::try_from conversions, adding clear context to each fallible conversion and propagating errors consistently with the existing file.length conversion. Preserve the existing piece-to-file mapping behavior when values fit.crates/libtortillas/src/torrent/swarm.rs (1)
170-179: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPublish
Updatedonce perbroadcast_to_peerscall, not per removed peer.The live-view publish now runs inside the
dead_peersloop, so a single call can emit N redundantUpdatedevents if several peers die together (e.g. a network blip). This churns the bounded broadcast channel and can trigger avoidableLaggedconditions for slow listeners, contrary to the "coalesced aggregate" design used elsewhere in this file.♻️ Proposed fix
+ let had_dead_peers = !dead_peers.is_empty(); for id in dead_peers { self.peers.remove(&id); - self.publish_live_view(|_| crate::live::TorrentEventKind::Updated); } + if had_dead_peers { + self.publish_live_view(|_| crate::live::TorrentEventKind::Updated); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/swarm.rs` around lines 170 - 179, Update the dead-peer cleanup in broadcast_to_peers so publish_live_view emits a single Updated event after all dead peers have been removed, rather than once inside the removal loop. Preserve cleanup for every entry in dead_peers and avoid publishing when no peers were removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/libtortillas/src/engine/actor.rs`:
- Line 21: Update EngineActor::on_start to publish a terminal failed engine
status through Hub before returning any TCP/uTP/UDP bind or optional DHT spawn
error. Reuse the existing Hub/LiveHealthLevel status-reporting API and preserve
the normal engine_started path on successful startup, ensuring failed startup
transitions consumers out of Starting even though on_stop is not invoked.
In `@crates/libtortillas/src/engine/messages.rs`:
- Around line 295-310: The restored-torrent initialization around GetLiveView
and Torrent::new_with_hub must avoid overwriting a newer view published between
the ask and registration. Change the initialization flow to atomically install
the fetched initial_view only when the torrent scope has no current live view,
or use an equivalent latest-writer-safe initialization mechanism before
register_torrent_scope re-emits the scope state.
In `@crates/libtortillas/src/live/handle.rs`:
- Around line 310-330: Ensure stopped tracker scopes are removed from the hub
before they can be re-registered: add and invoke a tracker-specific hub cleanup
operation from PeerHandle::stopped and close_without_parent_event, before
emitting any parent event, or otherwise make Hub::register_tracker_scope replace
an existing scope whose publisher is closed. Preserve current terminal-event
behavior while ensuring subsequent registration receives an active publisher.
In `@crates/libtortillas/src/live/hub.rs`:
- Around line 335-344: Update replace_torrent_view_and_emit so it applies the
new torrent view to the existing scope before checking torrent_handle. If no
handle is registered, return only after the scope view has been replaced;
otherwise continue with the existing engine fan-out and event emission path.
- Around line 239-246: Make HubReference::inner fallible instead of panicking
when a weak hub cannot be upgraded, returning None for an expired hub. Update
all publication paths that call inner() to return early when no HubInner is
available, matching LiveScope::hub()’s no-op behavior during shutdown while
preserving strong-reference behavior.
In `@crates/libtortillas/src/torrent/actor.rs`:
- Around line 871-876: Update on_link_died so the health event is emitted at
LiveHealthLevel::Error only for unexpected child termination; inspect the linked
child’s reason and suppress the health error for normal shutdown causes such as
idle disconnects or remote closes, while preserving the existing error log.
In `@crates/libtortillas/src/torrent/messages.rs`:
- Around line 195-216: Update set_piece_storage to reject switching away from
PieceStorageStrategy::Disk when a custom piece manager is installed, matching
the guard in set_piece_manager. Return the existing invalid-operation error
before creating directories or mutating piece_storage, while preserving normal
storage changes when no custom manager is active.
In `@crates/libtortillas/tests/dht_network.rs`:
- Line 52: Update the polling logic around listener.recv() to distinguish
EventStreamError::Closed from lag and timeout outcomes. Preserve continued
polling for lag or timeout results, but fail the test immediately when the
torrent event stream closes instead of discarding the result and waiting for
DOWNLOAD_TIMEOUT.
In `@crates/libtortillas/tests/persistence.rs`:
- Around line 447-460: Update
engine_snapshot_golden_fixtures_migrate_and_round_trip so the serialized
migrated snapshot is compared directly with the expected value parsed from
ENGINE_SNAPSHOT_V2. Remove the separately parsed current snapshot and serialize
migrated after validating its version, ensuring the test verifies the v1-to-v2
migration output.
---
Nitpick comments:
In `@crates/libtortillas/src/engine/messages.rs`:
- Around line 38-53: Rename EngineActor::discard_restored_torrent to a
creation/restoration-neutral name reflecting that it cleans up both newly
created and restored torrents, then update all four call sites, including the
GetLiveView failure path for CreateTorrentRequest::New. Preserve the existing
unregister, graceful-stop, and scope-removal behavior.
In `@crates/libtortillas/src/live/stream.rs`:
- Around line 76-92: Update the closed branch in subscribe to return the
existing EventSubscription::closed() constructor instead of recreating the
broadcast channel, downgrade, and receiver setup inline; leave the active
subscription path unchanged.
- Around line 129-172: Restrict the mutation methods on the publicly exported
LivePublisher so applications cannot alter or close live streams through an
acquired handle. Change replace_view, replace_view_and_emit,
emit_without_view_change, and close_with_terminal_event to crate-visible access
(or make LivePublisher crate-internal), while preserving public
listener/subscription APIs and internal call sites.
In `@crates/libtortillas/src/pieces/piece_manager.rs`:
- Around line 127-128: In piece_to_paths, replace the raw usize casts for
info.piece_length and the single-file length with checked usize::try_from
conversions, adding clear context to each fallible conversion and propagating
errors consistently with the existing file.length conversion. Preserve the
existing piece-to-file mapping behavior when values fit.
In `@crates/libtortillas/src/protocol/stream.rs`:
- Around line 234-249: Update PeerStream::split to avoid asserting on non-empty
read_buffer and triggering a panic. Make split recoverable by returning
Result<(PeerReader, PeerWriter), Self> and returning the original PeerStream
when buffered data exists, or transfer the buffered data into PeerReader so
splitting remains safe; preserve the existing transport-specific reader/writer
construction for empty buffers.
In `@crates/libtortillas/src/torrent/actor.rs`:
- Around line 524-549: Update the metrics aggregation in the torrent metrics
construction to use only peer sources for TransferMetrics totals and rates;
remove trackers from the metric_sources chain used by TransferRates::aggregate
and the totals fold. Preserve tracker metrics separately only if an existing
dedicated field or API supports exposing their overhead.
- Around line 220-232: Ensure recoverable pre_start failures do not permanently
block restarting the torrent: update the failure recovery flow around
TorrentState::Failed and can_start() so changing storage configuration through
set_output_folder or set_piece_storage resets the torrent to Paused, or
otherwise permits a subsequent start(). Preserve Failed for unrecoverable errors
and keep the existing pre_start error reporting.
In `@crates/libtortillas/src/torrent/piece_flow.rs`:
- Around line 155-168: Add the same state and readiness guard used by
fill_all_peer_request_windows to fill_peer_request_window_to, returning without
issuing requests unless the torrent is Downloading and is_ready(). Keep
fill_peer_request_window and all callers unchanged so every request-window
refill path enforces the guard centrally.
In `@crates/libtortillas/src/torrent/snapshot.rs`:
- Around line 182-202: The block_map validation loop must reject duplicate
piece_index values before accepting the snapshot. Update the validation
surrounding block_map to track each validated index and return self.invalid(...)
when an index appears more than once, while preserving the existing range,
bitfield, metadata, and block-count checks.
- Around line 318-342: Update the Disk branch’s
RestoreVerification::FileMetadata handling to call piece_length with ? instead
of converting errors via unwrap_or and u64::try_from fallbacks. Preserve the
existing metadata length comparison while propagating the typed InvalidSnapshot
error consistently with the InFile branch.
In `@crates/libtortillas/src/torrent/swarm.rs`:
- Around line 170-179: Update the dead-peer cleanup in broadcast_to_peers so
publish_live_view emits a single Updated event after all dead peers have been
removed, rather than once inside the removal loop. Preserve cleanup for every
entry in dead_peers and avoid publishing when no peers were removed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 67619d3d-e988-47d5-8c32-fa72c7c0e522
📒 Files selected for processing (51)
README.mdcrates/libtortillas/examples/live.rscrates/libtortillas/src/ARCHITECTURE.mdcrates/libtortillas/src/engine/actor.rscrates/libtortillas/src/engine/messages.rscrates/libtortillas/src/engine/mod.rscrates/libtortillas/src/engine/snapshot.rscrates/libtortillas/src/engine/source.rscrates/libtortillas/src/errors.rscrates/libtortillas/src/facade.rscrates/libtortillas/src/lib.rscrates/libtortillas/src/live/event.rscrates/libtortillas/src/live/handle.rscrates/libtortillas/src/live/hub.rscrates/libtortillas/src/live/mod.rscrates/libtortillas/src/live/stream.rscrates/libtortillas/src/live/view.rscrates/libtortillas/src/metainfo/file.rscrates/libtortillas/src/metrics.rscrates/libtortillas/src/peer/actor.rscrates/libtortillas/src/peer/mod.rscrates/libtortillas/src/peer/state.rscrates/libtortillas/src/pieces/piece_manager.rscrates/libtortillas/src/pieces/piece_scheduler.rscrates/libtortillas/src/protocol/messages.rscrates/libtortillas/src/protocol/stream.rscrates/libtortillas/src/settings.rscrates/libtortillas/src/torrent/actor.rscrates/libtortillas/src/torrent/choking.rscrates/libtortillas/src/torrent/choking_flow.rscrates/libtortillas/src/torrent/handle.rscrates/libtortillas/src/torrent/messages.rscrates/libtortillas/src/torrent/mod.rscrates/libtortillas/src/torrent/piece_flow.rscrates/libtortillas/src/torrent/snapshot.rscrates/libtortillas/src/torrent/state.rscrates/libtortillas/src/torrent/swarm.rscrates/libtortillas/src/tracker/actor.rscrates/libtortillas/src/tracker/http.rscrates/libtortillas/src/tracker/model.rscrates/libtortillas/src/tracker/stats.rscrates/libtortillas/src/tracker/udp.rscrates/libtortillas/tests/dht_network.rscrates/libtortillas/tests/engine_lifecycle.rscrates/libtortillas/tests/facade.rscrates/libtortillas/tests/fixtures/engine-snapshot-v1.jsoncrates/libtortillas/tests/fixtures/engine-snapshot-v2.jsoncrates/libtortillas/tests/fixtures/torrent-snapshot-v1.jsoncrates/libtortillas/tests/fixtures/torrent-snapshot-v2.jsoncrates/libtortillas/tests/live.rscrates/libtortillas/tests/persistence.rs
💤 Files with no reviewable changes (1)
- crates/libtortillas/src/ARCHITECTURE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/libtortillas/tests/facade.rs
- crates/libtortillas/src/tracker/model.rs
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/libtortillas/src/settings.rs (1)
159-161: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDocument that stale request expiry is ticked on rechoke.
release_stale_requestsonly runs insiderechoke_peers, and rechoke is scheduled atrechoke_interval(default 10s). Together with the 15speer_request_timeout, an unanswered request can remain unassignable for up to nearly 20s. Either expire requests independently or update the docs/test to clarify this polling granularity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/settings.rs` around lines 159 - 161, Update the documentation for peer_request_timeout to state that stale requests are expired during rechoke_peers polling, which runs at rechoke_interval, so effective reassignment may be delayed beyond the configured timeout. Add or adjust the relevant test to validate this documented polling granularity.crates/libtortillas/src/torrent/choking.rs (1)
141-178: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the choking tests behind
feature = "live"or provide legacyPeerStatshelpers.
PeerStatshas a livemetrics-based shape and a legacy{interested, choked, download_rate, upload_rate}shape, while this module still constructs the live shape unconditionally. Without--features live, this test module and its callsites fail to compile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/choking.rs` around lines 141 - 178, Make the choking test helpers stats and with_rates compatible with the non-live build by gating the relevant tests behind feature = "live" or providing equivalent legacy PeerStats construction for builds without that feature. Ensure all helper callsites, including selector_only_includes_interested_peers, compile under both configurations while preserving their existing test behavior.
🧹 Nitpick comments (1)
crates/libtortillas/src/engine/mod.rs (1)
314-323: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThree copies of the same
let _ = &torrent_ref;unused-binding workaround.Consider binding the ask result as
_torrent_ref(or gating the wholeletwith#[cfg(not(feature = "live"))]) so the suppression line disappears in all three methods.Also applies to: 358-367, 434-443
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/engine/mod.rs` around lines 314 - 323, Update the three affected methods to bind the ask result as _torrent_ref, or conditionally bind torrent_ref only for non-live builds, so the repeated let _ = &torrent_ref workaround is removed. Preserve the existing live torrent_handle and non-live Torrent::new behavior in each method.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/libtortillas/src/live/hub.rs`:
- Around line 565-580: Update mark_peer_disconnected to remove the peer via
ScopeRegistry::remove_value using handle pointer identity, so a stale PeerHandle
cannot remove a replacement registered under the same PeerId. Also update the
membership check in emit_peer_connected to use the same ptr-identity value
check, preserving events and live membership for the currently registered
handle.
In `@crates/libtortillas/src/pieces/piece_manager.rs`:
- Around line 86-87: The scheduler currently uses an unchecked
`Info::piece_length` conversion before request generation, while `PieceManager`
validates it later. Convert `piece_length` with a checked `usize::try_from` at
the `PieceScheduler` boundary, propagate that validated value through
`requests_for_peer` and the `piece_flow` request-building path, and reuse it for
storage validation instead of recasting the original value.
---
Outside diff comments:
In `@crates/libtortillas/src/settings.rs`:
- Around line 159-161: Update the documentation for peer_request_timeout to
state that stale requests are expired during rechoke_peers polling, which runs
at rechoke_interval, so effective reassignment may be delayed beyond the
configured timeout. Add or adjust the relevant test to validate this documented
polling granularity.
In `@crates/libtortillas/src/torrent/choking.rs`:
- Around line 141-178: Make the choking test helpers stats and with_rates
compatible with the non-live build by gating the relevant tests behind feature =
"live" or providing equivalent legacy PeerStats construction for builds without
that feature. Ensure all helper callsites, including
selector_only_includes_interested_peers, compile under both configurations while
preserving their existing test behavior.
---
Nitpick comments:
In `@crates/libtortillas/src/engine/mod.rs`:
- Around line 314-323: Update the three affected methods to bind the ask result
as _torrent_ref, or conditionally bind torrent_ref only for non-live builds, so
the repeated let _ = &torrent_ref workaround is removed. Preserve the existing
live torrent_handle and non-live Torrent::new behavior in each method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6b8050e-e921-4096-8643-c88c0f7194ce
📒 Files selected for processing (37)
.github/workflows/checks.ymlREADME.mdcrates/libtortillas/Cargo.tomlcrates/libtortillas/src/engine/actor.rscrates/libtortillas/src/engine/messages.rscrates/libtortillas/src/engine/mod.rscrates/libtortillas/src/engine/snapshot.rscrates/libtortillas/src/facade.rscrates/libtortillas/src/lib.rscrates/libtortillas/src/live/event.rscrates/libtortillas/src/live/handle.rscrates/libtortillas/src/live/hub.rscrates/libtortillas/src/live/mod.rscrates/libtortillas/src/live/stream.rscrates/libtortillas/src/live/view.rscrates/libtortillas/src/metrics.rscrates/libtortillas/src/peer/actor.rscrates/libtortillas/src/peer/state.rscrates/libtortillas/src/pieces/piece_manager.rscrates/libtortillas/src/protocol/stream.rscrates/libtortillas/src/settings.rscrates/libtortillas/src/torrent/actor.rscrates/libtortillas/src/torrent/choking.rscrates/libtortillas/src/torrent/choking_flow.rscrates/libtortillas/src/torrent/handle.rscrates/libtortillas/src/torrent/messages.rscrates/libtortillas/src/torrent/mod.rscrates/libtortillas/src/torrent/piece_flow.rscrates/libtortillas/src/torrent/snapshot.rscrates/libtortillas/src/torrent/swarm.rscrates/libtortillas/src/tracker/actor.rscrates/libtortillas/src/tracker/model.rscrates/libtortillas/src/tracker/stats.rscrates/libtortillas/tests/dht_network.rscrates/libtortillas/tests/facade.rscrates/libtortillas/tests/live.rscrates/libtortillas/tests/persistence.rs
🚧 Files skipped from review as they are similar to previous changes (26)
- crates/libtortillas/src/torrent/mod.rs
- crates/libtortillas/src/lib.rs
- crates/libtortillas/src/live/event.rs
- crates/libtortillas/tests/dht_network.rs
- crates/libtortillas/src/engine/actor.rs
- crates/libtortillas/src/tracker/model.rs
- README.md
- crates/libtortillas/src/facade.rs
- crates/libtortillas/src/torrent/choking_flow.rs
- crates/libtortillas/src/live/mod.rs
- crates/libtortillas/src/live/handle.rs
- crates/libtortillas/src/engine/snapshot.rs
- crates/libtortillas/src/protocol/stream.rs
- crates/libtortillas/src/live/stream.rs
- crates/libtortillas/src/tracker/actor.rs
- crates/libtortillas/src/torrent/swarm.rs
- crates/libtortillas/src/engine/messages.rs
- crates/libtortillas/tests/persistence.rs
- crates/libtortillas/src/torrent/snapshot.rs
- crates/libtortillas/src/torrent/messages.rs
- crates/libtortillas/src/live/view.rs
- crates/libtortillas/src/peer/actor.rs
- crates/libtortillas/src/peer/state.rs
- crates/libtortillas/src/torrent/handle.rs
- crates/libtortillas/src/torrent/piece_flow.rs
- crates/libtortillas/src/torrent/actor.rs
| pub(crate) fn mark_peer_disconnected(&self, peer: &PeerHandle) { | ||
| let Some(inner) = self.inner() else { | ||
| return; | ||
| }; | ||
| let Some(scope) = inner.torrents.get(&peer.torrent()) else { | ||
| return; | ||
| }; | ||
| if scope.peers.remove(&peer.id()).is_none() { | ||
| return; | ||
| } | ||
| Self::emit_without_torrent_view_change( | ||
| &inner, | ||
| &scope, | ||
| TorrentEventKind::PeerDisconnected(peer.clone()), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Peer removal keys on PeerId, so a stale handle can evict its replacement.
If the same PeerId reconnects (new PeerHandle inserted over the old key in register_peer_scope) and the old handle's disconnected() fires afterwards, scope.peers.remove(&peer.id()) drops the new registration and emits PeerDisconnected for the stale handle — the live view then loses an actually-connected peer. ScopeRegistry::remove_value already provides ptr-identity removal and is used for trackers; the same treatment fits here (and for the membership check in emit_peer_connected).
🔧 Suggested change
- if scope.peers.remove(&peer.id()).is_none() {
+ if !scope.peers.remove_value(&peer.inner) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub(crate) fn mark_peer_disconnected(&self, peer: &PeerHandle) { | |
| let Some(inner) = self.inner() else { | |
| return; | |
| }; | |
| let Some(scope) = inner.torrents.get(&peer.torrent()) else { | |
| return; | |
| }; | |
| if scope.peers.remove(&peer.id()).is_none() { | |
| return; | |
| } | |
| Self::emit_without_torrent_view_change( | |
| &inner, | |
| &scope, | |
| TorrentEventKind::PeerDisconnected(peer.clone()), | |
| ); | |
| } | |
| pub(crate) fn mark_peer_disconnected(&self, peer: &PeerHandle) { | |
| let Some(inner) = self.inner() else { | |
| return; | |
| }; | |
| let Some(scope) = inner.torrents.get(&peer.torrent()) else { | |
| return; | |
| }; | |
| if !scope.peers.remove_value(&peer.inner) { | |
| return; | |
| } | |
| Self::emit_without_torrent_view_change( | |
| &inner, | |
| &scope, | |
| TorrentEventKind::PeerDisconnected(peer.clone()), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/libtortillas/src/live/hub.rs` around lines 565 - 580, Update
mark_peer_disconnected to remove the peer via ScopeRegistry::remove_value using
handle pointer identity, so a stale PeerHandle cannot remove a replacement
registered under the same PeerId. Also update the membership check in
emit_peer_connected to use the same ptr-identity value check, preserving events
and live membership for the currently registered handle.
| let piece_len = usize::try_from(info.piece_length) | ||
| .context("piece length cannot be represented on this platform")?; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'piece_length\s+as\s+usize|file\.length\s+as\s+usize|as\s+usize' \
crates/libtortillas/src/torrent/piece_flow.rs \
crates/libtortillas/src/piecesRepository: artrixdotdev/tortillas
Length of output: 1280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline piece_flow.rs =="
ast-grep outline crates/libtortillas/src/torrent/piece_flow.rs --view expanded | sed -n '1,220p' || true
echo "== outline piece_manager.rs =="
ast-grep outline crates/libtortillas/src/pieces/piece_manager.rs --view expanded | sed -n '1,260p' || true
echo "== relevant piece_flow slices =="
sed -n '1,80p' crates/libtortillas/src/torrent/piece_flow.rs
sed -n '150,230p' crates/libtortillas/src/torrent/piece_flow.rs
echo "== relevant piece_manager slices =="
sed -n '60,110p' crates/libtortillas/src/pieces/piece_manager.rs
echo "== request scheduling references =="
rg -n 'requests_for_peer|piece_to_paths|piece_length|total_length|piece_count|Path' crates/libtortillas/src/torrent crates/libtortillas/src/piecesRepository: artrixdotdev/tortillas
Length of output: 28883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Behavioral probe for integer coercion semantics under the relevant casts:
# piece_length: u32 -> usize (platform usize)
for wasm32_usize_bits in (32,):
for piece_length_bits in (32, 40, 60, 64):
piece_length = 1 << (piece_length_bits - 1)
usize_value = piece_length & ((1 << wasm32_usize_bits) - 1)
checked = piece_length if piece_length < (1 << wasm32_usize_bits) else "overflow"
print(f"wasm32 piece_length_bits={piece_length_bits}: original={piece_length:`#x`} cast_usize={usize_value:`#x`} checked={checked}")
print("on_u8_target")
u8_value = 256 & 255
checked = "overflow"
print(f"u8=256: original=256 cast_u8={u8_value} checked={checked}")
PYRepository: artrixdotdev/tortillas
Length of output: 571
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== piece_scheduler.rs outline and length math =="
sed -n '100,260p' crates/libtortillas/src/pieces/piece_scheduler.rs
echo "== metainfo info types =="
rg -n -C4 'pub struct Info|struct Info|piece_length|total_length|piece_count|file:' crates/libtortillas/src -g '*.rs'Repository: artrixdotdev/tortillas
Length of output: 50378
Validate piece_length before scheduling requests.
file_length casts Info::piece_length to usize at crates/libtortillas/src/torrent/piece_flow.rs:190, which truncates values that do not fit on 32-bit targets. PieceScheduler::requests_for_peer then builds block ranges from that width, while storage validation rejects the overflow later in piece_manager.rs:86. Convert once at the scheduler boundary and propagate the checked value into request generation/storage validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/libtortillas/src/pieces/piece_manager.rs` around lines 86 - 87, The
scheduler currently uses an unchecked `Info::piece_length` conversion before
request generation, while `PieceManager` validates it later. Convert
`piece_length` with a checked `usize::try_from` at the `PieceScheduler`
boundary, propagate that validated value through `requests_for_peer` and the
`piece_flow` request-building path, and reuse it for storage validation instead
of recasting the original value.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/libtortillas/src/torrent/choking.rs (1)
69-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid cloning full
PeerStatsduring selection.
cloned()copies each interested peer, including live transfer sample history, on every rechoke. Collect borrowed references (or a lightweight(PeerId, rate)projection) instead; the selector only sorts peers and extracts IDs.Suggested change
let mut candidates: Vec<_> = peers .iter() .filter(|peer| peer.interested) - .cloned() .collect();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/libtortillas/src/torrent/choking.rs` around lines 69 - 73, Update the candidate collection in the rechoke selection flow to avoid cloning full PeerStats: collect references to interested peers, or project only the PeerId and rate fields needed by the sorting and ID-extraction logic. Preserve the existing filtering, ordering, and selection behavior while removing the per-peer cloned() allocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/libtortillas/src/torrent/choking.rs`:
- Around line 69-73: Update the candidate collection in the rechoke selection
flow to avoid cloning full PeerStats: collect references to interested peers, or
project only the PeerId and rate fields needed by the sorting and ID-extraction
logic. Preserve the existing filtering, ordering, and selection behavior while
removing the per-peer cloned() allocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 974c4d2e-50a0-4be5-91ce-367f4c628f11
📒 Files selected for processing (25)
crates/libtortillas/src/engine/actor.rscrates/libtortillas/src/engine/messages.rscrates/libtortillas/src/engine/mod.rscrates/libtortillas/src/facade.rscrates/libtortillas/src/lib.rscrates/libtortillas/src/live/event.rscrates/libtortillas/src/live/mod.rscrates/libtortillas/src/live/stream.rscrates/libtortillas/src/live/view.rscrates/libtortillas/src/peer/actor.rscrates/libtortillas/src/peer/state.rscrates/libtortillas/src/pieces/piece_scheduler.rscrates/libtortillas/src/protocol/stream.rscrates/libtortillas/src/torrent/actor.rscrates/libtortillas/src/torrent/choking.rscrates/libtortillas/src/torrent/choking_flow.rscrates/libtortillas/src/torrent/handle.rscrates/libtortillas/src/torrent/messages.rscrates/libtortillas/src/torrent/mod.rscrates/libtortillas/src/torrent/piece_flow.rscrates/libtortillas/src/torrent/swarm.rscrates/libtortillas/src/tracker/actor.rscrates/libtortillas/src/tracker/http.rscrates/libtortillas/src/tracker/model.rscrates/libtortillas/src/tracker/stats.rs
💤 Files with no reviewable changes (1)
- crates/libtortillas/src/live/event.rs
🚧 Files skipped from review as they are similar to previous changes (23)
- crates/libtortillas/src/tracker/model.rs
- crates/libtortillas/src/torrent/choking_flow.rs
- crates/libtortillas/src/tracker/http.rs
- crates/libtortillas/src/engine/actor.rs
- crates/libtortillas/src/torrent/mod.rs
- crates/libtortillas/src/live/view.rs
- crates/libtortillas/src/live/mod.rs
- crates/libtortillas/src/lib.rs
- crates/libtortillas/src/torrent/handle.rs
- crates/libtortillas/src/torrent/swarm.rs
- crates/libtortillas/src/pieces/piece_scheduler.rs
- crates/libtortillas/src/tracker/actor.rs
- crates/libtortillas/src/live/stream.rs
- crates/libtortillas/src/peer/actor.rs
- crates/libtortillas/src/protocol/stream.rs
- crates/libtortillas/src/engine/messages.rs
- crates/libtortillas/src/peer/state.rs
- crates/libtortillas/src/tracker/stats.rs
- crates/libtortillas/src/torrent/messages.rs
- crates/libtortillas/src/torrent/piece_flow.rs
- crates/libtortillas/src/engine/mod.rs
- crates/libtortillas/src/torrent/actor.rs
- crates/libtortillas/src/facade.rs
|
Fully reviewed all the code, was pretty in the loop for this so i dont feel to bad. Looks good though |
1 participant
Summary
LivePublisher<V, E>,EventListener<V, E>, andEventSubscription<E>primitives withfutures::Streamand Tokio broadcast semanticsEngine,Torrent,PeerHandle, andTrackerHandleTorrentEventKindhierarchy through engine events instead of maintaining duplicate engine/torrent/peer/tracker vocabulariesTorrent,PeerHandle, andTrackerHandlevalues so applications can descend into scoped detail only when neededEngineandTorrentmethods as the sole command API; no duplicate command enums or genericsendmethodsAPI boundary
Live rendering uses listeners, subscriptions, and coherent current views. Each scope owns a bounded publisher; lagging consumers receive
EventStreamError::Lagged, redraw fromlistener.view(), and continue. Terminal scope events close their stream, and late actor updates are rejected.Snapshots are not display models. They contain the complete resumable torrent state, while the application chooses the Serde format and storage location. Engine restore validates the complete snapshot and applies it as one authoritative actor operation.
Internal actor messages remain private. Public operations use direct methods such as
engine.add_torrent(...),engine.remove_torrent(...),torrent.start(), andtorrent.pause().Issue coverage
Closes #162
Closes #170
Closes #211
Closes #227
Closes #239
Closes #240
Advances #230, #231, and #238.
Part of #221.
Validation
cargo fmt --all -- --checkcargo nextest run --workspace --all-features— 143 passed, 8 skippedcargo clippy --workspace --all-targets --all-features -- -D warningsRUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-depsSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests