Conversation
When the judge evaluator raises (provider error, timeout), the DebateResolver now falls back to authority-based resolution instead of propagating the exception. Adds CONFLICT_DEBATE_EVALUATOR_FAILED event constant for observability. Closes #1117
- Add min_interval_seconds to MeetingTypeConfig for per-type cooldown on event-triggered meetings. MeetingScheduler tracks last-triggered timestamps and skips meetings within the cooldown window. - Add max_tasks_per_meeting to MeetingProtocolConfig to cap task creation from a single meeting's action items. - Add MEETING_EVENT_COOLDOWN_SKIPPED and MEETING_TASKS_CAPPED events. - Clock injection on MeetingScheduler for deterministic testing. Closes #1115
…breaker (#1116) - Replace state eviction on cooldown expiry with bounce_count reset, preserving trip_count across resets. Cooldown formula: base * 2^(trip_count - 1), capped at max_cooldown_seconds (default 1h). - Add max_cooldown_seconds field to CircuitBreakerConfig. - Add dirty-tracking with async persist_dirty()/load_state() methods for SQLite persistence. Hot path (check/record_delegation) stays sync. - New CircuitBreakerStateRepository protocol + SQLite implementation. - Add circuit_breaker_state table to schema.sql. - Wire into PersistenceBackend protocol and SQLitePersistenceBackend. - Add DELEGATION_LOOP_CIRCUIT_BACKOFF event constant. Closes #1116
Wire the new circuit_breaker_state property into FakePersistenceBackend so all test files using it satisfy the PersistenceBackend protocol.
…or guards) Pre-reviewed by 7 agents, 4 findings addressed: - Fix wrong event constant in persist_dirty (CIRCUIT_RESET -> PERSIST_FAILED) - Add MemoryError/RecursionError guard to debate_strategy evaluator fallback - Add CHECK constraints to circuit_breaker_state schema (length > 0, >= 0) - Add error handling + logging to load_state() before re-raising
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds per-meeting cooldown and task-cap configuration: MeetingTypeConfig.min_interval_seconds and MeetingProtocolConfig.max_tasks_per_meeting; MeetingScheduler enforces per-type cooldowns and MeetingOrchestrator enforces task caps. Changes the delegation circuit breaker to a per-agent-pair exponential backoff tracked by bounce_count and trip_count, capped by CircuitBreakerConfig.max_cooldown_seconds, preserves trip_count across resets, and exposes async load_state and persist_dirty on DelegationCircuitBreaker. DelegationGuard accepts an optional circuit-breaker repo and exposes load_state/persist. DebateResolver now falls back to authority when the evaluator raises (but re-raises MemoryError and RecursionError). New SQLite repo, schema, tests, and observability events added. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces several resilience and configuration enhancements, including exponential backoff and state persistence for the delegation circuit breaker, a cooldown mechanism for event-triggered meetings, and a cap on the number of tasks created from meeting action items. Additionally, the conflict resolution strategy now includes an authority fallback when the debate evaluator fails. Critical syntax errors were identified in multiple files where multiple exceptions are caught without the required parentheses, which will lead to runtime failures.
| conflict, | ||
| judge_id, | ||
| ) | ||
| except MemoryError, RecursionError: |
There was a problem hiding this comment.
Catching multiple exceptions in Python 3 requires a tuple. The current syntax except MemoryError, RecursionError: is invalid and will result in a SyntaxError at runtime.
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| return | ||
| try: | ||
| records = await self._state_repo.load_all() | ||
| except MemoryError, RecursionError: |
There was a problem hiding this comment.
Catching multiple exceptions in Python 3 requires a tuple. The current syntax except MemoryError, RecursionError: is a SyntaxError.
except (MemoryError, RecursionError):| opened_at=pair.opened_at, | ||
| ) | ||
| await self._state_repo.save(record) | ||
| except MemoryError, RecursionError: |
There was a problem hiding this comment.
Catching multiple exceptions in Python 3 requires a tuple. The current syntax except MemoryError, RecursionError: is a SyntaxError.
except (MemoryError, RecursionError):
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR hardens SynthOrg’s communication subsystem to reduce multi-agent failure modes by adding safety guardrails (meeting cooldown/task caps), improving circuit breaker behavior (exponential backoff + persistence), and making debate-based conflict resolution resilient to evaluator failures.
Changes:
- Add per-meeting-type event-trigger cooldowns and cap task creation from meeting action items.
- Implement circuit breaker exponential backoff, persist circuit breaker state to SQLite, and add dirty-tracking persistence hooks.
- Fall back to authority-based resolution when the debate judge evaluator raises, with dedicated observability events.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/synthorg/communication/config.py | Adds min_interval_seconds and max_cooldown_seconds to support meeting cooldowns and capped circuit breaker backoff. |
| src/synthorg/communication/conflict_resolution/debate_strategy.py | Wraps judge evaluator failures and falls back to authority resolution. |
| src/synthorg/communication/loop_prevention/circuit_breaker.py | Adds trip-count-based exponential backoff + async dirty-tracked persistence helpers. |
| src/synthorg/communication/loop_prevention/guard.py | Wires optional circuit breaker persistence and exposes async load/persist entrypoints. |
| src/synthorg/communication/meeting/config.py | Adds max_tasks_per_meeting to prevent unbounded task creation from action items. |
| src/synthorg/communication/meeting/orchestrator.py | Enforces the per-meeting task creation cap and emits capping observability. |
| src/synthorg/communication/meeting/scheduler.py | Implements event-trigger cooldown filtering with injected clock support. |
| src/synthorg/observability/events/conflict.py | Adds conflict event constant for debate evaluator failure. |
| src/synthorg/observability/events/delegation.py | Adds delegation loop events for backoff + persistence failures. |
| src/synthorg/observability/events/meeting.py | Adds meeting scheduler cooldown-skipped and task-capped events. |
| src/synthorg/observability/events/persistence.py | Adds persistence event constants for circuit breaker state CRUD. |
| src/synthorg/persistence/circuit_breaker_repo.py | Introduces persistence protocol + record model for circuit breaker state. |
| src/synthorg/persistence/protocol.py | Extends persistence backend protocol with circuit_breaker_state repo access. |
| src/synthorg/persistence/sqlite/backend.py | Wires SQLite circuit breaker state repository into the backend. |
| src/synthorg/persistence/sqlite/circuit_breaker_repo.py | Implements SQLite repository for circuit breaker state persistence. |
| src/synthorg/persistence/sqlite/schema.sql | Adds circuit_breaker_state table to SQLite schema. |
| tests/unit/persistence/test_protocol.py | Updates protocol tests for the new repository property. |
| tests/unit/persistence/sqlite/test_migrations.py | Ensures migrations include the new circuit breaker state table. |
| tests/unit/persistence/sqlite/test_circuit_breaker_repo.py | Adds unit tests for the SQLite circuit breaker repository. |
| tests/unit/communication/meeting/test_scheduler.py | Adds cooldown behavior coverage for event-triggered meetings. |
| tests/unit/communication/meeting/test_orchestrator.py | Adds tests for max_tasks_per_meeting task creation capping. |
| tests/unit/communication/loop_prevention/test_circuit_breaker.py | Adds tests for exponential backoff and dirty-tracking persistence hooks. |
| tests/unit/communication/conflict_resolution/test_debate_strategy.py | Adds tests for evaluator exception fallback + event logging. |
| tests/unit/api/fakes_backend.py | Extends fake backend with an in-memory circuit breaker state repo for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| last = self._last_triggered.get(mt.name, 0.0) | ||
| if (now - last) < mt.min_interval_seconds: |
There was a problem hiding this comment.
Cooldown guard can incorrectly skip the first event-triggered meeting after startup: last defaults to 0.0, so if the injected/monotonic clock is still < min_interval_seconds, (now - last) < min_interval_seconds will be true and the meeting will be suppressed even though it has never run. Track absence explicitly (e.g., default last to None / use if mt.name in _last_triggered before applying the interval check).
| last = self._last_triggered.get(mt.name, 0.0) | |
| if (now - last) < mt.min_interval_seconds: | |
| last = self._last_triggered.get(mt.name) | |
| if ( | |
| last is not None | |
| and (now - last) < mt.min_interval_seconds | |
| ): |
| if trip_count <= 0: | ||
| return float(self._config.cooldown_seconds) | ||
| backoff = self._config.cooldown_seconds * (2 ** (trip_count - 1)) | ||
| return min(float(backoff), float(self._config.max_cooldown_seconds)) | ||
|
|
There was a problem hiding this comment.
_compute_cooldown() computes 2 ** (trip_count - 1) before applying the max cap. Because trip_count is unbounded (and persisted), a corrupted/large value can trigger extremely large integer exponentiation and high CPU/memory usage. Consider capping the exponent based on max_cooldown_seconds / cooldown_seconds (or using a safe loop/bit-shift) before exponentiating, so cooldown computation remains O(1) even for huge trip_count.
| if trip_count <= 0: | |
| return float(self._config.cooldown_seconds) | |
| backoff = self._config.cooldown_seconds * (2 ** (trip_count - 1)) | |
| return min(float(backoff), float(self._config.max_cooldown_seconds)) | |
| base_cooldown = self._config.cooldown_seconds | |
| max_cooldown = self._config.max_cooldown_seconds | |
| if trip_count <= 0: | |
| return float(base_cooldown) | |
| if base_cooldown >= max_cooldown: | |
| return float(max_cooldown) | |
| # Cap the exponent before computing the power so persisted or | |
| # corrupted trip counts cannot trigger expensive big-int math. | |
| max_exponent = 0 | |
| scaled_cooldown = base_cooldown | |
| while scaled_cooldown <= max_cooldown // 2: | |
| scaled_cooldown *= 2 | |
| max_exponent += 1 | |
| exponent = min(trip_count - 1, max_exponent) | |
| backoff = base_cooldown * (2 ** exponent) | |
| return min(float(backoff), float(max_cooldown)) |
| dirty = set(self._dirty) | ||
| self._dirty.clear() | ||
| for key in dirty: |
There was a problem hiding this comment.
persist_dirty() clears _dirty before attempting any writes; if save() fails for a key, the failure is logged but the key is not re-marked dirty, so that state update is silently dropped and won’t be retried on the next persist cycle. Consider re-adding the key to _dirty on exception (or deferring _dirty.clear() until after successful saves) to preserve eventual persistence under transient SQLite errors.
|
|
||
|
|
There was a problem hiding this comment.
max_cooldown_seconds is described as a cap on exponential backoff, but the model currently allows it to be less than cooldown_seconds, which would unexpectedly reduce the base cooldown on the first trip. Add a validator to enforce max_cooldown_seconds >= cooldown_seconds (or document/rename if smaller-than-base is intended).
| @model_validator(mode="after") | |
| def _validate_cooldown_bounds(self) -> Self: | |
| """Ensure the exponential backoff cap is not below the base cooldown.""" | |
| if self.max_cooldown_seconds < self.cooldown_seconds: | |
| raise ValueError( | |
| "max_cooldown_seconds must be greater than or equal to cooldown_seconds" | |
| ) | |
| return self |
| async def save(self, record: CircuitBreakerStateRecord) -> None: | ||
| """Persist a circuit breaker state record (upsert by pair key).""" | ||
| try: | ||
| await self._db.execute( | ||
| """\ |
There was a problem hiding this comment.
This repo performs write + commit sequences on the shared aiosqlite.Connection without coordinating via the backend’s shared write lock. Given the existing guidance in the SQLite backend/other repos about interleaved writers causing transaction interference on a shared connection, consider accepting an optional write_lock in __init__ (wired from the backend) and wrapping execute()+commit() in async with to serialize writes across repositories.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1140 +/- ##
==========================================
- Coverage 89.02% 88.97% -0.05%
==========================================
Files 843 845 +2
Lines 49061 49228 +167
Branches 4934 4944 +10
==========================================
+ Hits 43676 43802 +126
- Misses 4467 4502 +35
- Partials 918 924 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/synthorg/communication/meeting/config.py (1)
82-111: 🧹 Nitpick | 🔵 TrivialDocstring missing
max_tasks_per_meetingin Attributes section.The class docstring lists other attributes (lines 89-95) but omits the newly added
max_tasks_per_meetingfield. For consistency and discoverability, add it to the Attributes section.📝 Suggested docstring update
Attributes: protocol: Which protocol strategy to use. auto_create_tasks: Whether to auto-create tasks from action items extracted during any protocol execution. + max_tasks_per_meeting: Optional cap on how many tasks to create + from a single meeting's action items. round_robin: Round-robin protocol settings. position_papers: Position-papers protocol settings. structured_phases: Structured-phases protocol settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/communication/meeting/config.py` around lines 82 - 111, The class docstring for MeetingProtocolConfig is missing the max_tasks_per_meeting attribute; update the Attributes section of the MeetingProtocolConfig docstring to include a brief entry for max_tasks_per_meeting describing it as an optional int (minimum 1) that limits how many tasks can be auto-created from a single meeting's action items, matching the Field description used in the max_tasks_per_meeting model field.src/synthorg/persistence/protocol.py (1)
66-96: 🧹 Nitpick | 🔵 TrivialDocstring Attributes section missing
circuit_breaker_state.The class-level docstring lists all repository properties but omits the newly added
circuit_breaker_state. Add it for completeness.📝 Suggested docstring update
risk_overrides: Repository for RiskTierOverride persistence. ssrf_violations: Repository for SsrfViolation persistence. + circuit_breaker_state: Repository for circuit breaker state persistence. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/persistence/protocol.py` around lines 66 - 96, The class-level docstring for the persistence protocol omits the newly added attribute circuit_breaker_state; update the Attributes section of the Protocol class docstring to include a line like "circuit_breaker_state: Repository for CircuitBreakerState persistence." so the docstring lists this repository alongside tasks, cost_records, messages, etc.; locate the Protocol (or equivalent) class docstring in src/synthorg/persistence/protocol.py and add the circuit_breaker_state entry consistent with the existing wording and formatting.src/synthorg/communication/loop_prevention/circuit_breaker.py (1)
183-186:⚠️ Potential issue | 🟡 MinorOpen-circuit message reports base cooldown, not backoff cooldown.
After exponential backoff, the message still uses
self._config.cooldown_seconds, which can mislead operators and users fortrip_count > 1.🔧 Suggested fix
if state is CircuitBreakerState.OPEN: + pair = self._get_pair(delegator_id, delegatee_id) + cooldown_seconds = float(self._config.cooldown_seconds) + if pair is not None: + cooldown_seconds = self._compute_cooldown(pair.trip_count) logger.info( DELEGATION_LOOP_CIRCUIT_OPEN, @@ message=( f"Circuit breaker open for pair " f"({delegator_id!r}, {delegatee_id!r}); " - f"cooldown {self._config.cooldown_seconds}s" + f"cooldown {cooldown_seconds}s" ), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/communication/loop_prevention/circuit_breaker.py` around lines 183 - 186, The log message uses self._config.cooldown_seconds even after exponential backoff; update the code that constructs the "Circuit breaker open" message (the method handling trips in circuit_breaker.py) to compute the effective cooldown based on trip_count and the configured backoff (e.g., effective_cooldown = self._config.cooldown_seconds * (self._config.backoff_multiplier ** (trip_count - 1)) or by calling the existing helper that returns the current cooldown) and use that effective value in the message instead of self._config.cooldown_seconds so the reported cooldown reflects the backoff-adjusted timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/communication/config.py`:
- Around line 122-126: Add validation to the Pydantic model that declares the
min_interval_seconds Field so that if min_interval_seconds is not None while
frequency is not the event-triggered value (e.g., "event" or the enum member
used for event-triggered meetings), the model raises a validation error;
implement this via a `@root_validator` (or a validator that checks both fields) in
the same class to reject the configuration and include both field names
(min_interval_seconds and frequency) in the error message so users cannot
silently misconfigure non-trigger meetings.
- Around line 243-247: The config allows max_cooldown_seconds to be set below
cooldown_seconds; add a Pydantic validator to enforce max_cooldown_seconds >=
cooldown_seconds by checking values.get('cooldown_seconds') and raising a
ValueError if the invariant fails; place the validator in the same model that
defines cooldown_seconds and max_cooldown_seconds (use
`@validator`('max_cooldown_seconds') or a `@root_validator`) so any invalid config
is rejected at model validation time.
In `@src/synthorg/communication/conflict_resolution/debate_strategy.py`:
- Around line 106-114: The except block that calls _authority_fallback(conflict)
must also guard against ConflictHierarchyError coming from
_hierarchy_tiebreak()/get_ancestors(); update the handler so that if
_authority_fallback raises ConflictHierarchyError you catch it and instead call
the deterministic fallback pick_highest_seniority(conflict, hierarchy=None)
(returning winning_agent_id and reasoning), and log the fallback decision;
ensure ConflictHierarchyError is imported or referenced correctly and keep the
original logger.exception for the initial evaluator failure.
In `@src/synthorg/communication/loop_prevention/circuit_breaker.py`:
- Around line 272-294: In persist_dirty(), do not clear self._dirty before
attempting saves; instead iterate over a snapshot (e.g., dirty =
set(self._dirty)) and only remove keys from self._dirty after a successful save
so failed saves are left in the dirty set for retry; on each key use self._pairs
to build the CircuitBreakerStateRecord and call self._state_repo.save, catching
non-fatal exceptions (but re-raising MemoryError/RecursionError) and logging
failures without removing the key from self._dirty so transient DB errors don't
drop state.
In `@src/synthorg/communication/meeting/scheduler.py`:
- Around line 201-203: The current use of self._last_triggered.get(mt.name, 0.0)
applies the cooldown when there is no prior entry and can incorrectly skip the
first trigger if now < mt.min_interval_seconds; change the logic to detect
absence instead of using 0.0 (e.g., last = self._last_triggered.get(mt.name);
then only apply the cooldown check if last is not None: if last is not None and
(now - last) < mt.min_interval_seconds: ...), and ensure that when a trigger
actually fires you store self._last_triggered[mt.name] = now so future checks
work correctly.
- Around line 196-225: The cooldown check in trigger_event is race-prone because
multiple concurrent callers can build eligible before any updates to
self._last_triggered; wrap the cooldown filter and the subsequent update of
self._last_triggered in a short mutex to serialize the check-and-set.
Specifically, add a lock (e.g., self._cooldown_lock) and perform the loop that
computes eligible and the for-loop that sets self._last_triggered[mt.name] = now
while holding that lock so that computing now via self._clock(), comparing
against mt.min_interval_seconds/last, logging MEETING_EVENT_COOLDOWN_SKIPPED,
and marking last-triggered are atomic relative to other trigger_event callers.
Ensure the lock is only held for that minimal critical section to avoid long
blocking of meeting execution.
In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py`:
- Around line 48-49: Replace the parenthesized tuple except syntax used in the
three exception handlers that read "except (sqlite3.Error, aiosqlite.Error) as
exc" with the PEP 758 comma-separated form "except sqlite3.Error,
aiosqlite.Error as exc" in all occurrences (the three except clauses currently
at lines 48, 74 and 110) so the handlers in this module use the project-required
"except A, B:" style when catching sqlite3.Error and aiosqlite.Error.
In `@tests/unit/api/fakes_backend.py`:
- Around line 146-156: The in-memory repo currently types records as Any; change
FakeCircuitBreakerStateRepository to use the concrete CircuitBreakerStateRecord
type: update the internal store to self._store: dict[tuple[str, str],
CircuitBreakerStateRecord], change save(self, record: CircuitBreakerStateRecord)
-> None and load_all(self) -> tuple[CircuitBreakerStateRecord, ...], and import
CircuitBreakerStateRecord at top of the file so tests use a strongly-typed fake
and catch contract drift early.
In `@tests/unit/communication/conflict_resolution/test_debate_strategy.py`:
- Around line 379-434: Add tests that exercise DebateResolver.resolve() when the
judge evaluator raises the system exceptions that must be re-raised: create two
new async tests in TestDebateResolverEvaluatorFailure that use a judge which
raises MemoryError and another which raises RecursionError (analogous to
RaisingJudgeEvaluator), call resolver.resolve(conflict) and assert that the
respective exception is propagated via pytest.raises(MemoryError) and
pytest.raises(RecursionError); reference DebateResolver.resolve,
DebateConfig(judge="shared_manager"), and the RaisingJudgeEvaluator-style
classes so the tests mirror the existing Exception cases rather than expecting
fallback behavior or logging.
---
Outside diff comments:
In `@src/synthorg/communication/loop_prevention/circuit_breaker.py`:
- Around line 183-186: The log message uses self._config.cooldown_seconds even
after exponential backoff; update the code that constructs the "Circuit breaker
open" message (the method handling trips in circuit_breaker.py) to compute the
effective cooldown based on trip_count and the configured backoff (e.g.,
effective_cooldown = self._config.cooldown_seconds *
(self._config.backoff_multiplier ** (trip_count - 1)) or by calling the existing
helper that returns the current cooldown) and use that effective value in the
message instead of self._config.cooldown_seconds so the reported cooldown
reflects the backoff-adjusted timeout.
In `@src/synthorg/communication/meeting/config.py`:
- Around line 82-111: The class docstring for MeetingProtocolConfig is missing
the max_tasks_per_meeting attribute; update the Attributes section of the
MeetingProtocolConfig docstring to include a brief entry for
max_tasks_per_meeting describing it as an optional int (minimum 1) that limits
how many tasks can be auto-created from a single meeting's action items,
matching the Field description used in the max_tasks_per_meeting model field.
In `@src/synthorg/persistence/protocol.py`:
- Around line 66-96: The class-level docstring for the persistence protocol
omits the newly added attribute circuit_breaker_state; update the Attributes
section of the Protocol class docstring to include a line like
"circuit_breaker_state: Repository for CircuitBreakerState persistence." so the
docstring lists this repository alongside tasks, cost_records, messages, etc.;
locate the Protocol (or equivalent) class docstring in
src/synthorg/persistence/protocol.py and add the circuit_breaker_state entry
consistent with the existing wording and formatting.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1eb9fa89-9b9a-4014-8eb8-0bf1c1148034
📒 Files selected for processing (24)
src/synthorg/communication/config.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/communication/loop_prevention/guard.pysrc/synthorg/communication/meeting/config.pysrc/synthorg/communication/meeting/orchestrator.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/observability/events/conflict.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/circuit_breaker_repo.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/api/fakes_backend.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/communication/loop_prevention/test_circuit_breaker.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/communication/meeting/test_scheduler.pytests/unit/persistence/sqlite/test_circuit_breaker_repo.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/test_protocol.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Agent
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations—Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax without parentheses (PEP 758 except syntax)—ruff enforces this on Python 3.14
All public functions require type hints; mypy strict mode is enforced
Google-style docstrings are required on public classes and functions; enforced by ruff D rules
Create new objects for immutability, never mutate existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Line length must be 88 characters (enforced by ruff)
Files:
src/synthorg/observability/events/conflict.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/test_protocol.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/protocol.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/config.pytests/unit/communication/meeting/test_orchestrator.pysrc/synthorg/communication/meeting/orchestrator.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/persistence/sqlite/test_circuit_breaker_repo.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/config.pytests/unit/communication/loop_prevention/test_circuit_breaker.pytests/unit/api/fakes_backend.pytests/unit/communication/meeting/test_scheduler.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/persistence/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/guard.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields—including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants—instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Functions must be fewer than 50 lines; files must be fewer than 800 lines
Handle errors explicitly, never silently swallow them
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool, etc.); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured logging:logger.info(EVENT, key=value)—neverlogger.info("msg %s", val)
All error paths must log at WARNING ...
Files:
src/synthorg/observability/events/conflict.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/protocol.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/config.pysrc/synthorg/communication/meeting/orchestrator.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/config.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/persistence/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/guard.py
!(src/synthorg/observability/{setup,sinks,syslog_handler,http_handler}.py)**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
import logging/logging.getLogger()/print()in application code (exception:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdliblogging)
Files:
src/synthorg/observability/events/conflict.pytests/unit/persistence/sqlite/test_migrations.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/persistence/test_protocol.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/protocol.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/config.pytests/unit/communication/meeting/test_orchestrator.pysrc/synthorg/communication/meeting/orchestrator.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/persistence/sqlite/test_circuit_breaker_repo.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/config.pytests/unit/communication/loop_prevention/test_circuit_breaker.pytests/unit/api/fakes_backend.pytests/unit/communication/meeting/test_scheduler.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/persistence/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/guard.py
!(docs/design/operations.md|.claude/**/*|src/synthorg/providers/presets.py)**/*.{py,md}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Vendor names may only appear in: (1) Operations design page, (2).claude/files, (3) third-party imports, (4)src/synthorg/providers/presets.pywhich is user-facing runtime data. Tests must usetest-provider,test-small-001, etc.
Files:
src/synthorg/observability/events/conflict.pytests/unit/persistence/sqlite/test_migrations.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/persistence/test_protocol.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/protocol.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/config.pytests/unit/communication/meeting/test_orchestrator.pysrc/synthorg/communication/meeting/orchestrator.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/persistence/sqlite/test_circuit_breaker_repo.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/config.pytests/unit/communication/loop_prevention/test_circuit_breaker.pytests/unit/api/fakes_backend.pytests/unit/communication/meeting/test_scheduler.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/persistence/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/guard.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/observability/events/conflict.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/protocol.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/config.pysrc/synthorg/communication/meeting/orchestrator.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/config.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/persistence/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/guard.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test classification
30-second timeout per test (global inpyproject.toml); do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing with@given+@settingsdecorators; Hypothesis profiles configured intests/conftest.py:ci(deterministic,max_examples=10+derandomize=True),dev(1000 examples),fuzz(10,000 examples, no deadline),extreme(500,000 examples, no deadline). Controlled viaHYPOTHESIS_PROFILEenv var.
NEVER skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins; for tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/test_protocol.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/persistence/sqlite/test_circuit_breaker_repo.pytests/unit/communication/loop_prevention/test_circuit_breaker.pytests/unit/api/fakes_backend.pytests/unit/communication/meeting/test_scheduler.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/test_protocol.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/persistence/sqlite/test_circuit_breaker_repo.pytests/unit/communication/loop_prevention/test_circuit_breaker.pytests/unit/api/fakes_backend.pytests/unit/communication/meeting/test_scheduler.py
🧠 Learnings (20)
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/conflict.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`, etc.); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/conflict.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/observability/events/delegation.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/conflict.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability includes structured logging via `get_logger(__name__)`, correlation tracking, and log sinks.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/persistence/circuit_breaker_repo.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/persistence/circuit_breaker_repo.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/persistence/protocol.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/observability/events/meeting.pysrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/meeting.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
tests/unit/communication/meeting/test_orchestrator.py
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
tests/unit/communication/meeting/test_orchestrator.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/communication/loop_prevention/test_circuit_breaker.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/persistence.py
🔇 Additional comments (20)
tests/unit/persistence/sqlite/test_migrations.py (1)
45-46: LGTM!The expected tables set correctly includes the new
circuit_breaker_statetable, consistent with the schema.sql addition.tests/unit/persistence/test_protocol.py (1)
580-583: LGTM!The new
circuit_breaker_stateproperty follows the established pattern in this file for minimal@runtime_checkableprotocol conformance (returningobject()withAnytype). The actual functional fake repository is intests/unit/api/fakes_backend.py.src/synthorg/persistence/sqlite/schema.sql (1)
604-611: LGTM!The schema is well-designed with appropriate CHECK constraints for non-empty keys and non-negative counters. The composite primary key on
(pair_key_a, pair_key_b)provides efficient lookups without needing additional indexes.src/synthorg/persistence/protocol.py (1)
271-275: LGTM!The new protocol property follows the established pattern and is properly typed.
src/synthorg/observability/events/meeting.py (1)
56-59: LGTM!The new event constants follow established naming conventions and are appropriately placed.
MEETING_EVENT_COOLDOWN_SKIPPEDfits naturally in the scheduler section, andMEETING_TASKS_CAPPEDhas clear categorization under task capping.src/synthorg/observability/events/delegation.py (1)
21-24: LGTM!The new circuit breaker event constants are appropriately named and placed in the "Loop prevention" section alongside related circuit breaker events.
tests/unit/communication/meeting/test_orchestrator.py (2)
528-577: LGTM!Good test coverage for the task cap feature. The test correctly verifies that only the first N action items are processed when
max_tasks_per_meetingis set, and validates the exact ordering behavior.
579-626: LGTM!Complements the capped test by verifying that the default behavior (no cap) processes all action items.
src/synthorg/communication/meeting/orchestrator.py (1)
328-347: Task capping is implemented correctly and applied consistently.The capped list is used for both task creation and aggregate counts, and the cap behavior is explicitly observable through
MEETING_TASKS_CAPPED.src/synthorg/persistence/sqlite/backend.py (1)
39-41: Circuit-breaker repository wiring is complete and consistent.Initialization, teardown, repository creation, and guarded property exposure are all aligned with the backend’s existing repository lifecycle pattern.
Also applies to: 146-147, 175-176, 278-280, 625-635
tests/unit/persistence/sqlite/test_circuit_breaker_repo.py (1)
14-106: Repository test coverage is solid for core contract behavior.The suite validates upsert semantics, nullable
opened_at, delete return semantics, and empty-state loading.tests/unit/api/fakes_backend.py (1)
234-235: Fake backend exposure forcircuit_breaker_stateis correctly wired.Also applies to: 375-377
src/synthorg/communication/loop_prevention/guard.py (2)
54-68: Repository injection into the guard is clean and correctly plumbed.
162-174:load_state()/persist()wrappers are concise and correctly delegated.tests/unit/communication/loop_prevention/test_circuit_breaker.py (1)
117-326: Backoff and persistence behavior is tested with strong deterministic coverage.The added cases exercise cap behavior, trip-count carryover, dirty tracking, and repository round-trip restoration using a mocked clock/repo.
src/synthorg/observability/events/persistence.py (1)
336-348: New persistence event constants are well-named and domain-consistent.tests/unit/communication/meeting/test_scheduler.py (1)
415-538: Good coverage for cooldown behavior.The new deterministic clock-based tests exercise the key cooldown paths and improve regression safety for the scheduler changes.
src/synthorg/persistence/circuit_breaker_repo.py (1)
10-31: Model + protocol design looks solid.
CircuitBreakerStateRecordandCircuitBreakerStateRepositoryare well-typed and align with the persistence abstraction cleanly.Also applies to: 33-71
src/synthorg/observability/events/conflict.py (1)
19-19: Dedicated event constant looks good.This keeps evaluator failures observable without overloading the generic conflict strategy error event.
tests/unit/communication/conflict_resolution/test_debate_strategy.py (1)
44-57: Useful failure-injection test double.Making the raised error configurable should make the evaluator-failure suite easy to extend as more branches are added.
src/synthorg/communication/conflict_resolution/debate_strategy.py
Outdated
Show resolved
Hide resolved
| except (sqlite3.Error, aiosqlite.Error) as exc: | ||
| msg = ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify tuple-style multi-except usage in this file.
rg -nP 'except\s*\([^)]*,[^)]*\)\s+as\s+\w+' src/synthorg/persistence/sqlite/circuit_breaker_repo.pyRepository: Aureliolo/synthorg
Length of output: 239
Use Python 3.14 PEP 758 except syntax: except A, B: without parentheses.
Lines 48, 74, and 110 use parenthesized tuple syntax for multiple exceptions. Replace with comma-separated form per project policy.
Suggested fix
- except (sqlite3.Error, aiosqlite.Error) as exc:
+ except sqlite3.Error, aiosqlite.Error as exc:Apply to lines 48, 74, and 110.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py` around lines 48 -
49, Replace the parenthesized tuple except syntax used in the three exception
handlers that read "except (sqlite3.Error, aiosqlite.Error) as exc" with the PEP
758 comma-separated form "except sqlite3.Error, aiosqlite.Error as exc" in all
occurrences (the three except clauses currently at lines 48, 74 and 110) so the
handlers in this module use the project-required "except A, B:" style when
catching sqlite3.Error and aiosqlite.Error.
| class FakeCircuitBreakerStateRepository: | ||
| """In-memory circuit breaker state repository for tests.""" | ||
|
|
||
| def __init__(self) -> None: | ||
| self._store: dict[tuple[str, str], Any] = {} | ||
|
|
||
| async def save(self, record: Any) -> None: | ||
| self._store[(record.pair_key_a, record.pair_key_b)] = record | ||
|
|
||
| async def load_all(self) -> tuple[Any, ...]: | ||
| return tuple(self._store.values()) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer a typed fake record over Any in the in-memory repo.
Using CircuitBreakerStateRecord here will make test doubles stricter and catch contract drift earlier.
Proposed refactor
+from synthorg.persistence.circuit_breaker_repo import CircuitBreakerStateRecord
@@
class FakeCircuitBreakerStateRepository:
@@
def __init__(self) -> None:
- self._store: dict[tuple[str, str], Any] = {}
+ self._store: dict[tuple[str, str], CircuitBreakerStateRecord] = {}
@@
- async def save(self, record: Any) -> None:
+ async def save(self, record: CircuitBreakerStateRecord) -> None:
self._store[(record.pair_key_a, record.pair_key_b)] = record
@@
- async def load_all(self) -> tuple[Any, ...]:
+ async def load_all(self) -> tuple[CircuitBreakerStateRecord, ...]:
return tuple(self._store.values())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/fakes_backend.py` around lines 146 - 156, The in-memory repo
currently types records as Any; change FakeCircuitBreakerStateRepository to use
the concrete CircuitBreakerStateRecord type: update the internal store to
self._store: dict[tuple[str, str], CircuitBreakerStateRecord], change save(self,
record: CircuitBreakerStateRecord) -> None and load_all(self) ->
tuple[CircuitBreakerStateRecord, ...], and import CircuitBreakerStateRecord at
top of the file so tests use a strongly-typed fake and catch contract drift
early.
…t, Gemini Source fixes (items 1-14): - persist_dirty() retains failed keys in dirty set for retry (#1) - Add asyncio.Lock to scheduler.trigger_event() cooldown check (#2) - Add max_cooldown_seconds >= cooldown_seconds validator (#3) - Use None sentinel for first-trigger detection in cooldown (#4) - Cap _compute_cooldown exponent at 63 to prevent DoS (#5) - Add write_lock to SQLiteCircuitBreakerStateRepository (#6) - Guard _authority_fallback against ConflictHierarchyError (#7) - Add _rollback_quietly to circuit breaker repo save/delete (#8) - Raise QueryError on ValidationError in load_all (#9) - Update docs/design/communication.md for implemented features (#10) - Add 11 new event constants to CLAUDE.md logging section (#11) - Add min_interval_seconds requires trigger validator (#12) - PEP 758 parens required with as-binding (item #13 invalid) (#13) - Fix check() message to show actual backoff cooldown (#14) Doc/test fixes (items 15-18): - Add max_tasks_per_meeting to MeetingProtocolConfig docstring (#15) - Add circuit_breaker_state to PersistenceBackend docstring (#16) - Type FakeCircuitBreakerStateRepository with concrete record (#17) - Add MemoryError/RecursionError reraise tests for debate (#18) CI fix: - Suppress aiosqlite worker thread cleanup warning in pyproject.toml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py`:
- Around line 44-53: The helper _rollback_quietly currently catches all
Exception and mislabels rollback failures; change its signature to accept an
event parameter (e.g., event) and only swallow non-system exceptions by
re-raising system errors (MemoryError and RecursionError) instead of catching
them, and log the provided event constant when warning; update callers save() to
call _rollback_quietly(event=PERSISTENCE_CIRCUIT_BREAKER_SAVE_FAILED) and
delete() to call
_rollback_quietLY(event=PERSISTENCE_CIRCUIT_BREAKER_DELETE_FAILED) so the
correct event is used in the logger and system-level failures propagate.
In `@tests/unit/communication/conflict_resolution/test_debate_strategy.py`:
- Around line 379-464: Add a test that triggers the ConflictHierarchyError
branch by using a HierarchyResolver stub that raises ConflictHierarchyError
during tiebreak and a conflict where positions have equal seniority (so the
hierarchy tiebreak is invoked); instantiate DebateResolver with that hierarchy
and DebateConfig(judge="shared_manager"), call DebateResolver.resolve(conflict)
and assert the resolver falls back to authority (winning_agent_id matches
authority decision), resolution.outcome ==
ConflictResolutionOutcome.RESOLVED_BY_DEBATE, resolution.reasoning mentions
“fallback” (or hierarchy fallback), and that the hierarchy-failure event is
logged; reference DebateResolver.resolve, ConflictHierarchyError, and
DebateConfig to locate where the behavior should be exercised.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f325ced-53d5-4633-8dc1-4b9143124e41
📒 Files selected for processing (14)
CLAUDE.mddocs/design/communication.mdpyproject.tomlsrc/synthorg/communication/config.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/loop_prevention/circuit_breaker.pysrc/synthorg/communication/meeting/config.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pytests/unit/api/fakes_backend.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/communication/conftest.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
No
from __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations.Use PEP 758 except syntax: use
except A, B:(no parentheses) for multi-exception handling -- ruff enforces this on Python 3.14.Type hints: all public functions must have type annotations, enforced by mypy strict mode.
Docstrings: Google style, required on public classes and functions (enforced by ruff D rules).
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,
BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.For dict/list fields in frozen Pydantic models, rely on
frozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using
model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.Models: use Pydantic v2 (
BaseModel,model_validator,computed_field,ConfigDict). Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.Models: use
@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens).Models: use
NotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.Async concurrency: prefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.Line length: 88 characters (enforced by ruff).
Functions: keep under 50 lines, files under 800 line...
Files:
src/synthorg/persistence/protocol.pysrc/synthorg/communication/meeting/config.pytests/unit/communication/conftest.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/communication/conflict_resolution/test_debate_strategy.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/config.pytests/unit/api/fakes_backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Every module with business logic MUST have:
from synthorg.observability import get_loggerthenlogger = get_logger(__name__).Never use
import logging/logging.getLogger()/print()in application code (exception:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlib logging and print for handler construction, bootstrap, and error reporting code).Variable name for logger: always
logger(not_logger, notlog).Event names: always use constants from the domain-specific module under
synthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool, etc.). Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.Structured logging: always use
logger.info(EVENT, key=value)-- neverlogger.info("msg %s", val).All error paths must log at WARNING or ERROR with context before raising.
All state transitions must log at INFO level.
DEBUG logging for object creation, internal flow, and entry/exit of key functions.
Pure data models, enums, and re-exports do NOT need logging.
Files:
src/synthorg/persistence/protocol.pysrc/synthorg/communication/meeting/config.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/config.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/persistence/protocol.pysrc/synthorg/communication/meeting/config.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/config.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/meeting/scheduler.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Test markers: use
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto categorize tests.Coverage: 80% minimum (enforced in CI).
Async:
asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded.Timeout: 30 seconds per test (global in
pyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed).Parallelism: include
-n 8when running pytest locally, never run tests sequentially. Use@pytest.mark.parametrizefor testing similar cases.Property-based testing with Hypothesis: use
@given+@settings. Hypothesis profiles intests/conftest.py:ci(deterministic,max_examples=10+derandomize=True),dev(1000 examples),fuzz(10,000 examples, no deadline),extreme(500,000 examples, no deadline). Controlled viaHYPOTHESIS_PROFILEenv var.Hypothesis workflow: CI runs 10 deterministic examples per property test. Random fuzzing runs locally with
HYPOTHESIS_PROFILE=dev(1000 examples) orHYPOTHESIS_PROFILE=fuzz(10,000 examples). When Hypothesis finds a failure, fix the underlying bug and add an explicit@example(...)decorator to permanently cover the case in CI.Never skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mock
time.monotonic()andasyncio.sleep()to make them deterministic. For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/communication/conftest.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/api/fakes_backend.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/communication/conftest.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/api/fakes_backend.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue. DESIGN_SPEC.md is a pointer file linking to the 12 design pages.
Files:
CLAUDE.mddocs/design/communication.md
**/*.{md,txt}
📄 CodeRabbit inference engine (CLAUDE.md)
Git Commits: use
<type>: <description>format -- types: feat, fix, refactor, docs, test, chore, perf, ci. Enforced by commitizen (commit-msg hook).
Files:
CLAUDE.mddocs/design/communication.md
🧠 Learnings (40)
📓 Common learnings
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: When implementation deviates from the spec (better approach found, scope evolved, etc.), alert the user and explain why -- user decides whether to proceed or update the spec. Do NOT silently diverge.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: At every phase of planning and implementation, be critical -- actively look for ways to improve the design in the spirit of what we're building (robustness, correctness, simplicity, future-proofing where it's free). Surface improvements as suggestions, not silent changes.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Prioritize issues by dependency order, not priority labels -- unblocked dependencies come first.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Shell Usage: NEVER use `cd` in Bash commands -- the working directory is already set to the project root. Use absolute paths or run commands directly. Exception: `bash -c "cd <dir> && <cmd>"` is safe (runs in a child process). Use this for tools without a `-C` flag.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: NEVER use Bash to write or modify files -- use the Write or Edit tools. Do not use `cat >`, `cat << EOF`, `echo >`, `echo >>`, `sed -i`, `python -c "open(...).write(...)", or `tee` to create or modify files. Read-only/inspection uses like piping to stdout are fine.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Signed commits: required on `main` via branch protection -- all commits must be GPG/SSH signed.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Branches: use `<type>/<slug>` naming from main.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Pre-commit hooks enforce: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint (Dockerfile), golangci-lint + go vet (CLI), no-em-dashes, no-redundant-timeout, eslint-web (zero warnings, conditional on `web/src/**/*.{ts,tsx}`).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Pre-push hooks: mypy type-check (affected modules only) + pytest unit tests (affected modules only) + golangci-lint + go vet + go test (CLI, conditional on `cli/**/*.go`) + eslint-web (web dashboard). Foundational module changes (core, config, observability) or conftest changes trigger full runs.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Pre-commit.ci: autoupdate disabled (`autoupdate_schedule: never`) -- Dependabot owns hook version bumps via `pre-commit` ecosystem.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: GitHub issue queries: use `gh issue list` via Bash (not MCP tools) -- MCP `list_issues` has unreliable field data.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Merge strategy: squash merge -- PR body becomes the squash commit message on main. Trailers (e.g. `Release-As`, `Closes `#N``) must be in the PR body to land in the final commit.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: PR issue references: preserve existing `Closes `#NNN`` references -- never remove unless explicitly asked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: After finishing an issue implementation: always create a feature branch (`<type>/<slug>`), commit, and push -- do NOT create a PR automatically. Do NOT leave work uncommitted on main.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: NEVER create a PR directly -- `gh pr create` is blocked by hookify. ALWAYS use `/pre-pr-review` to create PRs -- it runs automated checks + review agents + fixes before creating the PR. For trivial/docs-only changes: `/pre-pr-review quick` skips agents but still runs automated checks.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: After the PR exists, use `/aurelio-review-pr` to handle external reviewer feedback. The `/commit-push-pr` command is effectively blocked (it calls `gh pr create` internally).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:13:20.062Z
Learning: Fix everything valid when review agents find issues -- including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes. No deferring, no "out of scope" skipping.
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/persistence/protocol.py
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: When Hypothesis finds a failure, it is a real bug; the shrunk example is saved to `~/.synthorg/hypothesis-examples/` but is NOT replayed automatically. Fix the underlying bug and add an explicit `example(...)` decorator to the test so the case is permanently covered in CI.
Applied to files:
tests/unit/communication/conflict_resolution/test_debate_strategy.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-04-07T22:12:30.141Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.141Z
Learning: Applies to **/*.py : Use `except A, B:` syntax without parentheses (PEP 758 except syntax)—ruff enforces this on Python 3.14
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pysrc/synthorg/persistence/sqlite/circuit_breaker_repo.pysrc/synthorg/communication/loop_prevention/circuit_breaker.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability includes structured logging via `get_logger(__name__)`, correlation tracking, and log sinks.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.md
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/communication/conflict_resolution/debate_strategy.pyCLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`, etc.); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
CLAUDE.mdsrc/synthorg/communication/meeting/scheduler.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use import logging / logging.getLogger() / print() in application code.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: Applies to src/synthorg/**/*.py : Always use structured logging: `logger.info(EVENT, key=value)`—never `logger.info("msg %s", val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-02T12:07:44.443Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T12:07:44.443Z
Learning: Applies to src/synthorg/**/*.py : Always use structured logging: `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/communication/**/*.py : Communication package (communication/): message bus, dispatcher, messenger, channels, delegation, loop prevention, conflict resolution; meeting/ subpackage for meeting protocol (round-robin, position papers, structured phases), scheduler (frequency, participant resolver), orchestrator
Applied to files:
docs/design/communication.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/communication.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
docs/design/communication.md
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow them
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/communication/meeting/scheduler.py
🔇 Additional comments (15)
pyproject.toml (1)
260-260: This filter targets an external library thread that does not exist in the synthorg codebase. The only worker thread locally created islog-http-flusherinHttpBatchHandler, which already has proper lifecycle management withjoin(timeout=...)andis_alive()checks. The pattern.*_connection_worker_threadsuppresses warnings from an external dependency (likely urllib or similar), not from application code. This is a defensive test filter, not a masking of real lifecycle bugs in the hardening work.> Likely an incorrect or invalid review comment.src/synthorg/persistence/sqlite/circuit_breaker_repo.py (1)
69-69: Use the repo’s PEP 758 multi-except form consistently.These three handlers still use tuple-style
except (...) as exc; the project’s Python 3.14 style here is the comma form.🔧 Apply the same change in all three handlers
- except (sqlite3.Error, aiosqlite.Error) as exc: + except sqlite3.Error, aiosqlite.Error as exc:#!/bin/bash # Verify any remaining tuple-style multi-except handlers in this file. rg -nP 'except\s*\([^)]*,[^)]*\)\s+as\s+\w+' src/synthorg/persistence/sqlite/circuit_breaker_repo.pyAs per coding guidelines, "Use PEP 758 except syntax: use
except A, B:(no parentheses) for multi-exception handling -- ruff enforces this on Python 3.14."Also applies to: 96-96, 138-138
src/synthorg/communication/conflict_resolution/debate_strategy.py (1)
104-134: Evaluator failure now degrades cleanly.The extra
ConflictHierarchyErrorguard pluspick_highest_seniority(..., hierarchy=None)closes the last propagation path here while still lettingMemoryError/RecursionErrorescape.src/synthorg/communication/config.py (1)
137-139: Both new config invariants are enforced at the boundary.Rejecting
min_interval_secondson non-trigger meetings and rejectingmax_cooldown_seconds < cooldown_secondskeeps both misconfigurations out of runtime code.As per coding guidelines, "Validate at system boundaries (user input, external APIs, config files)".
Also applies to: 252-258
src/synthorg/communication/meeting/scheduler.py (1)
198-232: Cooldown check-and-set is now properly atomic.Keeping the eligibility scan and
_last_triggeredupdate under a short lock closes the double-trigger race, and thelast is not Nonesentinel preserves first-trigger behavior.tests/unit/api/fakes_backend.py (2)
156-160: Use typed signatures instead ofAnyfor protocol compliance.The
_storeis correctly typed asdict[tuple[str, str], CircuitBreakerStateRecord], but the method signatures still useAny. This weakens type checking and was already flagged in a previous review.
238-238: LGTM!The fake backend correctly instantiates and exposes the circuit breaker state repository, aligning with the production
PersistenceBackendprotocol.Also applies to: 379-381
src/synthorg/persistence/sqlite/backend.py (1)
39-41: LGTM!The circuit breaker state repository integration follows the established patterns:
- Import, field initialization, and cleanup in
_clear_state()are consistent with other repositories- Repository instantiation correctly injects the shared write lock for transaction safety
- Property accessor uses
_require_connectedwith appropriate docstringAlso applies to: 146-146, 175-175, 278-281, 626-636
src/synthorg/communication/loop_prevention/circuit_breaker.py (4)
101-119: LGTM!The exponential backoff implementation is well-designed:
- Exponent capped at 63 prevents big-integer DoS from corrupted
trip_count- Formula
base * 2^(trip_count - 1)correctly yields base cooldown on first trip- Final result capped at
max_cooldown_seconds
241-270: LGTM!The
load_state()implementation correctly:
- Re-raises
MemoryError/RecursionErrorimmediately (system errors)- Logs with context before re-raising other exceptions, letting callers decide recovery
- Restores all pair state fields including
trip_count
272-306: LGTM! Past review concern has been addressed.The
persist_dirty()implementation now correctly handles transient failures:
- Takes a snapshot of dirty keys (line 282)
- Discards keys only on successful save (line 297)
- Failed keys remain in
_dirtyfor retry on next persist cycle (lines 300-306)
147-158: Consider logging at DEBUG level when cooldown expires.The reset path logs at INFO (line 152), which is appropriate for state transitions per guidelines. However, the log currently includes
cooldown_secondsandtrip_countwhich is good for observability.docs/design/communication.md (2)
466-470: LGTM!Documentation accurately describes the two meeting-task feedback loop guardrails matching the implementation.
491-498: LGTM!Circuit breaker documentation correctly captures:
- Exponential backoff formula:
base * 2^(trip_count - 1)- Default cap at 3600s
- Bounce count reset with trip count preservation
- SQLite persistence via
CircuitBreakerStateRepository- In-memory dedup/rate limiter (by design)
CLAUDE.md (1)
93-93: LGTM!The event constants list correctly includes all new observability events introduced in this PR, maintaining the established documentation pattern.
| async def _rollback_quietly(self) -> None: | ||
| """Roll back the current transaction, swallowing errors.""" | ||
| try: | ||
| await self._db.rollback() | ||
| except Exception: | ||
| logger.warning( | ||
| PERSISTENCE_CIRCUIT_BREAKER_SAVE_FAILED, | ||
| error="rollback failed", | ||
| exc_info=True, | ||
| ) |
There was a problem hiding this comment.
Rollback cleanup should not swallow system-level failures.
_rollback_quietly() catches every Exception, so a MemoryError/RecursionError during rollback gets downgraded into a warning and the caller continues toward QueryError. Because this helper is also used by delete(), rollback warnings on that path are additionally mislabeled as PERSISTENCE_CIRCUIT_BREAKER_SAVE_FAILED.
🛠️ Suggested change
- async def _rollback_quietly(self) -> None:
+ async def _rollback_quietly(
+ self,
+ *,
+ event: str,
+ **context: object,
+ ) -> None:
"""Roll back the current transaction, swallowing errors."""
try:
await self._db.rollback()
+ except MemoryError, RecursionError:
+ raise
except Exception:
logger.warning(
- PERSISTENCE_CIRCUIT_BREAKER_SAVE_FAILED,
- error="rollback failed",
+ event,
+ note="rollback failed",
exc_info=True,
+ **context,
)Pass event=PERSISTENCE_CIRCUIT_BREAKER_SAVE_FAILED from save() and event=PERSISTENCE_CIRCUIT_BREAKER_DELETE_FAILED from delete().
As per coding guidelines, "except builtins.MemoryError, RecursionError: raise pattern is intentional project convention for system-error propagation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py` around lines 44 -
53, The helper _rollback_quietly currently catches all Exception and mislabels
rollback failures; change its signature to accept an event parameter (e.g.,
event) and only swallow non-system exceptions by re-raising system errors
(MemoryError and RecursionError) instead of catching them, and log the provided
event constant when warning; update callers save() to call
_rollback_quietly(event=PERSISTENCE_CIRCUIT_BREAKER_SAVE_FAILED) and delete() to
call _rollback_quietLY(event=PERSISTENCE_CIRCUIT_BREAKER_DELETE_FAILED) so the
correct event is used in the logger and system-level failures propagate.
- _rollback_quietly() re-raises MemoryError/RecursionError and accepts event parameter to avoid mislabeling delete-path rollback failures - Add test for ConflictHierarchyError fallback branch in DebateResolver
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/synthorg/persistence/sqlite/circuit_breaker_repo.py (2)
71-71:⚠️ Potential issue | 🟠 MajorUse PEP 758 multi-except syntax in all three handlers.
Lines 71, 100, and 142 still use tuple-style
except (...) as exc. This codebase’s Python 3.14 convention isexcept sqlite3.Error, aiosqlite.Error as exc:.🧩 Proposed fix
- except (sqlite3.Error, aiosqlite.Error) as exc: + except sqlite3.Error, aiosqlite.Error as exc: ... - except (sqlite3.Error, aiosqlite.Error) as exc: + except sqlite3.Error, aiosqlite.Error as exc: ... - except (sqlite3.Error, aiosqlite.Error) as exc: + except sqlite3.Error, aiosqlite.Error as exc:#!/bin/bash # Verify no tuple-style multi-except handlers remain in this module. rg -nP 'except\s*\([^)]*,[^)]*\)\s+as\s+\w+' src/synthorg/persistence/sqlite/circuit_breaker_repo.pyExpected: no output after the fix.
As per coding guidelines, "Use PEP 758 except syntax:
except A, B:(no parentheses) -- ruff enforces this on Python 3.14."Also applies to: 100-100, 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py` at line 71, Replace tuple-style multi-except handlers in src/synthorg/persistence/sqlite/circuit_breaker_repo.py with PEP 758 syntax: change occurrences like "except (sqlite3.Error, aiosqlite.Error) as exc:" to "except sqlite3.Error, aiosqlite.Error as exc:" in all three handlers in this module (the three except blocks currently catching sqlite3.Error and aiosqlite.Error). Update each handler (the circuit breaker repo exception handlers) accordingly and re-run the provided grep/rg check to verify no tuple-style multi-excepts remain.
94-99:⚠️ Potential issue | 🟠 MajorAcquire the write lock before reading in
load_all().The
save()anddelete()methods serialize their database operations usingself._write_lock, butload_all()reads from the shared connection without holding that lock. With a single shared connection, a concurrentSELECTcan execute after one coroutine's write but before itscommit(), returning rows that may later be rolled back.Wrap the
SELECTandfetchall()calls withasync with self._write_lock:to ensure reads observe only committed state.Additionally, lines 100, 71, and 142 use parenthesized multi-except syntax
except (Error, Error) as exc:, which should use PEP 758 formexcept Error, Error as exc:(comma-separated, no parentheses) as enforced by ruff on Python 3.14.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py` around lines 94 - 99, Acquire the write lock around the read in load_all(): wrap the SELECT and fetchall() calls in an async with self._write_lock: block so load_all() observes only committed state (mirroring the serialization used by save() and delete()). Also update the multi-except clauses that currently use parenthesized syntax (e.g., except (ErrorA, ErrorB) as exc:) to the PEP 758 comma form (except ErrorA, ErrorB as exc:) at the sites referenced (around load_all(), and the except blocks near save() and delete()) so the exception handling matches ruff/Python 3.14 style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py`:
- Around line 86-90: The save/delete completed state transitions are currently
logged with logger.debug; change those logger.debug calls that emit
PERSISTENCE_CIRCUIT_BREAKER_SAVED and PERSISTENCE_CIRCUIT_BREAKER_DELETED to
logger.info so committed state transitions are visible in operational logs.
Locate the logger.debug invocation that passes PERSISTENCE_CIRCUIT_BREAKER_SAVED
with pair_key_a/pair_key_b (and the analogous logger.debug for
PERSISTENCE_CIRCUIT_BREAKER_DELETED around the later block) and replace debug
with info while keeping the same message keys and parameters.
In `@tests/unit/communication/conflict_resolution/test_debate_strategy.py`:
- Around line 410-434: The test test_evaluator_exception_logs_event should use
the pytest caplog fixture instead of capsys to reliably assert logging events:
replace the capsys parameter with caplog, call await resolver.resolve(conflict)
as before, then inspect caplog (e.g., caplog.text or iterate caplog.records and
check record.getMessage()) to assert that "conflict.debate.evaluator_failed" was
logged; update the test signature and assertion accordingly so it checks log
records emitted by DebateResolver.resolve and RaisingJudgeEvaluator rather than
stdout.
---
Duplicate comments:
In `@src/synthorg/persistence/sqlite/circuit_breaker_repo.py`:
- Line 71: Replace tuple-style multi-except handlers in
src/synthorg/persistence/sqlite/circuit_breaker_repo.py with PEP 758 syntax:
change occurrences like "except (sqlite3.Error, aiosqlite.Error) as exc:" to
"except sqlite3.Error, aiosqlite.Error as exc:" in all three handlers in this
module (the three except blocks currently catching sqlite3.Error and
aiosqlite.Error). Update each handler (the circuit breaker repo exception
handlers) accordingly and re-run the provided grep/rg check to verify no
tuple-style multi-excepts remain.
- Around line 94-99: Acquire the write lock around the read in load_all(): wrap
the SELECT and fetchall() calls in an async with self._write_lock: block so
load_all() observes only committed state (mirroring the serialization used by
save() and delete()). Also update the multi-except clauses that currently use
parenthesized syntax (e.g., except (ErrorA, ErrorB) as exc:) to the PEP 758
comma form (except ErrorA, ErrorB as exc:) at the sites referenced (around
load_all(), and the except blocks near save() and delete()) so the exception
handling matches ruff/Python 3.14 style.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c3e6983-77ab-42a7-a38e-903cc8e30194
📒 Files selected for processing (2)
src/synthorg/persistence/sqlite/circuit_breaker_repo.pytests/unit/communication/conflict_resolution/test_debate_strategy.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Do NOT use
from __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations.Use PEP 758 except syntax:
except A, B:(no parentheses) -- ruff enforces this on Python 3.14.Type hints: all public functions must have type hints, enforced by mypy strict mode.
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules).
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement.
Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves.
Never mix static config fields with mutable runtime fields in one Pydantic model.
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time.
Use
@computed_fieldfor derived values instead of storing + validating redundant fields (e.g. TokenUsage.total_tokens).Use NotBlankStr (from core.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.
Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Line length: 88 characters (ruff).
Functions: < 50 lines, files < 800 lines.
Errors: handle explicitly, never silently swallow.
Validate: at system boundaries (user input, external APIs, config files).
Never use
import logging/logging.getLogger()/print()in application code (exception: observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py may use stdlib logging and print...
Files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Every module with business logic MUST have:
from synthorg.observability import get_loggerthenlogger = get_logger(__name__).
Files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Vendor-agnostic code: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, or comments. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2) .claude/ skill/agent files, (3) third-party import paths/module names, (4) provider presets (src/synthorg/providers/presets.py).
Files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.pytests/unit/communication/conflict_resolution/test_debate_strategy.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use markers:
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow.Coverage: 80% minimum (enforced in CI).
Async: asyncio_mode = 'auto' -- no manual
@pytest.mark.asyncioneeded.Timeout: 30 seconds per test (global in pyproject.toml -- do not add per-file pytest.mark.timeout(30) markers; non-default overrides like timeout(60) are allowed).
Parallelism: pytest-xdist via
-n 8-- ALWAYS include-n 8when running pytest locally, never run tests sequentially.Parametrize: Prefer
@pytest.mark.parametrizefor testing similar cases.Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases. Use test-provider, test-small-001, etc. in tests.
Property-based testing: Python uses Hypothesis (
@given+@settings). Hypothesis profiles configured in tests/conftest.py: ci (deterministic, max_examples=10 + derandomize=True), dev (1000 examples), fuzz (10,000 examples, no deadline), extreme (500,000 examples, no deadline). Controlled via HYPOTHESIS_PROFILE env var.When Hypothesis finds a failure, it is a real bug -- the shrunk example is saved but is not replayed automatically. Do NOT just rerun and move on. Fix the underlying bug and add an explicit
@example(...) decorator to the test.Flaky tests: NEVER skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic. For tasks that must block indefinitely until cancelled, use asyncio.Event().wait() instead of asyncio.sleep(large_number).
Files:
tests/unit/communication/conflict_resolution/test_debate_strategy.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/communication/conflict_resolution/test_debate_strategy.py
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: At every phase of planning and implementation, be critical -- actively look for ways to improve the design in the spirit of what we're building (robustness, correctness, simplicity, future-proofing where it's free). Surface improvements as suggestions, not silent changes -- user decides.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: Prioritize issues by dependency order, not priority labels -- unblocked dependencies come first.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: ALWAYS use `/pre-pr-review` to create PRs -- gh pr create is blocked by hookify. For trivial/docs-only changes: `/pre-pr-review quick` skips agents but still runs automated checks. After the PR exists, use `/aurelio-review-pr` to handle external reviewer feedback.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: Fix everything valid when review agents find issues -- never skip. When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all. No deferring, no 'out of scope' skipping.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: After finishing an issue implementation: always create a feature branch (<type>/<slug>), commit, and push -- do NOT create a PR automatically. Do NOT leave work uncommitted on main -- branch, commit, push immediately after finishing.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: Signed commits: required on main via branch protection -- all commits must be GPG/SSH signed.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: Branches: <type>/<slug> from main.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: GitHub issue queries: use `gh issue list` via Bash (not MCP tools) -- MCP `list_issues` has unreliable field data.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: Merge strategy: squash merge -- PR body becomes the squash commit message on main. Trailers (e.g. Release-As, Closes `#N`) must be in the PR body to land in the final commit.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T06:59:29.069Z
Learning: PR issue references: preserve existing `Closes `#NNN`` references -- never remove unless explicitly asked.
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-04-07T22:12:30.141Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.141Z
Learning: Applies to **/*.py : Use `except A, B:` syntax without parentheses (PEP 758 except syntax)—ruff enforces this on Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow them
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/persistence/sqlite/circuit_breaker_repo.py
📚 Learning: 2026-04-07T22:12:30.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T22:12:30.142Z
Learning: When Hypothesis finds a failure, it is a real bug; the shrunk example is saved to `~/.synthorg/hypothesis-examples/` but is NOT replayed automatically. Fix the underlying bug and add an explicit `example(...)` decorator to the test so the case is permanently covered in CI.
Applied to files:
tests/unit/communication/conflict_resolution/test_debate_strategy.py
🔇 Additional comments (2)
tests/unit/communication/conflict_resolution/test_debate_strategy.py (2)
44-58: Good fault-injection helper for evaluator-failure paths.
RaisingJudgeEvaluatoris clean and purpose-built, and thecallstracking improves assertions in downstream tests.
436-501: Strong regression coverage for guarded exceptions and hierarchy fallback.The parametrized
MemoryError/RecursionErrorpropagation test and theConflictHierarchyErrorfallback test both align with the new resolver contract and reduce regression risk.
| async def test_evaluator_exception_logs_event( | ||
| self, | ||
| hierarchy: HierarchyResolver, | ||
| capsys: pytest.CaptureFixture[str], | ||
| ) -> None: | ||
| """Evaluator failure is logged with the dedicated event.""" | ||
| judge = RaisingJudgeEvaluator() | ||
| resolver = DebateResolver( | ||
| hierarchy=hierarchy, | ||
| config=DebateConfig(judge="shared_manager"), | ||
| judge_evaluator=judge, | ||
| ) | ||
| conflict = make_conflict( | ||
| positions=( | ||
| make_position(agent_id="sr_dev", level=SeniorityLevel.SENIOR), | ||
| make_position( | ||
| agent_id="jr_dev", | ||
| level=SeniorityLevel.JUNIOR, | ||
| position="Other", | ||
| ), | ||
| ), | ||
| ) | ||
| await resolver.resolve(conflict) | ||
| captured = capsys.readouterr() | ||
| assert "conflict.debate.evaluator_failed" in captured.out |
There was a problem hiding this comment.
Use caplog instead of capsys for log-event assertions.
Asserting captured.out is stream-dependent and can become flaky if logging sinks change. Prefer record-level assertions via caplog.
Suggested change
- async def test_evaluator_exception_logs_event(
- self,
- hierarchy: HierarchyResolver,
- capsys: pytest.CaptureFixture[str],
- ) -> None:
+ async def test_evaluator_exception_logs_event(
+ self,
+ hierarchy: HierarchyResolver,
+ caplog: pytest.LogCaptureFixture,
+ ) -> None:
@@
- await resolver.resolve(conflict)
- captured = capsys.readouterr()
- assert "conflict.debate.evaluator_failed" in captured.out
+ await resolver.resolve(conflict)
+ assert any(
+ "conflict.debate.evaluator_failed" in record.message
+ for record in caplog.records
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/communication/conflict_resolution/test_debate_strategy.py` around
lines 410 - 434, The test test_evaluator_exception_logs_event should use the
pytest caplog fixture instead of capsys to reliably assert logging events:
replace the capsys parameter with caplog, call await resolver.resolve(conflict)
as before, then inspect caplog (e.g., caplog.text or iterate caplog.records and
check record.getMessage()) to assert that "conflict.debate.evaluator_failed" was
logged; update the test signature and assertion accordingly so it checks log
records emitted by DebateResolver.resolve and RaisingJudgeEvaluator rather than
stdout.
Matches convention (CLAUDE.md: "All state transitions must log at INFO") and aligns with newer repos (artifact, project, user).
Summary
Three bug fixes for communication subsystem vulnerabilities identified in the multi-agent failure audit (
docs/research/multi-agent-failure-audit.md, #690):DebateResolver: fall back to AuthorityResolver when judge evaluator raises #1117 -- DebateResolver evaluator fallback: When the judge evaluator raises (provider error, timeout), fall back to authority-based resolution instead of propagating the exception. Adds
CONFLICT_DEBATE_EVALUATOR_FAILEDevent constant.Add cooldown guard to event-triggered meetings to prevent task/meeting feedback loops #1115 -- Meeting cooldown guard + task cap: Add
min_interval_secondstoMeetingTypeConfigfor per-type cooldown on event-triggered meetings. Addmax_tasks_per_meetingtoMeetingProtocolConfigto cap task creation from action items. Prevents unbounded meeting-task feedback loops.Harden delegation guard: exponential backoff on circuit breaker reset + persistent state #1116 -- Circuit breaker exponential backoff + SQLite persistence: Replace state eviction on cooldown expiry with bounce_count reset, preserving trip_count across resets. Cooldown formula:
base * 2^(trip_count - 1), capped atmax_cooldown_seconds. NewCircuitBreakerStateRepositoryprotocol + SQLite implementation with dirty-tracking for async persistence without cascading sync-to-async changes.Changed files (24)
Source (11 files):
src/synthorg/communication/config.py--min_interval_seconds,max_cooldown_secondsfieldssrc/synthorg/communication/conflict_resolution/debate_strategy.py-- evaluator fallbacksrc/synthorg/communication/loop_prevention/circuit_breaker.py-- exponential backoff + dirty-tracking persistencesrc/synthorg/communication/loop_prevention/guard.py-- pass-through repo + async load/persistsrc/synthorg/communication/meeting/config.py--max_tasks_per_meetingfieldsrc/synthorg/communication/meeting/orchestrator.py-- task cap enforcementsrc/synthorg/communication/meeting/scheduler.py-- cooldown guard with clock injectionsrc/synthorg/observability/events/{conflict,delegation,meeting,persistence}.py-- new event constantssrc/synthorg/persistence/circuit_breaker_repo.py-- NEW: protocol + modelsrc/synthorg/persistence/protocol.py+sqlite/backend.py-- wire new reposrc/synthorg/persistence/sqlite/circuit_breaker_repo.py-- NEW: SQLite implementationTests (8 files): 23 new tests covering all new code paths
Test plan
uv run python -m pytest tests/unit/communication/ tests/unit/persistence/ -n 8-- 1273 passeduv run python -m pytest tests/ -m unit -n 8-- 15562 passed, 5 skipped (symlinks on Windows)uv run mypy src/ tests/-- 0 errorsuv run ruff check src/ tests/-- 0 errorsReview coverage
Pre-reviewed by 7 agents (code-reviewer, silent-failure-hunter, conventions-enforcer, persistence-reviewer, async-concurrency-reviewer, issue-resolution-verifier, test-analyzer). 4 valid findings addressed in final commit.
Closes #1115
Closes #1116
Closes #1117