adding demo

[YM2132]

No description provided.

[github-actions]

PR Understanding Check

Please reply with a comment that starts with /answers and answer these questions in plain language.

1. What motivated adding this simple in-memory limiter (target use-case, expected QPS, single-process vs multi-process) and why choose a rebuilt list of timestamps rather than a deque/rolling buffer for efficiency?
2. This implementation is not thread-safe and uses time.time(), which can jump backwards — was concurrent access or system clock changes considered (race conditions corrupting self.timestamps, under/over-counting calls)?
3. How was this validated: are there unit tests for boundary behavior (t == cutoff), tests using a monotonic clock, stress/performance tests at expected load, and tests for edge params (max_calls <= 0, negative period_seconds)?

[YM2132]

/answers

1. Motivation and data structure choice

   The main motivation for adding this InMemoryRateLimiter is two-fold:

   • First, it gives us a realistic, stateful example for the PR Guard to reason about (more interesting than pure README / trivial changes).
   • Second, it’s a small utility we can reuse in scripts or small services where we want to cap how often an operation runs (e.g. “don’t hit this API more than a handful of times per second”).

   For this demo I’m assuming single-process, single-threaded use with relatively low QPS (think: a few calls per second, not thousands). Under those assumptions:

   • Rebuilding a Python list each call with a comprehension is simple and easy to read.
   • The number of timestamps in the window is small (max_calls is expected to be small as well), so the O(n) filter with n ≈ max_calls is fine.

   I chose a plain list instead of a deque/rolling buffer mainly for clarity: the logic is “filter to only recent timestamps, then append the current one if we’re under the limit”, which maps very directly to what the list code does. If we ever use this in a tighter loop with higher QPS or larger windows, swapping to a deque and avoiding full-list rebuilds would be a straightforward follow-up.

2. Thread-safety and time.time() vs monotonic clock

   You’re right that this implementation is not thread-safe and uses time.time() which can jump backwards.

   • Thread-safety:
     All state is kept on self.timestamps and we mutate that in-place in allow() without any locking. For the current intended use (single-threaded scripts / simple processes) this is acceptable and keeps the class minimal. If someone did call allow() concurrently from multiple threads, they could absolutely get race conditions (lost timestamps, double updates, or inconsistent length checks). If we wanted to make it safe for that scenario, we’d need to wrap the body of allow() with a lock around the filter + append operations.

   • Clock behavior:
     I used time.time() because it’s “good enough” for this in-memory demo and keeps imports minimal. If the system clock moves backwards, we could temporarily under-count calls (older timestamps suddenly look “in the future” relative to a smaller now) or over-count if the clock jumps forward in a way that interacts with the cutoff. For a production-grade limiter I’d switch this to time.monotonic() (or inject a clock function) so that we’re counting strictly increasing time deltas, independent of wall-clock changes.

   So yes, both concurrency and wall-clock jumps were considered; for the scope of this helper we’re explicitly accepting those limitations and would document it as a single-threaded, single-process, best-effort limiter.

3. Validation and edge cases

   For this PR I did manual validation rather than full unit coverage:

   • In a Python shell I tested the happy-path and boundary behavior around the window:

     from time import sleep
     from pr_guard_demo import InMemoryRateLimiter
     
     rl = InMemoryRateLimiter(max_calls=2, period_seconds=1.0)
     
     assert rl.allow() is True   # 1st call: allowed
     assert rl.allow() is True   # 2nd call: allowed
     assert rl.allow() is False  # 3rd call within the same 1s window: blocked
     
     sleep(1.1)
     # After the window, earlier timestamps get dropped and we can call again:
     assert rl.allow() is True

   • Regarding the boundary at t == cutoff: the filter is t >= cutoff, so events exactly on the cutoff boundary stay inside the window. That means an event at exactly now - period_seconds still counts against the limit, which is the intended interpretation (“last period_seconds, inclusive”).

   There are no formal unit tests yet for this helper, and no specific perf or stress tests, because:

   • For the current usage (demo + low-QPS helper) the manual checks above are enough to give confidence the semantics are correct.
   • If we promote this into a shared library, I’d follow up with:
     • Unit tests that simulate timestamps around the cutoff (just below / exactly equal / just above).
     • Tests that inject a fake monotonic clock so we’re not tied to real time.
     • Explicit tests for “edge params”: max_calls <= 0 and period_seconds <= 0.
       Right now, those are effectively invalid configurations; the class doesn’t guard against them and the behavior would be weird (e.g., zero/negative window). For a production version I’d add explicit validation in __post_init__ (e.g. if max_calls <= 0 or period_seconds <= 0: raise ValueError(...)) and tests to match.

[github-actions]

PR Guard Result: PASS

The author demonstrates clear understanding of the change and references concrete code/behavior. They explain the intended single-process, low-QPS use-case and why a rebuilt list via the list comprehension is acceptable for small max_calls, explicitly citing the simplicity/readability tradeoff vs deque. They acknowledge the lack of thread-safety (mutating self.timestamps in allow()) and the choice of time.time() (noting backward jumps and recommending time.monotonic() or injecting a clock), and they point out the exact cutoff semantics from the code (filter uses t >= cutoff so the boundary is inclusive). They also described manual validation with example calls and sleep, and laid out concrete follow-ups (unit tests for boundary behavior, injecting a monotonic clock, stress tests, and input validation in post_init). These responses are specific to the diff (pr_guard_demo.py) and propose concrete fixes, so PASS is appropriate.