Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
in reviewhumanPR #95Other

metrics: fix delivery_rate/success_rate exceeding 1.0 on broadcast-heavy scenarios

Fixes delivery_rate (and its success_rate alias) returning a value greater than 1.0 on any scenario that uses broadcasts. A delivery "rate" above 1.0 is impossible and misreports every broadcast-heavy run.

Author

SwasthikaDev avatar

@SwasthikaDev

github profile →
Status
In review
Opened on
Jul 8
Branch
hackathon/swasthika-delivery-rate-broadcast

Description

The pitch.

## Summary

Fixes `delivery_rate` (and its `success_rate` alias) returning a value **greater than 1.0** on any scenario that uses broadcasts. A delivery "rate" above 1.0 is impossible and misreports every broadcast-heavy run.

## Where — exact location

- **File:** `packages/nest-core/nest_core/metrics.py`
- **Function:** `_delivery_rate` at **line 57**, surfaced as the `delivery_rate` metric and via the alias `_success_rate = _delivery_rate` at **line 92** (the `success_rate` metric name).

## Root cause

The simulator logs a one-to-one message as a `send` record (sender) plus a `receive` record (recipient). A **broadcast** is logged differently in `packages/nest-core/nest_core/sim/simulator.py`: one `broadcast` record on the sender and one `receive` record per recipient — **no `send` records at all**.

The old `_delivery_rate` used `send` events as the denominator and `receive` events as the numerator:

```python
# BEFORE  (metrics.py:57)
sends = 0
receives = 0
for ev in events:
    kind = ev.get("kind", "")
    if kind == "send":
        sends += 1
    elif kind == "receive":
        receives += 1
if sends == 0:
    return 0.0
return receives / sends
```

Every broadcast adds N to `receives` and 0 to `sends`, so the ratio climbs above 1.0.

## Reproduction — a shipped scenario

`scenarios_builtin/reputation.py`'s observer calls `ctx.broadcast(...)`:

```
$ uv run nest run reputation
# trace event kinds: {'send': 280, 'receive': 340, 'broadcast': 3, ...}
# delivery_rate = 340 / 280 = 1.2143   <-- impossible
```

## The fix

Count **delivery attempts** for the denominator. Every attempt the simulator makes ends as exactly one record — a `receive` (delivered) or a `dropped` (lost) — whether it came from a `send` or a `broadcast`:

```python
# AFTER  (metrics.py:57)
receives = 0
dropped = 0
for ev in events:
    kind = ev.get("kind", "")
    if kind == "receive":
        receives += 1
    elif kind == "dropped":
        dropped += 1
attempts = receives + dropped
if attempts == 0:
    return 0.0
return receives / attempts
```

**Why this is safe for existing traces:** for unicast, every `send` yields exactly one `receive` or one `dropped`, so `sends == receives + dropped` and the value is *identical*. It only changes the broadcast case, and it can never exceed 1.0. After the fix, `reputation` reports `1.0`.

## Tests

- **Unchanged, still pass:** `test_delivery_rate` (`test_metrics.py:35`) and `test_success_rate_backward_compat` (`test_metrics.py:41`) — a unicast trace with 3 sends / 2 receives / 1 dropped, still `2/3`.
- **Added:** `test_delivery_rate_with_broadcast_stays_within_one` at **`packages/nest-core/tests/test_metrics.py:48`** — a trace with two broadcasts (one fully delivered, one with a dropped fan-out). Asserts the rate is `<= 1.0` and equals `3/4` (3 delivered of 4 attempts). This test **fails** on the old `receives / sends` formula.

## Files changed (2 files, +19 / -7 and +26)

| File | +/- | What |
|---|---|---|
| `packages/nest-core/nest_core/metrics.py` | +19 / -7 | Rewrote `_delivery_rate` denominator (line 57) |
| `packages/nest-core/tests/test_metrics.py` | +26 | Broadcast regression test (line 48) |

## Verification — full CI, branch merged with current `main`

`uv run ruff check .` clean · `uv run ruff format --check .` clean (172 files) · `uv run pyright` strict **0 errors** · `uv run pytest` **792 passed**, 1 skipped. Scoped to the one metric plus its test; no other metric or behavior is touched.

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/95/head:pr-95
git checkout pr-95