[jacderida]

Linear issue

Closes V2-1872

Risk tier

• T0 — docs / tooling / CI / pure UX-output. Repo CI only.
• T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
• T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
• T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

The visible change is progress text, but it reaches it by threading a new
return value through the client's chunk-fetch path, so it is more than a pure
output change. No request, response or stored byte changes.

Compatibility

• Wire: none. No request or response shape changes; the GETs sent are the same.
• Storage: none.
• API: no ant-core public signature changes — chunk_get,
  chunk_get_from_closest_peers, chunk_get_observed and
  chunk_get_observed_from_closest_peers keep their current types. The
  browser's downloadPublicFile/downloadPrivateFile progress strings gain a
  suffix; the Downloaded chunk n/total prefix is unchanged, so the startup
  probes (browser-tests/read-startup.mjs, browser-tests/sdk-startup.mjs)
  that match on it still work.

Semver impact

• breaking
• feature
• fix

Test evidence

Gates run locally:

• cargo fmt --all -- --check — pass.
• cargo check -p ant-core --target wasm32-unknown-unknown --no-default-features --features browser-wasm — pass.
• cargo clippy -p ant-core --target wasm32-unknown-unknown --no-default-features --features browser-wasm -- -D warnings — pass.
• cargo clippy --all-targets --all-features -- -D warnings — left to CI (the
  full-feature build had not finished locally when this was opened); the
  lint job covers it.

New and updated tests, run by the browser-wasm job:

• ant-core/wasm-tests/large-download.test.mjs — the in-memory download test
  now asserts each delivered chunk reports a 64-character lowercase peer hex
  belonging to one of the mock nodes and after 1 attempts, with the first
  line still the bare Downloaded chunk 0/3.
• A new case asserts a second download of the same file reports
  from cache after 0 attempts for every chunk, and that it re-requested none
  of the file's data records.
• chunk_get_outcome's unit test covers the generic signature, including the
  chunk-plus-source pair the browser path carries.

New dependency

none

ADR

n/a (Tier 1). docs/adr/ADR-0004-direct-browser-read-client.md is updated
where it documents the progress line.

Mitigation / rollback

Revert the commit. The change is additive text on one progress line plus an
internal return value; nothing persists it and no peer sees it.

🤖 Generated with Claude Code

[MaidSafe-Hermes]

REQUEST_CHANGES — 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The seats differed: sol-1 identified the retry-accounting defect; glm-1 approved but also noted deferred rounds reset the count. I verified the defect in the frozen source.

• ant-core/src/data/client/chunk.rs:1186-1194 reports only the successful chunk_get call's local count. The browser download path retries failed records in later deferred rounds (ant-core/src/client_engine/files.rs:396-434), and each retry calls fetch_data_record_with_source again, which starts a fresh chunk_get count. Thus if one or more GETs fail and a later retry succeeds, progress reports only the final round's attempts, not all GETs spent fetching that chunk. This contradicts the documented “GETs it cost” claim and can hide costly retries.

Please carry the attempt total across deferred rounds (and preserve it through failed fetch results) and add a browser-WASM test where an initial GET fails and a later round succeeds, asserting the reported total. This is a source-confirmed defect, not a CI-only concern.

CI at review time also has failures (Clippy, Documentation, Security Audit, Unit Tests on Ubuntu, E2E on Ubuntu, Build on Ubuntu, and Merkle E2E on Ubuntu); several other jobs remain in progress. The run is still active, so its failed-job logs are not yet available. These checks are separate from the code finding and need diagnosis before merge.

[jacderida]

Confirmed and fixed in 1b64f1f — the finding was right. deferred_batch retries a
not-found record in rounds [0, 0, 15, 45] (client_engine/files.rs), each round calls
fetch_data_record_with_source again, and each of those is a fresh chunk_get with its own
counter, so a chunk whose first round found nothing reported only its last round's GETs. That
hid precisely the costly retries the line exists to surface.

To be explicit about scope: counting per chunk_get call was what V2-1872 specified, and its
agent notes rejected threading the round number because that needs a files.rs signature change
touching every native caller. Your point stands regardless — the line claims "the GETs it cost",
so it has to be the total. The fix gets there without touching files.rs.

