[tahodev]

Closes #153.

Adds a persistent "N completed downloads" line to the send-side progress display, and prints the same count as a short summary on shutdown. Each completed request bumps the count, so it's easy to see at a glance whether anything has been fully served.

One thing to be aware of: a single sendme receive of one file shows up as two completed downloads, since it issues both a hash seq and a blob request. I kept the counting at the request level since that's what the progress UI already shows per request.

Adds a CLI test covering a full send/receive round trip and the shutdown summary. cargo fmt, cargo clippy and the full test suite pass locally.

Details

Prepared with AI assistance (various models); implementation, tests and this description reviewed by me.

[ramfox]

Hey! Thank you for writing this up. I have been looking at it between work deliverables since you pushed it, sorry I haven't gotten back to you sooner.

The code is sound! The problem is I think it's too strange for folks who are unaware of sendme's internals (and the nuances of iroh-blobs) to understand that "2 Completed Downloads" is the "normal" thing to get when you only are trying to send something to one person.

Since we can't distinguish between requests that just ask for the size of the content and the request to download the content itself, I think the right place to be tracking the number of "transfers" is around the connections. The sendme paradigm expects that one transfer of content data happens per connection, since we close after the content request.

For this we would need to instead keep some state once a connection opens, where we tally:

• the number of requests made
• the number of requests completed
• if the connection has been closed (in ProviderMessage::ConnectionClosed)

However, we are in a bit of a racy situation: the per_request_progress method is spawned on a detached task separate from the show_provide_progress loop that lets us know a connection has been closed w/ the ProviderMessage::ConnectionClosed, so we could actually get a ConnectionClosed message before we find out that the request is completed in the per_request_progress function.

So we would probably have to do something like this in the show_provider_progress function:

// PSEUDOCODE
    ...
    // loop in show_provider_progress
    loop {
        tokio::select! {
            biased;
            item = recv.recv() => {
                let Some(item) = item else {
                    break;
                };

                trace!("got event {item:?}");
                match item {
                    ProviderMessage::ClientConnectedNotify(msg) => {
                        let endpoint_id = msg.endpoint_id.map(|id| id.fmt_short().to_string()).unwrap_or_else(|| "?".to_string());
                        let connection_id = msg.connection_id;
                        // START TRACKING THE TRANSFER HERE
                        );
                        connections.lock().unwrap().insert(
                              ...
                        );
                    }
                    ProviderMessage::ConnectionClosed(msg) => {
                        if let Some(connection) = connections.lock().unwrap().remove(&msg.connection_id) {
                        ....
                        }
                        // UPDATE TRANSFER INFO TO SAY THIS CONNECTION CLOSED
                        // This method checks the transfer state struct and sees if we can call this transfer closed now
                        try_finalize_connection(...);
                    }
                    ProviderMessage::GetRequestReceivedNotify(msg) => {
                        let request_id = msg.request_id;
                        let connection_id = msg.connection_id;
                        let connections = connections.clone();
                        let mp = mp.clone();
                        // INCREMENT THE REQUEST COUNTER FOR THIS TRANSFER
                        // per_request_progress now returns the outcome of the request, whether the request
                        // was aborted or completed
                        tasks.push(per_request_progress(mp, connection_id, request_id, connections, msg.rx));
                    }
                    _ => {}
                }
            }
            // new place to run the tasks to completion, use it to collect the outcomes of each request
            Some((connection_id, outcome)) = tasks.next(), if !tasks.is_empty() => {
                // some new function that updates the transfer state to add the outcome of the request from `per_request_progress`
                record_request_outcome(...);
            }
        }
    }
     // still need to drain the rest of the tasks, this is the same as what's on main, we just now need to handle
     // the outcomes from `per_request_progress`
    while let Some((connection_id, outcome)) = tasks.next().await {
        record_request_outcome(...);
    }
    Ok(())

The last weird bit would be that since that last loop over the tasks means that we could be handling the outcome of requests after all of the connections have ended, we also need to try_finalized_connection inside record_request_outcome

I would also change the language to say "X Transfers Completed," rather than "downloads."

OOOooooookay. Was that a terrible way to explain where things should probably go?

Are you still interested in working on this 😂 ?

Let me know if you see/experience where this won't work.

[ramfox]

Great test, but we should also add the --relay disabled flag here and for the receive side, likely. This removes a "there is no internet where I am running this local test" issue and also removes some flakey-ness that can happen when folks are running tests in low latency places.