feat(events_bus,api): EventBus lifecycle + layer guard + public-API surface lock#167
Open
Gradata wants to merge 1 commit intopr/dualwrite-atomicity-2026-05-02from
Open
Conversation
…urface lock
Council v4 (council_2026-05-02T11-59-00.md) flagged three remaining
production blockers after PR1+PR2: thread-lifecycle leaks in EventBus,
layer-violation discipline, and an undefined public API surface that
escapes SemVer guarantees.
What
- src/gradata/events_bus.py
- Added EventBus.close()/shutdown() with idempotent drain semantics:
drain in-flight async handlers, clear listeners, reject late on()/
emit() calls (logged at DEBUG, not raised).
- Tracks every live EventBus in a WeakSet and registers a single
atexit handler so background threads cannot outlive the process if
a caller forgets to close.
- Single threading.Lock guards listeners + closed flag.
- src/gradata/brain.py
- Brain.close() now drains the EventBus before re-encrypting the
database; covers the typical `with Brain(...) as b:` lifecycle.
- src/gradata/__init__.py
- Locked __all__ to the 25-name documented public surface
(Brain, BrainContext, Lesson, LessonState, ScopedBrain, the
9 exception classes, the 6 enhancement helpers, and __version__).
- src/gradata/contrib/patterns/orchestrator.py
- Removed eager Layer 1 import from Layer 0 module; remaining cross-
layer access is via lazy function-local imports documented in
test_layer_enforcement.ALLOWED_UPWARD_IMPORTS.
- tests/test_eventbus_lifecycle.py — 3 tests covering concurrent
subscribe/unsubscribe, executor wait-on-close + late-submit rejection,
and 100-cycle Brain init/close with active_count() leak detection.
- tests/test_layer_enforcement.py — ast-walks src/gradata/, classifies
every upward import; fails the build on any new unflagged violation.
Pre-existing _core.py and _scoped_brain.py edges are catalogued as
DEFERRED (>50-line refactor) or LAZY-IMPORT-OK.
- tests/test_public_api_surface.py — golden test that fails the build
if gradata.__all__ drifts from the locked set in either direction.
Why
- EventBus had no shutdown — Brain.close() left ThreadPoolExecutor
workers alive. Long-running test suites accumulated threads, and
process-exit could hit "thread still running at interpreter
shutdown" in noisy environments.
- Layer-violation drift is silent: every accidental
Layer-0->Layer-1 import was a future cycle waiting to happen.
- Without an __all__ lock, every implicit export becomes a SemVer
contract by accident.
Test plan
- pytest tests/test_eventbus_lifecycle.py tests/test_layer_enforcement.py
tests/test_public_api_surface.py — 5 passed.
- pytest tests/test_brain.py tests/test_middleware_core.py
tests/test_rule_pipeline.py tests/test_retrieval_fusion.py — 73 passed.
- pyright src/ — 0 errors, 27 warnings (unchanged baseline).
Layering check
- All upward imports introduced or pre-existing are catalogued in
tests/test_layer_enforcement.ALLOWED_UPWARD_IMPORTS with a
classification (PUBLIC BARREL / LAZY-IMPORT-OK / DEFERRED).
- Net change: orchestrator.py loses a real upward edge; nothing new.
Risk
- EventBus.close() is now called from Brain.close(). Any code path that
uses Brain after close() will see emit/on/off become no-ops. This is
the intended contract.
- atexit handler is best-effort (timeout 2s per bus). Hung handlers in
the pool will not be force-killed; they get logged.
Council references
- council_2026-05-02T11-59-00.md
- council_2026-05-02T12-24-08.md (autonomous-mode policy)
Stacks on #164.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the last 3 council blockers from
council_2026-05-02T11-59-00.md: thread lifecycle, layer-violation discipline, public API surface lock.Stacks on #164 — please merge that one first.
Changes
EventBus lifecycle
EventBus.close()/shutdown()— idempotent drain semantics. Drains in-flight async handlers, clears listeners, rejects lateon()/emit()(DEBUG log, never raised).atexithandler so backgroundgradata-bus*threads cannot outlive the process even if a caller forgetsclose().Brain.close()now drains the EventBus before re-encrypting the database.Layer-violation guard
tests/test_layer_enforcement.pyast-walkssrc/gradata/, classifies every upward import (L0 → L1/L1 → L2etc.), fails the build on any new unflagged violation._core.py(DEFERRED — >50-line refactor) and_scoped_brain.py(LAZY-IMPORT-OK) are catalogued inALLOWED_UPWARD_IMPORTSwith rationale.contrib/patterns/orchestrator.pylost its eager Layer-1 import.Public API surface lock
gradata/__init__.py—__all__locked to the 25-name documented surface (Brain, BrainContext, Lesson, LessonState, ScopedBrain, 9 exception classes, 6 enhancement helpers,__version__).tests/test_public_api_surface.py— golden test fails on any drift in either direction.Test plan
pytest tests/test_eventbus_lifecycle.py tests/test_layer_enforcement.py tests/test_public_api_surface.py— 5 passed.pytest tests/test_brain.py tests/test_middleware_core.py tests/test_rule_pipeline.py tests/test_retrieval_fusion.py— 73 passed.pyright src/— 0 errors, 27 warnings (unchanged baseline).Layering check
All upward imports — introduced or pre-existing — are catalogued in
ALLOWED_UPWARD_IMPORTSwith classification (PUBLIC BARREL / LAZY-IMPORT-OK / DEFERRED). Net change:orchestrator.pyloses a real upward edge; no new edges introduced.Risk
EventBus.close()runs fromBrain.close(). Code that touches Brain after close sees emit/on/off as no-ops — intended contract.atexithandler is best-effort (2s timeout/bus). Hung handlers in the pool aren't force-killed; logged.Council references
council_2026-05-02T11-59-00.mdcouncil_2026-05-02T12-24-08.md(autonomous-mode policy)