Carrying the total across rounds. The awkward part is that a round which finds nothing
returns an error and so carries no ChunkSource to report through.
chunk_get_observed_with_source now takes a dispatched: &AtomicUsize that
chunk_get_from_closest_peers_with_diagnostics raises as it sends each peer GET, and it is added
before the error path returns:

.await;
// Report the GETs before the error path returns: a round that found
// nothing still cost them, and a retrying caller needs them counted.
let attempts = attempts.load(Ordering::Relaxed);
if let Some(dispatched) = dispatched {
    dispatched.fetch_add(attempts, Ordering::Relaxed);
}
let result = outcome?;

data_download_indexed_chunks keeps a per-record running total across rounds and reports it,
taking a successful round's count from the returned source and a failed round's from the counter:

let round = match &fetched {
    Ok((_, source)) => source.attempts,
    Err(_) => dispatched.load(Ordering::Relaxed),
};

The split matters for single-flight: a joiner sends no GET of its own, so its dispatched stays
0, but the leader's count reaches it in the cloned ChunkSource — hence the source on success and
the counter only on failure. It also avoids double counting, since for a leader the two are equal.
An AtomicUsize rather than a Cell because the native data_download futures are asserted
Send. Native callers pass None and are unaffected.

Test. ant-core/wasm-tests/large-download.test.mjs now has "a chunk retried in a later round
reports every GET it cost". It withholds one data record from the mock's store and supplies it only
once two GETs for it have been answered — the mock builds the response before the respond hook
runs, so the record cannot be found until the GET after that. One chunk_get cannot outlive two
NotFounds from the only peer, so the record is necessarily found in a later round. The assertion is
that the retried chunk reports every GET the mock actually served for that address, which makes it
self-validating rather than pinned to an internal retry count.

Run locally against the built bindings, instrumented to show the numbers:

PROBE served=3 attempts=[1,1,3] order=[".","W",".",".","W","W"]
✔ a chunk retried in a later round reports every GET it cost (1035.843741ms)
ℹ tests 15  ℹ pass 15  ℹ fail 0

Three GETs for the withheld record, reported as 3. The succeeding round sent one of them, so the
previous code reported 1 and this test fails against it. The whole large-download suite passes.

On the CI failures you saw — they were all one cause, now fixed in ebcf49b: the workflow sets
RUSTFLAGS: "-D warnings" globally, and ChunkSource's fields are read only on the browser path,
so the native build failed on fields 'peer' and 'attempts' are never read. That is why Clippy,
Documentation, Build, Unit Tests and the E2E jobs all died at the ~3-minute ant-core compile while
the wasm-only gates passed. The struct now carries
#[cfg_attr(not(all(feature = "browser-wasm", target_arch = "wasm32")), allow(dead_code))], and
Clippy, Documentation, Build, Unit Tests (both OS), Browser WASM and Real Chromium all passed on
that commit before this push.

Security Audit is not from this branch — it fails on #248 as well, so it is a new advisory against
main.

🤖 Generated with Claude Code

[MaidSafe-Hermes]

REQUEST_CHANGES — 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). Both seats confirm the original retry-round omission is fixed. They differ on a remaining concurrency case; I verified sol-1's finding against the frozen source.

• ant-core/src/data/client/chunk.rs:549-563 returns a shared fetch result to joiners, but only the leader's dispatched counter is updated (:541-547 passes that counter into the fetch closure). When the shared fetch fails, the result carries no ChunkSource, so a joiner's counter stays zero. Then ant-core/src/data/client/data.rs:518-525 uses that zero as the failed round's cost. With two concurrent browser downloads of the same uncached chunk, a failed shared GET round is therefore counted for the leader but omitted from the follower's later progress total; the successful retry count is included. This still underreports “every GET it cost” under supported single-flight sharing.

Please propagate failed-round attempt metadata through the shared unsuccessful result as well, and add a test with two concurrent downloads sharing a failed round before success; assert that both progress callbacks report the same cumulative count. The new single-download retry test does not exercise this joiner path.

CI: the browser-WASM tests, Clippy, unit tests, documentation and Ubuntu build have passed. The Security Audit fails on crypto-bigint 0.5.5 (RUSTSEC-2026-0387); the base and PR have identical Cargo.lock hashes, so this appears inherited, but the required check still needs maintainer disposition. E2E, Merkle E2E and real Chromium checks were still running at this review snapshot.

