feat: split event listener protocols - #150
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThe PR splits ChangesEvent listener protocol split
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/guides/observability.md (1)
39-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the
docs/llms.txtObservability entry.The entry lists only
EventListener. Add the three narrow protocols and state that core/storage hooks use breaker names while pipeline hooks use strategy names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/observability.md` around lines 39 - 52, Update the Observability entry in docs/llms.txt to list CoreEventListener, StorageEventListener, and PipelineEventListener alongside EventListener. Document that core and storage hooks receive breaker names, while pipeline hooks receive strategy names.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/observability.md`:
- Around line 39-52: Revise the timing documentation in the observability guide
to apply the “after the protected call returns” behavior only to core and
storage breaker hooks. Document pipeline hook timing separately, noting that
PipelineEventListener callbacks occur during strategy execution, including retry
backoff, bulkhead admission failure, and fallback substitution paths.
- Around line 9-33: Make the protocol example runnable by adding the necessary
imports and definitions for Protocol, State, and Outcome before the listener
declarations, using the current public API symbols. Keep the CoreEventListener,
StorageEventListener, PipelineEventListener, and EventListener contracts
unchanged.
---
Nitpick comments:
In `@docs/guides/observability.md`:
- Around line 39-52: Update the Observability entry in docs/llms.txt to list
CoreEventListener, StorageEventListener, and PipelineEventListener alongside
EventListener. Document that core and storage hooks receive breaker names, while
pipeline hooks receive strategy names.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a927b3ec-b503-41fb-990b-57ef70aba461
📒 Files selected for processing (18)
CHANGELOG.mddocs/guides/observability.mddocs/llms-full.txtinterlock/__init__.pyinterlock/_engine.pyinterlock/_notify.pyinterlock/breaker.pyinterlock/integrations/_registry.pyinterlock/integrations/aiohttp.pyinterlock/integrations/httpx.pyinterlock/integrations/httpx2.pyinterlock/integrations/requests.pyinterlock/integrations/tenacity.pyinterlock/pipeline.pyinterlock/protocols.pyinterlock/registry.pytests/test_notify.pytests/typing_surface.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Run benchmarks
- GitHub Check: quality (3.14t)
- GitHub Check: quality (3.14)
- GitHub Check: quality (3.11)
- GitHub Check: quality (3.13)
- GitHub Check: quality (3.12)
- GitHub Check: Coverage
⚠️ CI failures not shown inline (2)
GitHub Actions: Code scanning AI findings on PR #150 / github-advanced-security: Code scanning AI findings on PR #150
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "RUNNER_TEMP=$RUNNER_TEMP"�[0m
�[36;1mfind "$RUNNER_TEMP" -maxdepth 1 -type f -name 'git-credentials-*.config' -print -delete�[0m
�[36;1m�[0m
�[36;1m# Generate a unique token and stop processing workflow commands to prevent the runtime from injecting commands�[0m
�[36;1mSTOP_***REDACTED_SECRET_ASSIGNMENT*** /proc/sys/kernel/random/uuid)�[0m
�[36;1m�[0m
�[36;1m# Use a trap to ensure we always resume command processing and check for�[0m
�[36;1m# fallback error annotations, even if the runtime exits with a non-zero code�[0m
�[36;1m# (which would otherwise cause set -e to abort the shell before we get here).�[0m
�[36;1m# The trap preserves the original exit code.�[0m
�[36;1mcopilot_cleanup() {�[0m
�[36;1m �[0m
�[36;1m if [ -n "${GIT_PROXY_PID:-}" ] && kill -0 "$GIT_PROXY_PID" 2>/dev/null; then�[0m
�[36;1m echo "Stopping git-proxy (pid=$GIT_PROXY_PID)..."�[0m
�[36;1m kill "$GIT_PROXY_PID" 2>/dev/null || true�[0m
�[36;1m for _ in {1..25}; do�[0m
�[36;1m if ! kill -0 "$GIT_PROXY_PID" 2>/dev/null; then break; fi�[0m
�[36;1m sleep 0.2�[0m
�[36;1m done�[0m
�[36;1m if kill -0 "$GIT_PROXY_PID" 2>/dev/null; then�[0m
�[36;1m echo "git-proxy did not stop gracefully; forcing termination."�[0m
�[36;1m kill -KILL "$GIT_PROXY_PID" 2>/dev/null || true�[0m
�[36;1m fi�[0m
�[36;1m wait "$GIT_PROXY_PID" 2>/dev/null || true�[0m
�[36;1m fi�[0m
�[36;1m �[0m
�[36;1m echo "::$STOP_***REDACTED_SECRET_ASSIGNMENT***
�[36;1m FALLBACK_FILE="${RUNNER_TEMP}/copilot-fallback-error.txt"�[0m
�[36;1m if [ -f "$FALLBACK_FILE" ]; then�[0m
�[36;1m FALLBACK_MSG=$(head -c 500 "$FALLBACK_FILE" | tr -d '\n\r')�[0m
�[36;1m echo "::error title=Copilot Error::${FALLBACK_MSG}"�[0m
GitHub Actions: Code scanning AI findings on PR #150 / 0_github-advanced-security.txt: Code scanning AI findings on PR #150
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mecho "RUNNER_TEMP=$RUNNER_TEMP"�[0m
�[36;1mfind "$RUNNER_TEMP" -maxdepth 1 -type f -name 'git-credentials-*.config' -print -delete�[0m
�[36;1m�[0m
�[36;1m# Generate a unique token and stop processing workflow commands to prevent the runtime from injecting commands�[0m
�[36;1mSTOP_***REDACTED_SECRET_ASSIGNMENT*** /proc/sys/kernel/random/uuid)�[0m
�[36;1m�[0m
�[36;1m# Use a trap to ensure we always resume command processing and check for�[0m
�[36;1m# fallback error annotations, even if the runtime exits with a non-zero code�[0m
�[36;1m# (which would otherwise cause set -e to abort the shell before we get here).�[0m
�[36;1m# The trap preserves the original exit code.�[0m
�[36;1mcopilot_cleanup() {�[0m
�[36;1m �[0m
�[36;1m if [ -n "${GIT_PROXY_PID:-}" ] && kill -0 "$GIT_PROXY_PID" 2>/dev/null; then�[0m
�[36;1m echo "Stopping git-proxy (pid=$GIT_PROXY_PID)..."�[0m
�[36;1m kill "$GIT_PROXY_PID" 2>/dev/null || true�[0m
�[36;1m for _ in {1..25}; do�[0m
�[36;1m if ! kill -0 "$GIT_PROXY_PID" 2>/dev/null; then break; fi�[0m
�[36;1m sleep 0.2�[0m
�[36;1m done�[0m
�[36;1m if kill -0 "$GIT_PROXY_PID" 2>/dev/null; then�[0m
�[36;1m echo "git-proxy did not stop gracefully; forcing termination."�[0m
�[36;1m kill -KILL "$GIT_PROXY_PID" 2>/dev/null || true�[0m
�[36;1m fi�[0m
�[36;1m wait "$GIT_PROXY_PID" 2>/dev/null || true�[0m
�[36;1m fi�[0m
�[36;1m �[0m
�[36;1m echo "::$STOP_***REDACTED_SECRET_ASSIGNMENT***
�[36;1m FALLBACK_FILE="${RUNNER_TEMP}/copilot-fallback-error.txt"�[0m
�[36;1m if [ -f "$FALLBACK_FILE" ]; then�[0m
�[36;1m FALLBACK_MSG=$(head -c 500 "$FALLBACK_FILE" | tr -d '\n\r')�[0m
�[36;1m echo "::error title=Copilot Error::${FALLBACK_MSG}"�[0m
🧰 Additional context used
📓 Path-based instructions (15)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Support Python 3.11 and newer; use Python 3.11+ features where required.
Keep the core zero-dependency and use only the standard library; external dependencies must be isolated behind optional integrations.
Use injectedClockinstances for all time access; do not calltime.monotonic()orsleep()directly in logic.
Implement the core as an I/O-free state machine with a singlethreading.Lockaround the await-free critical section, never held across the protected call.
UseProtocols for extension points:Clock,SlidingWindow,Storage,FailureClassifier, andEventListener; do not inherit from internal classes.
Expose one publicCircuitBreakerclass for sync and async operation, with separate internal paths selected by coroutine detection.
Expose the public API through the package__init__.py; keep helpers underscore-prefixed and hidden.
Use absolute imports, placed at the top of the file, ordered as standard library, third-party, then local imports with blank lines between groups.
Use a maximum line length of 100 characters, single-quoted strings, f-strings, andpathlib.Pathinstead ofos.path.
Annotate every parameter and return value; use modern generic syntax andX | Noneinstead ofOptional[X].
UseStrEnumor module-level constants instead of magic constants.
When a constructor or function has three or more arguments, pass them by keyword.
Keep functions focused on one job, generally no longer than 20–30 lines, with minimal side effects and extracted repeated loop logic.
Useasync/awaitfor I/O-bound work,asyncio.TaskGroupinstead ofasyncio.gather, andasyncio.to_threadorProcessPoolExecutorfor CPU-bound work.
Do not mix sync and async in one function; never await a sync callable or block on an async callable.
Fail fast on invalid input or state by raising immediately; do not continue with partial results or invented defaults.
Catch only expected exceptions, log them with context, and re-raise; do not use ...
Files:
interlock/_notify.pyinterlock/_engine.pyinterlock/integrations/_registry.pyinterlock/__init__.pyinterlock/breaker.pyinterlock/integrations/requests.pyinterlock/registry.pyinterlock/integrations/aiohttp.pytests/test_notify.pyinterlock/pipeline.pyinterlock/integrations/httpx.pyinterlock/protocols.pyinterlock/integrations/tenacity.pytests/typing_surface.pyinterlock/integrations/httpx2.py
interlock/_notify.py
📄 CodeRabbit inference engine (AGENTS.md)
In
interlock/_notify.py, logEventListenerhook exceptions with traceback and swallow them; allowBaseExceptionto propagate.
Files:
interlock/_notify.py
{interlock/**/*.py,docs/**/*.md,docs/llms-full.txt,docs/llms.txt}
📄 CodeRabbit inference engine (Custom checks)
When a change affects user-facing behaviour through the public API, integrations, or configuration options, update the relevant page under
docs/and regeneratedocs/llms-full.txt; when adding a new documentation page, list it under## Docsindocs/llms.txt.
Files:
interlock/_notify.pyinterlock/_engine.pyinterlock/integrations/_registry.pyinterlock/__init__.pyinterlock/breaker.pyinterlock/integrations/requests.pyinterlock/registry.pyinterlock/integrations/aiohttp.pydocs/llms-full.txtinterlock/pipeline.pyinterlock/integrations/httpx.pydocs/guides/observability.mdinterlock/protocols.pyinterlock/integrations/tenacity.pyinterlock/integrations/httpx2.py
interlock/**/*.py
📄 CodeRabbit inference engine (Custom checks)
Every production behaviour change in
interlock/must be accompanied by a change undertests/; changes limited to docstrings, comments, or type annotations are exempt. Bug fixes must include at least one regression test that fails without the production fix.Keep the core dependency-free; external dependencies must belong to extras and be imported lazily.
Files:
interlock/_notify.pyinterlock/_engine.pyinterlock/integrations/_registry.pyinterlock/__init__.pyinterlock/breaker.pyinterlock/integrations/requests.pyinterlock/registry.pyinterlock/integrations/aiohttp.pyinterlock/pipeline.pyinterlock/integrations/httpx.pyinterlock/protocols.pyinterlock/integrations/tenacity.pyinterlock/integrations/httpx2.py
⚙️ CodeRabbit configuration file
Core rules (AGENTS.md is authoritative): (1) Zero-dependency core — anything under interlock/ except interlock/integrations/ may import stdlib only. Flag every third-party import as a blocking issue. (2) No fallbacks, no silent excepts, no
a or b or cfor required config or data, no hidden retries. Invalid input or state raises immediately. interlock/_notify.py is the one sanctioned swallow (listener hooks are observability, logged with traceback, BaseException still propagates) — do not suggest generalising or "fixing" it. (3) Time comes only from the injected Clock protocol; direct time.monotonic()/time.sleep() in library logic is a bug. (4) Style: 100-char lines, single quotes, f-strings, pathlib, full annotations,X | NoneneverOptional[X], keyword arguments for calls with 3+ arguments, no magic constants (StrEnum or module constants), functions under ~30 lines. (5) Extension points are Protocols (Clock, SlidingWindow, Storage, FailureClassifier, EventListener) — do not propose inheriting internal classes. (6) Sync and async live in one CircuitBreaker with separate internal paths; never propose Sync*/Async* twins and never mix the two paths in one function. (7) Public API is exported from interlock/init.py; everything else is underscore-prefixed. New public symbols need__all__and a docstring. (8) Python 3.11 is the floor — no 3.12+ syntax or stdlib.
Files:
interlock/_notify.pyinterlock/_engine.pyinterlock/integrations/_registry.pyinterlock/__init__.pyinterlock/breaker.pyinterlock/integrations/requests.pyinterlock/registry.pyinterlock/integrations/aiohttp.pyinterlock/pipeline.pyinterlock/integrations/httpx.pyinterlock/protocols.pyinterlock/integrations/tenacity.pyinterlock/integrations/httpx2.py
{interlock/*.py,interlock/!(integrations)/**/*.py,pyproject.toml}
📄 CodeRabbit inference engine (Custom checks)
Keep the core zero-dependency: files under
interlock/outsideinterlock/integrations/may import only the standard library or otherinterlockmodules;[project] dependenciesinpyproject.tomlmust remain empty; andinterlock/__init__.pymust not re-export frominterlock.integrations.
Files:
interlock/_notify.pyinterlock/_engine.pyinterlock/__init__.pyinterlock/breaker.pyinterlock/registry.pyinterlock/pipeline.pyinterlock/protocols.py
interlock/_engine.py
📄 CodeRabbit inference engine (AGENTS.md)
Run mutation testing with
mutmutwhenever_engine.pyis changed.Mutation testing covers lock scope, dispatch, and recording order; survivors outside the documented equivalent-mutant classes indicate missing tests and should be fixed with tests.
Files:
interlock/_engine.py
interlock/{_state_machine,_engine,_coordination}.py
⚙️ CodeRabbit configuration file
The critical section. Verify: the state machine stays I/O-free and unaware of sync vs async; the threading.Lock covers only the await-free acquire and record sections and is never held across the protected call (a call under the lock is a deadlock and a throughput bug); HALF_OPEN still bounds concurrent probes. Ask whether
uv run mutmut runwas run — AGENTS.md requires it for these files, because 100% coverage here does not prove the new assertions bite.
Files:
interlock/_engine.py
interlock/integrations/**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Integration dependencies must be optional extras and imported lazily.
Files:
interlock/integrations/_registry.pyinterlock/integrations/requests.pyinterlock/integrations/aiohttp.pyinterlock/integrations/httpx.pyinterlock/integrations/tenacity.pyinterlock/integrations/httpx2.py
⚙️ CodeRabbit configuration file
Optional extras. The third-party import must stay inside this package, must never be re-exported from interlock/init.py, and a missing extra must fail with a clear install hint rather than a fallback. Wrap the dependency behind the project's own types so its objects do not leak into core signatures. Check that the extra is declared in pyproject.toml
[project.optional-dependencies]and documented under docs/integrations/.
Files:
interlock/integrations/_registry.pyinterlock/integrations/requests.pyinterlock/integrations/aiohttp.pyinterlock/integrations/httpx.pyinterlock/integrations/tenacity.pyinterlock/integrations/httpx2.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Document user-facing changes in English Markdown documentation and keep generated documentation mirrors synchronized.
Files:
CHANGELOG.mddocs/guides/observability.md
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Add every change to the
[Unreleased]section underAdded,Fixed, orChanged, explaining user impact rather than only symbol movement.User-facing changes must update the
[Unreleased]section.
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
Keep a Changelog format. New entries go under
## [Unreleased]in Added / Fixed / Changed. An entry describes what a user could not do before and can now, not which symbol moved. Only the release commit dates a section and updates the link references.
Files:
CHANGELOG.md
{interlock/__init__.py,interlock/pipeline.py}
📄 CodeRabbit inference engine (Custom checks)
Do not remove or change signatures of symbols exported from
interlock/__init__.pyorinterlock/pipeline.py—including removed or reordered positional parameters, narrowed types, or renamed public names—unless the PR has thebreaking-changelabel andCHANGELOG.mdincludes a migration note. CI enforces this withuv run griffe check.
Files:
interlock/__init__.pyinterlock/pipeline.py
docs/llms-full.txt
⚙️ CodeRabbit configuration file
Generated artefact — produced by
uv run python scripts/build_llms_full.py. Do not review its content or suggest edits; only confirm it was regenerated together with the docs/ changes in the same PR.
Files:
docs/llms-full.txt
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest functions rather than test classes, with names formatted astest__unit_of_work__state_under_test__expected_behavior.
Mirror package layout in test filenames, use Arrange-Act-Assert, and create fixtures for repeated setup.
Use injectedClockinstances for deterministic tests; do not usesleep()in tests.
Usepytest-asyncioand@pytest.mark.asynciofor asynchronous tests, and usepytest-mockto isolate external dependencies.
Use Hypothesis property-based tests for the state machine and cover all transitions and races.
Write the reproducing test before a bug fix and specify the required behavior before implementing a feature.Tests must preserve 100% coverage, avoid
sleepfor time-dependent behavior, and use injected clocks instead.
Files:
tests/test_notify.pytests/typing_surface.py
⚙️ CodeRabbit configuration file
pytest functions only, never test classes. Names follow
test__unit_of_work__state_under_test__expected_behaviorin lower case. One behaviour per test, Arrange-Act-Assert. Time is the injected fake Clock — any real sleep or wall-clock read is flakiness, flag it. Async tests use@pytest.mark.asyncio; state-machine work carries hypothesis property tests. Coverage must stay at 100%: point out uncovered branches the diff introduces. Tests run under-n auto, so anything relying on ordering or shared global state is a bug.
Files:
tests/test_notify.pytests/typing_surface.py
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-facing changes must update the relevant documentation page under
docs/.
Files:
docs/guides/observability.md
docs/**/*.md
⚙️ CodeRabbit configuration file
User-facing documentation. Check that code samples match the current public API and would actually run. A new page must also be listed in docs/llms.txt under
## Docs. Keep the existing voice: short sentences, no marketing.
Files:
docs/guides/observability.md
🔇 Additional comments (14)
docs/guides/observability.md (1)
145-165: LGTM!docs/llms-full.txt (1)
1781-1824: LGTM!Also applies to: 1917-1937
tests/test_notify.py (2)
26-33: LGTM!
145-152: 🎯 Functional CorrectnessNo runtime-checkability issue
All three listener protocols use
@runtime_checkable; theseisinstance()assertions do not raiseTypeError.> Likely an incorrect or invalid review comment.tests/typing_surface.py (1)
16-25: LGTM!Also applies to: 35-49, 59-68
CHANGELOG.md (1)
11-18: LGTM!interlock/_engine.py (2)
31-38: LGTM!
92-92: 📐 Maintainability & Code QualityRun the required mutation test.
interlock/_engine.pychanged. Runuv run mutmut runand confirm coverage for lock scope, dispatch, and recording order.As per path instructions, run mutation testing with
mutmutwhenever_engine.pychanges.Source: Path instructions
interlock/breaker.py (2)
29-36: LGTM!
77-77: 🗄️ Data Integrity & IntegrationKeep the specialized listener protocols.
EventListenerextends the specialized protocols, so existing complete listeners remain accepted. The specialized annotations also allow partial listeners and match the documented API.> Likely an incorrect or invalid review comment.interlock/integrations/requests.py (1)
35-35: LGTM!Also applies to: 115-115
interlock/pipeline.py (1)
40-40: LGTM!interlock/integrations/tenacity.py (1)
72-72: LGTM!Also applies to: 198-198
interlock/_notify.py (1)
27-41: LGTM!
## Summary Prepare the `2.5.0` minor release. - Bump the package version from `2.4.0` to `2.5.0`. - Move the current `[Unreleased]` changelog entries into `[2.5.0] - 2026-08-08`. - Update changelog comparison links and the comparison-page release version. - Regenerate `docs/llms-full.txt`. Minor, not patch: the release includes backward-compatible public additions: configurable transport breaker names, shared caller-owned registries, and narrower listener protocols. ## Checklist - [x] Tests added or updated (suite stays at 100% coverage) - [x] `uv run ruff format --check` and `uv run ruff check` pass - [x] `uv run mypy`, `uv run pyright` and `uv run pyrefly check` pass - [x] Docs updated (`docs/`) for user-facing changes - [x] `CHANGELOG.md` `[Unreleased]` updated - [x] Commits follow Conventional Commits Additional release checks: package and strict documentation builds pass. Griffe reports only the expected public `VERSION` change (`2.4.0` → `2.5.0`). ## Related issues Closes #148 Closes #149 Closes #150 Closes #151 Closes #152
Summary
EventListenercontractnamenamespace and regenerate the LLM documentation mirrorChecklist
uv run ruff format --checkanduv run ruff checkpassuv run mypy,uv run pyrightanduv run pyrefly checkpassdocs/) for user-facing changesCHANGELOG.md[Unreleased]updatedRelated issues
Closes #136
Added
CoreEventListener,StorageEventListener, andPipelineEventListenerto the public API.Changed
CoreEventListenerorStorageEventListener.PipelineEventListener.EventListenercontract as a composite of the narrow protocols.docs/llms-full.txtand updated observability documentation.Public API
EventListenermembers ornotifybehavior.