Review This PR

This is a sample repository. It has a rules file (CLAUDE.md), a branch that adds reset(key) to an alert throttle, and the author's summary for that branch. The review below is the real, unedited output.

I reviewed feat/throttle-reset against main and found one blocking bug: reset("disk") clears the throttle for every alert, not just "disk". You asked for scrutiny of reset(), and that's where it is. Nothing in your notes covers it.

What I checked: I read your context notes (.claude/review-context/feat-throttle-reset.md) and the rules in CLAUDE.md, and looked at all three changed files. I ran python3 -m unittest -q and all 4 tests pass, as your notes say. There's no PR description to compare against because the repo has no git remote.

Blocking

1. reset(key) ignores key and clears every key (src/alert_throttle.py:20)

def reset(key):
    """Let the next alert for `key` through, e.g. after an operator acks it."""
    _last_sent.clear()

The docstring says it resets one key, but _last_sent.clear() empties the whole table. I ran this to confirm:

should_send("disk", now=0); should_send("cpu", now=0)
reset("disk")
should_send("cpu", now=10)   # → True; should be False

In production, acking one alert would release every throttled alert at once. That's the alert storm your notes say the 300 s window exists to prevent. The fix is _last_sent.pop(key, None).

2. The new test can't catch this (tests/test_alerts.py:20-23)

test_reset_lets_next_alert_through only uses "disk", so a reset that clears everything still passes. CLAUDE.md rule 2 asks for a test of each behaviour change, and the behaviour here is "reset only this key". Please add a test that sends "disk" and "cpu", resets "disk", and asserts that should_send("cpu", now=10) is False. That test fails on the current code.

Nit (not blocking)

• tests/test_alerts.py:18 still hard-codes now=301. The code now reads the window from settings, but this test still assumes it's 300. If someone changes ALERT_THROTTLE_WINDOW, the test breaks. settings.ALERT_THROTTLE_WINDOW + 1 would keep it in step. This line was already on main, so it's your call.

No issues

• Moving the 300 s window into settings.py follows rule 1, and the setting is defined there.
• An in-memory dict is a reasonable choice given one process on one VM, so I didn't suggest Redis. Wiring reset() to the ack button is out of scope as your notes say, so I didn't review it.

Verdict: request changes. The fix is a one-line code change plus one test.