[jacderida]

Confirmed and fixed in 369ddba. sol-1 is right: I had documented the joiner's zero as correct
("it dispatched none"), and that reasoning was wrong. What the line reports is what the chunk
cost, and a shared GET cost the follower exactly what it cost the leader. A failed shared round
was therefore counted once and silently free for everyone else sharing it.

The fix. The count now rides in the shared result rather than in a counter only the leader
held:

/// What one shared `chunk_get` resolves to: its outcome, and the number of
/// peer GETs the fetch dispatched. Both reach a joiner, so sharing a fetch
/// costs a caller the same as leading it — including a round that found
/// nothing, whose outcome is an error and so carries no chunk to report with.
pub(crate) type SharedChunkGet = (Result<Option<SourcedChunk>>, usize);

The single-flight map holds that pair, and chunk_get_shared charges whichever caller it is
serving before handing back the outcome:

let (outcome, sent) = match flown {
    Flown::Led { value, observe } => { /* observe value.0 */ value }
    Flown::Joined(value) => value,
};
if let Some(dispatched) = dispatched {
    dispatched.fetch_add(sent, Ordering::Relaxed);
}
outcome

It also removes the split that made the bug possible. Previously the download path took a
successful round's count from the returned ChunkSource and a failed round's from the counter —
two sources for one number, and the joiner fell through the gap between them. Now chunk_get
reports only the serving peer (SourcedChunk is (DataChunk, Option<PeerId>)) and leaves all
counting to the counter, so data_download_indexed_chunks accumulates one number from one place:

let total = { *spent.entry(address).or_default() += dispatched.load(Relaxed); ... };
let (bytes, peer) = fetched?;
sources.borrow_mut().insert(address, ChunkSource { peer, attempts: total });

A side benefit: ChunkSource is now built where it is reported, so it can be browser-only and the
allow(dead_code) the native build needed is gone.

Test — "downloads sharing a failed round are both charged for it". Two concurrent
downloadPublicFile calls on one client, one record withheld for its first two GETs, and every
get_chunk response held open 60 ms so the second download reliably joins the first's fetch rather
than racing past it. Instrumented, five consecutive runs, identical every time:

PROBE served=3 reported=[[1,1,3],[1,1,3]] gets=6

Three GETs for the withheld record, both downloads reporting all three. Before this change the
joiner's failed round contributed 0 and its successful round took the leader's single-call count,
so it reported [1,1,1] — both reported[0].at(-1) === served and the equality assertion fail
against the old code, whichever download won the lead.

One note on the suggested assertion: "both report the same cumulative count" holds only when the
two downloads share every round, so the test first asserts the withheld record was served more GETs
than one round can spend and that the leading download's total equals that figure. If sharing broke
down the GET count would rise and that assertion would fail loudly rather than the equality
quietly passing on two equal-but-wrong numbers.

Full large-download suite: 16/16 pass locally against freshly built bindings. cargo fmt,
native cargo check -p ant-core --all-features --all-targets and the wasm clippy -D warnings
gate are all clean.

Security Audit: agreed it is inherited — RUSTSEC-2026-0387 against crypto-bigint 0.5.5,
identical Cargo.lock on base and PR, and it fails on #248 too. Nothing in this branch touches
dependencies, so it needs a maintainer call rather than a change here.

🤖 Generated with Claude Code

[MaidSafe-Hermes]

APPROVE — 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The original retry-round accounting issue and the shared-fetch joiner case are fixed in this head. The new single-download and concurrent-download retry tests passed in the Browser WASM CI job; the real Chromium/local-node job also passed.

GLM-1 returned APPROVE. Sol-1 returned UNCERTAIN because it could not confirm test determinism from the supplied evidence. I checked the mock's synchronous response ordering and the current-head Browser WASM result; I find no unresolved code objection.

CI is not all green: E2E and Merkle E2E jobs remain in progress. Security Audit fails on crypto-bigint 0.5.5 (RUSTSEC-2026-0387); base and PR have the same Cargo.lock object. Per the WithAutonomi organization owner's direction, I treat these unrelated/inherited checks as non-blocking for this browser-only review. This approval does not authorize merge or release.