Skip to content

feat: throughput-adaptive and external-trigger ceremony scheduling strategies#1003

Merged
Aureliolo merged 10 commits intomainfrom
feat/ceremony-strategies-batch2
Apr 2, 2026
Merged

feat: throughput-adaptive and external-trigger ceremony scheduling strategies#1003
Aureliolo merged 10 commits intomainfrom
feat/ceremony-strategies-batch2

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Summary

Batch 2 of the pluggable ceremony scheduling system. Implements two new CeremonySchedulingStrategy protocol implementations:

  • ThroughputAdaptiveStrategy (feat: throughput-adaptive ceremony scheduling strategy #973): Ceremonies fire when rolling throughput rate drops or spikes beyond configured thresholds. Stateful -- tracks task completion timestamps in a rolling window, establishes baseline rate from the first full window (frozen per sprint). Default velocity calculator: TaskDrivenVelocityCalculator.

  • ExternalTriggerStrategy (feat: external-trigger ceremony scheduling strategy #974): Ceremonies fire on named external signals (webhooks, git events, MCP invocations). Stateful -- buffers received events via lifecycle hooks. Source registration is declarative (transport integration handled externally). Default velocity calculator: PointsPerSprintVelocityCalculator.

Changes

  • New: src/synthorg/engine/workflow/strategies/throughput_adaptive.py -- ThroughputAdaptiveStrategy implementation
  • New: src/synthorg/engine/workflow/strategies/external_trigger.py -- ExternalTriggerStrategy implementation
  • New: tests/unit/engine/workflow/strategies/test_throughput_adaptive.py -- 46 unit tests
  • New: tests/unit/engine/workflow/strategies/test_external_trigger.py -- 41 unit tests
  • Modified: src/synthorg/engine/workflow/strategies/__init__.py -- register both strategies
  • Modified: src/synthorg/engine/workflow/__init__.py -- export both strategies (also adds missing BudgetDrivenStrategy/EventDrivenStrategy exports)
  • Modified: src/synthorg/observability/events/workflow.py -- 8 new event constants
  • Modified: docs/design/ceremony-scheduling.md -- roadmap subsection, observability events table

Key Design Decisions

  • Baseline rate (throughput-adaptive): Established from the first measurement_window_tasks completions, then frozen for the sprint. A continuously rolling baseline would chase the current rate, defeating anomaly detection.
  • Blocked tasks: Increment an informational counter but do not affect the completion rate deque (blocked tasks already manifest as reduced throughput).
  • Per-ceremony config: on_drop (default True) / on_spike (default False) booleans control which anomaly types fire each ceremony.
  • External source registration: sources config is validated and stored, but actual webhook/git event transport integration is out of scope -- the on_external_event hook is the injection point.

Test Plan

  • 87 new unit tests covering protocol compliance, core evaluation (fire/no-fire conditions), sprint transitions, lifecycle hooks, state reset, config validation (happy + parametrized error cases), edge cases (zero time span, max received events, whitespace stripping, non-dict source filtering)
  • All 263 strategy tests pass, all 592 workflow tests pass, full suite 12420 pass
  • Pre-reviewed by 6 agents, 7 findings addressed

Closes #973
Closes #974

Copilot AI review requested due to automatic review settings April 2, 2026 16:35
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

Walkthrough

Adds two stateful ceremony scheduling strategies: ThroughputAdaptiveStrategy, which tracks task completions in a rolling window, establishes a baseline, and edge-triggers ceremonies on detected throughput drops or spikes; and ExternalTriggerStrategy, which validates and buffers external event names, fires ceremonies or triggers sprint transitions when configured events are observed, and manages per-sprint source registration. Exports both strategies from workflow packages, adds related workflow observability event-name constants, updates the design document to mark these strategies implemented, and adds unit tests covering both strategies.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding two new ceremony scheduling strategies (throughput-adaptive and external-trigger) to the pluggable workflow system.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, covering implementation of two new strategies, file changes, design decisions, and test coverage.
Linked Issues check ✅ Passed The PR implements all core objectives from #973 and #974: both strategies implement the CeremonySchedulingStrategy protocol with required lifecycle hooks, fire/transition logic, configuration validation, and default velocity calculators.
Out of Scope Changes check ✅ Passed All changes directly support the linked issue requirements. Updates to init.py files (re-exporting strategies) and observability constants are necessary supporting changes; documentation updates properly document the implementation.
Docstring Coverage ✅ Passed Docstring coverage is 64.17% which is sufficient. The required threshold is 40.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA eeb59f4.
Ensure 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 Files

None

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements the ThroughputAdaptiveStrategy and ExternalTriggerStrategy for ceremony scheduling. The throughput-adaptive strategy fires ceremonies based on velocity anomalies relative to a rolling baseline, while the external-trigger strategy responds to signals like webhooks and git events. The changes include implementation logic, updated design documentation, new observability events, and unit tests. I have no feedback to provide.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements two additional CeremonySchedulingStrategy implementations (Throughput-Adaptive and External-Trigger) as part of the pluggable ceremony scheduling system, along with observability event constants, exports/registration wiring, docs updates, and unit tests.

Changes:

  • Add ThroughputAdaptiveStrategy with rolling-window throughput anomaly detection and baseline establishment.
  • Add ExternalTriggerStrategy that buffers and matches named external events and declarative source registrations.
  • Register/export both strategies, add new workflow observability event constants, and document the additions (plus extensive unit tests).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/synthorg/engine/workflow/strategies/throughput_adaptive.py New throughput anomaly-based strategy with lifecycle state (baseline + rolling window) and config validation.
src/synthorg/engine/workflow/strategies/external_trigger.py New strategy for firing ceremonies / transitioning sprints based on external event names and buffered events.
src/synthorg/engine/workflow/strategies/__init__.py Exports/registers the two new strategies from the strategies package.
src/synthorg/engine/workflow/__init__.py Re-exports the new strategies (and adds missing re-exports for existing strategies).
src/synthorg/observability/events/workflow.py Adds observability event-name constants for the two new strategies.
docs/design/ceremony-scheduling.md Updates observability events table and roadmap/status notes for #973/#974.
tests/unit/engine/workflow/strategies/test_throughput_adaptive.py New unit tests covering throughput-adaptive behavior, lifecycle, and validation.
tests/unit/engine/workflow/strategies/test_external_trigger.py New unit tests covering external-trigger behavior, lifecycle, and validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +157 to +161
if on_drop and self._check_velocity_drop(ceremony.name, current_rate):
return True

return on_spike and self._check_velocity_spike(ceremony.name, current_rate)

Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should_fire_ceremony() is level-triggered: once the rolling rate falls below (or above) the threshold, this will return True on every subsequent evaluation while the anomaly persists. Since CeremonyScheduler evaluates on every task completion and does not have one-shot suppression for this strategy, this can repeatedly trigger the same ceremony many times for a single sustained drop/spike. Consider making anomaly detection edge-triggered (track last anomaly state per ceremony and only fire on transitions into drop/spike), or introduce an explicit cooldown/once-per-window mechanism tied to context.completions_since_last_trigger or time.

Suggested change
if on_drop and self._check_velocity_drop(ceremony.name, current_rate):
return True
return on_spike and self._check_velocity_spike(ceremony.name, current_rate)
# Cooldown: avoid repeatedly firing for a sustained anomaly.
# Require at least `window_size` completions since the last trigger.
completions_since_last_trigger = getattr(
context,
"completions_since_last_trigger",
0,
)
cooldown_reached = completions_since_last_trigger >= self._window_size
if on_drop and cooldown_reached and self._check_velocity_drop(
ceremony.name,
current_rate,
):
return True
if on_spike and cooldown_reached and self._check_velocity_spike(
ceremony.name,
current_rate,
):
return True
return False

Copilot uses AI. Check for mistakes.
Comment on lines +121 to +126
return self._match_event(
ceremony.name,
event_name,
context.external_events,
)

Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffered external events are never consumed/acknowledged. Once an event name is added to _received_events, _match_event() will keep returning True for that ceremony on every later evaluation, causing repeated ceremony triggers without any new external signal. Use _event_counts to make matching edge-triggered (e.g., track last-seen count per ceremony/event) or remove/mark events as processed after a match (being careful if multiple ceremonies can match the same event).

Suggested change
return self._match_event(
ceremony.name,
event_name,
context.external_events,
)
# Edge-triggered matching: fire only when the count of this event
# increases compared to the last evaluation for this ceremony.
ceremony_event_key = f"{ceremony.name}:{event_name}"
# Derive a numeric "current count" from context.external_events.
events = getattr(context, "external_events", {}) or {}
current_value = None
if hasattr(events, "get"):
current_value = events.get(event_name)
elif isinstance(events, dict):
current_value = events.get(event_name)
else:
current_value = None
if isinstance(current_value, int):
current_count = current_value
elif current_value:
# Treat any truthy presence as a single occurrence.
current_count = 1
else:
current_count = 0
last_count = self._event_counts.get(ceremony_event_key, 0)
self._event_counts[ceremony_event_key] = current_count
return current_count > last_count

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 93.57326% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.68%. Comparing base (43e0707) to head (eeb59f4).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../engine/workflow/strategies/throughput_adaptive.py 89.40% 16 Missing and 7 partials ⚠️
...org/engine/workflow/strategies/external_trigger.py 98.70% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1003      +/-   ##
==========================================
+ Coverage   91.66%   91.68%   +0.02%     
==========================================
  Files         656      658       +2     
  Lines       35719    36108     +389     
  Branches     3512     3568      +56     
==========================================
+ Hits        32742    33107     +365     
- Misses       2357     2374      +17     
- Partials      620      627       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/engine/workflow/strategies/external_trigger.py`:
- Around line 198-204: The current initialization for self._sources silently
drops non-dict entries and only shallow-copies dicts; instead validate and
fail-fast if strategy_config.get(_KEY_SOURCES) is present but not a list, and if
the list contains any non-dict entries raise a clear ValueError; for valid
entries deep-copy each dict (use copy.deepcopy) and wrap each copied dict with
types.MappingProxyType to produce an immutable snapshot, then set self._sources
to a tuple of those MappingProxyType instances (ensure you import copy and
MappingProxyType and update any callers expecting mutable dicts).
- Around line 91-93: _received_events currently accumulates names forever
causing duplicate signals to be treated as permanently present; change the
buffer semantics to "pending" and consume events when matched: replace the
set-based _received_events/_event_counts usage with a consumable buffer (e.g.,
per-source lists or deque stored in _sources/_event_counts) so that
should_fire_ceremony() and should_transition_sprint() pop/consume matching
events from CeremonyEvalContext.external_events rather than just testing
membership; apply the same consume-once change to the other occurrence around
lines 297-304 so events are removed/decremented when used and duplicates do not
remain hot for the rest of the sprint.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py`:
- Around line 153-160: The current evaluation is level-triggered and repeatedly
returns True while the rolling rate stays beyond thresholds; change it to only
fire on threshold crossings by tracking per-ceremony anomaly state and comparing
transitions. Add a map keyed by ceremony.name (e.g., self._last_anomaly_state)
that stores None, "drop", or "spike"; compute current_state from
_check_velocity_drop/_check_velocity_spike, then only return True and update the
map when current_state != last_state and current_state is "drop" or "spike"; set
the map back to None when the window returns to normal. Apply the same
transition logic for all checks referenced in
_check_velocity_drop/_check_velocity_spike and the surrounding evaluation block
so sustained anomalies don't retrigger until a state change occurs.
🪄 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: 7b42a642-a54e-4ae5-a4cc-31b1a2f446a0

📥 Commits

Reviewing files that changed from the base of the PR and between a4cbc4e and 4773e8c.

📒 Files selected for processing (8)
  • docs/design/ceremony-scheduling.md
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/observability/events/workflow.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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). (8)
  • GitHub Check: Agent
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Build Web
  • GitHub Check: Build Sandbox
  • GitHub Check: Build Backend
  • GitHub Check: Dependency Review
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Do not use from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations.
Use except A, B: syntax without parentheses for exception handling — PEP 758 syntax, enforced by ruff on Python 3.14.
All public functions and classes must have type hints. Use mypy strict mode for type checking.
Use Google-style docstrings on all public classes and functions. Enforced by ruff D rules.
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction plus MappingProxyType wrapping for read-only enforcement. For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries.
Use frozen Pydantic v2 models (BaseModel with frozen=True) 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.
Use Pydantic v2 with: allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time; @computed_field for derived values instead of storing redundant fields; NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
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.
Lines must not exceed 88 characters. Enforced by ruff formatter.
Functions must be fewer than 50 lines, and files must be fewer than 800 lines.
Handle errors explicitly and never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • src/synthorg/observability/events/workflow.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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 or print() in application code.
Always use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured logging: logger.info(EVENT, key=value) — never logger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG logs are used for object creation, internal flow, and entry/exit of key functions.
Never implement retry logic in provider driver subclasses or calling code. All provider calls go through BaseCompletionProvider which applies retry and rate limiting automatically. Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig.
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. Tests must use test-provider, test-small-001, etc.

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/observability/events/workflow.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
docs/design/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

docs/design/**/*.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. The design spec is the starting point for architecture, data models, and behavior.
If implementation deviates from the spec (better approach found, scope evolved, etc.), alert the user and explain why. Do NOT silently diverge—every deviation needs explicit user approval. When approved deviations occur, update the relevant docs/design/ page to reflect the new reality.

Files:

  • docs/design/ceremony-scheduling.md
**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Documentation is built with Zensical from Markdown sources in docs/. Design spec is in docs/design/ (12 pages). Architecture docs in docs/architecture/. REST API reference in docs/rest-api.md (with generated docs/_generated/api-reference.html). Auto-generated library reference via mkdocstrings + Griffe in docs/api/.

Files:

  • docs/design/ceremony-scheduling.md
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Always include @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, or @pytest.mark.slow markers on test functions.
Maintain 80% code coverage minimum (enforced in CI). 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, use asyncio.Event().wait() instead of large asyncio.sleep() values.
Always run pytest with -n auto for parallel execution via pytest-xdist. Never run tests sequentially.
Prefer @pytest.mark.parametrize for testing similar cases instead of writing separate test functions.
Use Hypothesis for property-based testing with @given and @settings decorators. Run dev profile tests with HYPOTHESIS_PROFILE=dev to use 1000 examples instead of the default 50.

Files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
🧠 Learnings (12)
📚 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/engine/workflow/__init__.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/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)

Applied to files:

  • src/synthorg/engine/workflow/__init__.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/observability/events/workflow.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/observability/events/workflow.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/observability/events/workflow.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
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/observability/events/workflow.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/observability/events/workflow.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/workflow.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 src/synthorg/**/*.py : Always 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 rather than using string literals

Applied to files:

  • src/synthorg/observability/events/workflow.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/workflow.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to src/synthorg/**/*.py : Always use constants from domain-specific modules under `synthorg.observability.events` for event names (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/observability/events/workflow.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/observability/events/workflow.py

Comment on lines +91 to +93
self._received_events: set[str] = set()
self._event_counts: dict[str, int] = {}
self._sources: tuple[dict[str, Any], ...] = ()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Model buffered external events as pending, not a sprint-long set.

_received_events is only ever added to, and matching only consults that set. That collapses duplicate signals and keeps a matched name hot for the rest of the sprint: after one pr_merged, every later should_fire_ceremony() / should_transition_sprint() call can keep returning True. CeremonyEvalContext.external_events is documented as pending in src/synthorg/engine/workflow/ceremony_context.py, so the hook-backed buffer needs consume-once semantics as well.

Also applies to: 297-304

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 91
- 93, _received_events currently accumulates names forever causing duplicate
signals to be treated as permanently present; change the buffer semantics to
"pending" and consume events when matched: replace the set-based
_received_events/_event_counts usage with a consumable buffer (e.g., per-source
lists or deque stored in _sources/_event_counts) so that should_fire_ceremony()
and should_transition_sprint() pop/consume matching events from
CeremonyEvalContext.external_events rather than just testing membership; apply
the same consume-once change to the other occurrence around lines 297-304 so
events are removed/decremented when used and duplicates do not remain hot for
the rest of the sprint.

@Aureliolo Aureliolo force-pushed the feat/ceremony-strategies-batch2 branch from 4773e8c to e4c93fc Compare April 2, 2026 17:02
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview April 2, 2026 17:04 — with GitHub Actions Inactive
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/external_trigger.py (1)

176-183: ⚠️ Potential issue | 🟠 Major

Make buffered transition events one-shot as well.

This path only checks _received_events, so one buffered transition_event stays hot until sprint deactivation. If should_transition_sprint() is evaluated more than once before state is cleared, the same occurrence keeps returning IN_REVIEW instead of behaving like the one-shot buffered signals used in should_fire_ceremony().

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 176
- 183, The buffered transition event check in should_transition_sprint() only
reads self._received_events and never consumes the event, so the same
transition_event can repeatedly trigger IN_REVIEW; update the block that handles
transition_event (the clause checking transition_event in self._received_events,
logging SPRINT_AUTO_TRANSITION and returning SprintStatus.IN_REVIEW) to
consume/remove the event from self._received_events (one-shot) before returning
so it behaves like the one-shot signals used in should_fire_ceremony().
🤖 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/engine/workflow/strategies/external_trigger.py`:
- Line 72: The allow-list _VALID_SOURCE_TYPES currently only includes "webhook"
and "git_event" but must accept declarative MCP sources; update the frozenset
_VALID_SOURCE_TYPES in external_trigger.py to include "mcp" (or the exact
canonical string used by MCP events) and search for other occurrences of source
validation (references around the module's source validation logic) to add the
same value so MCP-originated triggers pass validation; ensure any tests or error
messages that enumerate valid types are updated to reflect the new accepted
value.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py`:
- Around line 128-187: The method should_fire_ceremony is too long; extract the
anomaly computation and the edge-trigger bookkeeping into two helpers to reduce
length: implement a private method _determine_anomaly_state(current_rate: float
| None, on_drop: bool, on_spike: bool) -> str | None that uses _is_velocity_drop
and _is_velocity_spike to return "drop", "spike", or None, and implement another
private method _update_and_check_edge_trigger(ceremony_name: str, new_state: str
| None) -> bool that reads/modifies self._last_anomaly_state and returns True
only when transitioning into a non-None state (edge-trigger). Then simplify
should_fire_ceremony to resolve config, cold-start guard, compute current_rate,
call _determine_anomaly_state and _update_and_check_edge_trigger, log via
_log_anomaly_trigger when the edge returns True, and return the result. Ensure
helper names (_determine_anomaly_state, _update_and_check_edge_trigger) and uses
of _is_velocity_drop/_is_velocity_spike and
self._last_anomaly_state/_log_anomaly_trigger match the existing symbols.

---

Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/external_trigger.py`:
- Around line 176-183: The buffered transition event check in
should_transition_sprint() only reads self._received_events and never consumes
the event, so the same transition_event can repeatedly trigger IN_REVIEW; update
the block that handles transition_event (the clause checking transition_event in
self._received_events, logging SPRINT_AUTO_TRANSITION and returning
SprintStatus.IN_REVIEW) to consume/remove the event from self._received_events
(one-shot) before returning so it behaves like the one-shot signals used in
should_fire_ceremony().
🪄 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: 90a1ab27-94d6-4f0b-a638-00a163abcc35

📥 Commits

Reviewing files that changed from the base of the PR and between 4773e8c and e4c93fc.

📒 Files selected for processing (8)
  • docs/design/ceremony-scheduling.md
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/observability/events/workflow.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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). (6)
  • 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 (5)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Do not use from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations.
Use except A, B: syntax without parentheses for exception handling — PEP 758 syntax, enforced by ruff on Python 3.14.
All public functions and classes must have type hints. Use mypy strict mode for type checking.
Use Google-style docstrings on all public classes and functions. Enforced by ruff D rules.
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction plus MappingProxyType wrapping for read-only enforcement. For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries.
Use frozen Pydantic v2 models (BaseModel with frozen=True) 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.
Use Pydantic v2 with: allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time; @computed_field for derived values instead of storing redundant fields; NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
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.
Lines must not exceed 88 characters. Enforced by ruff formatter.
Functions must be fewer than 50 lines, and files must be fewer than 800 lines.
Handle errors explicitly and never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
  • src/synthorg/observability/events/workflow.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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 or print() in application code.
Always use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured logging: logger.info(EVENT, key=value) — never logger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG logs are used for object creation, internal flow, and entry/exit of key functions.
Never implement retry logic in provider driver subclasses or calling code. All provider calls go through BaseCompletionProvider which applies retry and rate limiting automatically. Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig.
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. Tests must use test-provider, test-small-001, etc.

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/observability/events/workflow.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
docs/design/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

docs/design/**/*.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. The design spec is the starting point for architecture, data models, and behavior.
If implementation deviates from the spec (better approach found, scope evolved, etc.), alert the user and explain why. Do NOT silently diverge—every deviation needs explicit user approval. When approved deviations occur, update the relevant docs/design/ page to reflect the new reality.

Files:

  • docs/design/ceremony-scheduling.md
**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Documentation is built with Zensical from Markdown sources in docs/. Design spec is in docs/design/ (12 pages). Architecture docs in docs/architecture/. REST API reference in docs/rest-api.md (with generated docs/_generated/api-reference.html). Auto-generated library reference via mkdocstrings + Griffe in docs/api/.

Files:

  • docs/design/ceremony-scheduling.md
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Always include @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, or @pytest.mark.slow markers on test functions.
Maintain 80% code coverage minimum (enforced in CI). 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, use asyncio.Event().wait() instead of large asyncio.sleep() values.
Always run pytest with -n auto for parallel execution via pytest-xdist. Never run tests sequentially.
Prefer @pytest.mark.parametrize for testing similar cases instead of writing separate test functions.
Use Hypothesis for property-based testing with @given and @settings decorators. Run dev profile tests with HYPOTHESIS_PROFILE=dev to use 1000 examples instead of the default 50.

Files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
🧠 Learnings (24)
📚 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/engine/workflow/__init__.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/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)

Applied to files:

  • src/synthorg/engine/workflow/__init__.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to tests/**/*.py : Maintain 80% code coverage minimum (enforced in CI). 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, use `asyncio.Event().wait()` instead of large `asyncio.sleep()` values.

Applied to files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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 tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).

Applied to files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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/observability/events/workflow.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/observability/events/workflow.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/observability/events/workflow.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/workflow.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/observability/events/workflow.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
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/observability/events/workflow.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 src/synthorg/**/*.py : Always 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 rather than using string literals

Applied to files:

  • src/synthorg/observability/events/workflow.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/workflow.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to src/synthorg/**/*.py : Always use constants from domain-specific modules under `synthorg.observability.events` for event names (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/observability/events/workflow.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/observability/events/workflow.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 src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 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.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to **/*.py : Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use `copy.deepcopy()` at construction plus `MappingProxyType` wrapping for read-only enforcement. For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py

_MAX_SOURCES: int = 20
_MAX_RECEIVED_EVENTS: int = 256
_MAX_LOG_VALUE_LEN: int = 64
_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event"})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Accept an MCP source in the validated source list.

The strategy/doc updates say external triggers can come from MCP invocations, but this allow-list only admits "webhook" and "git_event". A declarative MCP source will therefore fail validation even though the feature is being documented as implemented.

As per coding guidelines, "If implementation deviates from the spec (better approach found, scope evolved, etc.), alert the user and explain why. Do NOT silently diverge—every deviation needs explicit user approval."

Also applies to: 507-518

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` at line 72, The
allow-list _VALID_SOURCE_TYPES currently only includes "webhook" and "git_event"
but must accept declarative MCP sources; update the frozenset
_VALID_SOURCE_TYPES in external_trigger.py to include "mcp" (or the exact
canonical string used by MCP events) and search for other occurrences of source
validation (references around the module's source validation logic) to add the
same value so MCP-originated triggers pass validation; ensure any tests or error
messages that enumerate valid types are updated to reflect the new accepted
value.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
src/synthorg/engine/workflow/strategies/external_trigger.py (2)

72-72: ⚠️ Potential issue | 🟠 Major

MCP source type missing from validation allow-list.

The module docstring (line 5) and linked issue #974 specify that MCP tool invocations are a supported external trigger source. However, _VALID_SOURCE_TYPES only permits "webhook" and "git_event". A declarative MCP source will fail validation at runtime even though the feature is documented.

🔧 Proposed fix to add MCP source type
-_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event"})
+_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event", "mcp"})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` at line 72, The
validation set _VALID_SOURCE_TYPES in external_trigger.py is missing the "mcp"
source type declared in the module docstring and issue `#974`; update the
frozenset assigned to _VALID_SOURCE_TYPES to include "mcp" so that functions
using this constant (e.g., any validation logic referencing _VALID_SOURCE_TYPES)
will accept MCP tool invocations at runtime.

212-220: ⚠️ Potential issue | 🟡 Minor

Silent filtering of non-dict source entries may hide configuration errors.

While validate_strategy_config (lines 484-498) validates source entries, on_sprint_activated still silently drops non-dict entries at line 217. If activation occurs without prior validation (e.g., during testing, direct API usage, or future refactoring), misconfigured sources are silently ignored.

As per coding guidelines: "Handle errors explicitly and never silently swallow exceptions" and "Validate at system boundaries."

🛡️ Proposed fix to add defensive logging
         raw_sources = strategy_config.get(_KEY_SOURCES)
         if isinstance(raw_sources, list):
-            self._sources = tuple(
-                MappingProxyType(copy.deepcopy(entry))
-                for entry in raw_sources
-                if isinstance(entry, dict)
-            )
+            valid_sources: list[MappingProxyType[str, Any]] = []
+            for i, entry in enumerate(raw_sources):
+                if isinstance(entry, dict):
+                    valid_sources.append(MappingProxyType(copy.deepcopy(entry)))
+                else:
+                    logger.warning(
+                        SPRINT_STRATEGY_CONFIG_INVALID,
+                        strategy="external_trigger",
+                        key=f"sources[{i}]",
+                        value=type(entry).__name__,
+                        reason="expected dict, skipping",
+                    )
+            self._sources = tuple(valid_sources)
         else:
             self._sources = ()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 212
- 220, The code in on_sprint_activated silently drops non-dict entries from
strategy_config[_KEY_SOURCES]; detect and report those bad entries instead of
ignoring them: inspect raw_sources (from strategy_config.get(_KEY_SOURCES)),
build a list of non-dict items (or their indices), and if any exist emit a
defensive warning including their indices/values and the strategy identifier
(use the same logger used elsewhere in this class, e.g., self._logger or module
logger) before continuing to set self._sources to the tuple of
MappingProxyType(copy.deepcopy(entry)) for the valid dicts; keep existing
behavior for valid entries but do not swallow misconfigurations silently.
src/synthorg/engine/workflow/strategies/throughput_adaptive.py (1)

66-74: ⚠️ Potential issue | 🟠 Major

Don't accept on_drop / on_spike at sprint scope unless you actually apply them there.

activate_sprint() only validates config.ceremony_policy.strategy_config, but this strategy reads those booleans from get_ceremony_config(ceremony) and never stores sprint-level defaults in on_sprint_activated(). A sprint-level on_spike: true therefore validates cleanly and then does nothing. Either remove these keys from _KNOWN_CONFIG_KEYS / validate_strategy_config() or treat them as sprint defaults and merge them into each ceremony evaluation.

As per coding guidelines, "Validate at system boundaries (user input, external APIs, config files)."

Also applies to: 144-146, 313-329, 432-447

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py` around lines
66 - 74, The strategy lists on_drop/on_spike in _KNOWN_CONFIG_KEYS (and thus
validate_strategy_config()) but never applies sprint-scoped values:
activate_sprint() only validates ceremony_policy.strategy_config and
on_sprint_activated() does not merge sprint defaults, while
get_ceremony_config(ceremony) reads booleans only from ceremony-level config;
fix by either removing _KEY_ON_DROP/_KEY_ON_SPIKE from
_KNOWN_CONFIG_KEYS/validate_strategy_config() or (preferably) treat them as
sprint defaults by updating on_sprint_activated() to merge sprint-level
on_drop/on_spike into each ceremony’s config (so get_ceremony_config() sees the
merged value) and ensure activate_sprint()/on_sprint_activated() propagate those
keys to ceremony configs before any ceremony evaluation.
🤖 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/engine/workflow/strategies/throughput_adaptive.py`:
- Around line 302-343: Add INFO-level structured logs in on_sprint_activated and
on_sprint_deactivated to record state transitions: in on_sprint_activated
(function name) log an INFO entry after initializing _drop_threshold_pct,
_spike_threshold_pct and _window_size that includes the threshold values and
window size (and optionally initial
baseline/_blocked_count/_completion_timestamps length) so activation is visible;
in on_sprint_deactivated log an INFO entry when you clear
_completion_timestamps, _baseline_rate, _blocked_count, _last_anomaly_state and
reset _drop_threshold_pct/_spike_threshold_pct/_window_size so deactivation is
visible. Use the class logger (e.g., self.logger or the existing logger used in
this module) and emit structured fields for drop_threshold_pct,
spike_threshold_pct, window_size, and a short note like "throughput tracking
activated"/"throughput tracking reset".

---

Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/external_trigger.py`:
- Line 72: The validation set _VALID_SOURCE_TYPES in external_trigger.py is
missing the "mcp" source type declared in the module docstring and issue `#974`;
update the frozenset assigned to _VALID_SOURCE_TYPES to include "mcp" so that
functions using this constant (e.g., any validation logic referencing
_VALID_SOURCE_TYPES) will accept MCP tool invocations at runtime.
- Around line 212-220: The code in on_sprint_activated silently drops non-dict
entries from strategy_config[_KEY_SOURCES]; detect and report those bad entries
instead of ignoring them: inspect raw_sources (from
strategy_config.get(_KEY_SOURCES)), build a list of non-dict items (or their
indices), and if any exist emit a defensive warning including their
indices/values and the strategy identifier (use the same logger used elsewhere
in this class, e.g., self._logger or module logger) before continuing to set
self._sources to the tuple of MappingProxyType(copy.deepcopy(entry)) for the
valid dicts; keep existing behavior for valid entries but do not swallow
misconfigurations silently.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py`:
- Around line 66-74: The strategy lists on_drop/on_spike in _KNOWN_CONFIG_KEYS
(and thus validate_strategy_config()) but never applies sprint-scoped values:
activate_sprint() only validates ceremony_policy.strategy_config and
on_sprint_activated() does not merge sprint defaults, while
get_ceremony_config(ceremony) reads booleans only from ceremony-level config;
fix by either removing _KEY_ON_DROP/_KEY_ON_SPIKE from
_KNOWN_CONFIG_KEYS/validate_strategy_config() or (preferably) treat them as
sprint defaults by updating on_sprint_activated() to merge sprint-level
on_drop/on_spike into each ceremony’s config (so get_ceremony_config() sees the
merged value) and ensure activate_sprint()/on_sprint_activated() propagate those
keys to ceremony configs before any ceremony evaluation.
🪄 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: c09d3ac6-f2d2-4559-adfc-fb6fad0af985

📥 Commits

Reviewing files that changed from the base of the PR and between e4c93fc and bec44a8.

📒 Files selected for processing (2)
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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: Test (Python 3.14)
  • GitHub Check: Build Web
  • GitHub Check: Build Sandbox
  • GitHub Check: Build Backend
  • GitHub Check: Dependency Review
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Do not use from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations.
Use except A, B: syntax without parentheses for exception handling — PEP 758 syntax, enforced by ruff on Python 3.14.
All public functions and classes must have type hints. Use mypy strict mode for type checking.
Use Google-style docstrings on all public classes and functions. Enforced by ruff D rules.
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction plus MappingProxyType wrapping for read-only enforcement. For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries.
Use frozen Pydantic v2 models (BaseModel with frozen=True) 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.
Use Pydantic v2 with: allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time; @computed_field for derived values instead of storing redundant fields; NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
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.
Lines must not exceed 88 characters. Enforced by ruff formatter.
Functions must be fewer than 50 lines, and files must be fewer than 800 lines.
Handle errors explicitly and never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).

Files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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 or print() in application code.
Always use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured logging: logger.info(EVENT, key=value) — never logger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG logs are used for object creation, internal flow, and entry/exit of key functions.
Never implement retry logic in provider driver subclasses or calling code. All provider calls go through BaseCompletionProvider which applies retry and rate limiting automatically. Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig.
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. Tests must use test-provider, test-small-001, etc.

Files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
🧠 Learnings (14)
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 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.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to **/*.py : Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use `copy.deepcopy()` at construction plus `MappingProxyType` wrapping for read-only enforcement. For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
🔇 Additional comments (7)
src/synthorg/engine/workflow/strategies/external_trigger.py (6)

75-105: LGTM!

The class structure is well-designed with __slots__ for memory efficiency and clear separation of state variables. The docstring accurately describes the edge-triggered matching semantics.


108-136: LGTM!

Clean implementation that validates input, normalizes whitespace, and delegates matching to the well-documented _match_event helper.


138-187: LGTM!

The one-shot consumption semantics for sprint transitions is correctly implemented using discard() at line 178. Context events are checked first (inherently one-shot per the relevant snippet showing external_events=() is rebuilt each evaluation), and buffered events are consumed to prevent re-triggering.


273-321: LGTM!

The method properly validates input at the boundary, guards against unbounded event accumulation with _MAX_RECEIVED_EVENTS, and logs appropriately at WARNING level when limits are exceeded. The dual tracking (set for presence, dict for counts) enables the edge-triggered semantics.


366-397: LGTM!

The edge-triggered matching logic is well-implemented. Context events are inherently one-shot (per the relevant snippet showing external_events=() is rebuilt fresh for each ceremony evaluation), and buffered events use count comparison to ensure each on_external_event call fires the ceremony at most once. This correctly addresses the consume-once semantics requirement.


429-520: LGTM!

Comprehensive validation with clear error messages and proper logging at WARNING level before raising exceptions. The length limits (_MAX_EVENT_NAME_LEN, _MAX_SOURCES) provide sensible bounds. The validation logic at line 509 correctly uses _VALID_SOURCE_TYPES (though that set needs updating per the earlier comment about MCP support).

src/synthorg/engine/workflow/strategies/throughput_adaptive.py (1)

410-413: Remove this comment — the implementation correctly matches the protocol.

The protocol CeremonySchedulingStrategy already defines strategy_type as a @property (line 203), not a method. The implementation in ThroughputAdaptiveStrategy correctly uses the same @property decorator. No method/property mismatch exists, and no call sites invoke it as a method call.

			> Likely an incorrect or invalid review comment.

Comment on lines +302 to +343
async def on_sprint_activated(
self,
sprint: Sprint, # noqa: ARG002
config: SprintConfig,
) -> None:
"""Initialize rate tracking from sprint config.

Args:
sprint: The activated sprint.
config: Sprint configuration.
"""
strategy_config = (
config.ceremony_policy.strategy_config
if config.ceremony_policy.strategy_config is not None
else {}
)

self._drop_threshold_pct = self._resolve_threshold(
strategy_config,
_KEY_VELOCITY_DROP_THRESHOLD_PCT,
_DEFAULT_DROP_THRESHOLD_PCT,
)
self._spike_threshold_pct = self._resolve_threshold(
strategy_config,
_KEY_VELOCITY_SPIKE_THRESHOLD_PCT,
_DEFAULT_SPIKE_THRESHOLD_PCT,
)
self._window_size = self._resolve_window_size(strategy_config)
self._completion_timestamps = deque(maxlen=self._window_size)
self._baseline_rate = None
self._blocked_count = 0
self._last_anomaly_state = {}

async def on_sprint_deactivated(self) -> None:
"""Clear all internal state."""
self._completion_timestamps = deque(maxlen=_DEFAULT_WINDOW_SIZE)
self._baseline_rate = None
self._blocked_count = 0
self._last_anomaly_state = {}
self._drop_threshold_pct = _DEFAULT_DROP_THRESHOLD_PCT
self._spike_threshold_pct = _DEFAULT_SPIKE_THRESHOLD_PCT
self._window_size = _DEFAULT_WINDOW_SIZE
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Log activation and reset of throughput-tracking state at INFO.

Both lifecycle hooks rewrite the full anomaly-tracking state, but neither emits an INFO event. That makes mid-run baseline resets and threshold/window reinitialization invisible when you're debugging scheduler behavior. Add structured INFO logs for activation and deactivation, ideally including the configured window and thresholds on activation.

As per coding guidelines, "All state transitions must log at INFO."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py` around lines
302 - 343, Add INFO-level structured logs in on_sprint_activated and
on_sprint_deactivated to record state transitions: in on_sprint_activated
(function name) log an INFO entry after initializing _drop_threshold_pct,
_spike_threshold_pct and _window_size that includes the threshold values and
window size (and optionally initial
baseline/_blocked_count/_completion_timestamps length) so activation is visible;
in on_sprint_deactivated log an INFO entry when you clear
_completion_timestamps, _baseline_rate, _blocked_count, _last_anomaly_state and
reset _drop_threshold_pct/_spike_threshold_pct/_window_size so deactivation is
visible. Use the class logger (e.g., self.logger or the existing logger used in
this module) and emit structured fields for drop_threshold_pct,
spike_threshold_pct, window_size, and a short note like "throughput tracking
activated"/"throughput tracking reset".

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
src/synthorg/engine/workflow/strategies/external_trigger.py (2)

68-72: ⚠️ Potential issue | 🟠 Major

Allow the documented MCP source type.

The module docstring and PR objective say MCP invocations are supported, but this allow-list still rejects that source during validation, so the documented config path can never work.

🛠️ Suggested change
-_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event"})
+_VALID_SOURCE_TYPES: frozenset[str] = frozenset(
+    {"webhook", "git_event", "mcp"}
+)

Also applies to: 519-523

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 68
- 72, The allow-list _VALID_SOURCE_TYPES incorrectly omits the "mcp" source type
so MCP invocations are rejected; update the frozenset assigned to
_VALID_SOURCE_TYPES (and any duplicate definition of that constant used later)
to include "mcp" alongside "webhook" and "git_event" so validation accepts the
documented MCP source type (ensure you update both occurrences of
_VALID_SOURCE_TYPES if duplicated).

58-64: ⚠️ Potential issue | 🟠 Major

Reject on_external at sprint scope.

on_external is documented as a per-ceremony override, but validate_strategy_config() currently whitelists it in sprint-level config. A misplaced sprint-level value will validate cleanly and then be ignored by activation/transition logic.

🛠️ Suggested change
 _KNOWN_CONFIG_KEYS: frozenset[str] = frozenset(
     {
-        _KEY_ON_EXTERNAL,
         _KEY_TRANSITION_EVENT,
         _KEY_SOURCES,
     }
 )
@@
-        self._validate_string_key(config, _KEY_ON_EXTERNAL)
         self._validate_string_key(config, _KEY_TRANSITION_EVENT)
As per coding guidelines, "Validate at system boundaries (user input, external APIs, config files)."

Also applies to: 357-368

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 58
- 64, The sprint-level whitelist currently includes _KEY_ON_EXTERNAL inside
_KNOWN_CONFIG_KEYS causing validate_strategy_config() to accept a per-ceremony
key at sprint scope; remove _KEY_ON_EXTERNAL from the sprint/global whitelist
(the frozenset bound named _KNOWN_CONFIG_KEYS) and update
validate_strategy_config() checks to only allow _KEY_ON_EXTERNAL in
ceremony/activation-level config paths (not sprint-level validation). Also audit
the analogous whitelist used around the other occurrence noted (the second block
that mirrors lines 357-368) and ensure _KEY_ON_EXTERNAL is not permitted there
either so misplaced sprint values fail validation.
src/synthorg/engine/workflow/strategies/throughput_adaptive.py (1)

303-344: ⚠️ Potential issue | 🟡 Minor

Log throughput activation and reset at INFO.

These hooks rewrite thresholds, window size, baseline, blocked count, and anomaly state without an INFO log, so mid-sprint resets are invisible in workflow telemetry.

Based on learnings, "All state transitions must log at INFO."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py` around lines
303 - 344, Add INFO-level logs in on_sprint_activated and on_sprint_deactivated
to record state transitions and values: in on_sprint_activated log that the
throughput adaptive strategy was activated and include the resolved
_drop_threshold_pct, _spike_threshold_pct, _window_size, and initial
_baseline_rate/_blocked_count/_last_anomaly_state; in on_sprint_deactivated log
that the strategy was reset and include the reset values for
_drop_threshold_pct, _spike_threshold_pct, _window_size, _baseline_rate,
_blocked_count, and _last_anomaly_state. Use the class logger (e.g.,
self._logger or existing logger instance) and INFO level so these mid-sprint
activations/resets appear in workflow telemetry.
🤖 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/engine/workflow/strategies/external_trigger.py`:
- Around line 100-104: The code stores ceremony/event state using a concatenated
string key (e.g., f"{ceremony_name}:{event_name}") which can collide if
ceremony_name or event_name contains ':'; update the storage and lookup to use a
tuple key (ceremony_name, event_name) instead of a formatted string for
_event_counts and _ceremony_last_fired_counts and anywhere else the string key
is constructed or accessed (refer to variables _event_counts,
_ceremony_last_fired_counts and places that build keys for ceremony/event edges)
so all insertions, increments, reads, and deletions use the tuple form
consistently.

---

Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/external_trigger.py`:
- Around line 68-72: The allow-list _VALID_SOURCE_TYPES incorrectly omits the
"mcp" source type so MCP invocations are rejected; update the frozenset assigned
to _VALID_SOURCE_TYPES (and any duplicate definition of that constant used
later) to include "mcp" alongside "webhook" and "git_event" so validation
accepts the documented MCP source type (ensure you update both occurrences of
_VALID_SOURCE_TYPES if duplicated).
- Around line 58-64: The sprint-level whitelist currently includes
_KEY_ON_EXTERNAL inside _KNOWN_CONFIG_KEYS causing validate_strategy_config() to
accept a per-ceremony key at sprint scope; remove _KEY_ON_EXTERNAL from the
sprint/global whitelist (the frozenset bound named _KNOWN_CONFIG_KEYS) and
update validate_strategy_config() checks to only allow _KEY_ON_EXTERNAL in
ceremony/activation-level config paths (not sprint-level validation). Also audit
the analogous whitelist used around the other occurrence noted (the second block
that mirrors lines 357-368) and ensure _KEY_ON_EXTERNAL is not permitted there
either so misplaced sprint values fail validation.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py`:
- Around line 303-344: Add INFO-level logs in on_sprint_activated and
on_sprint_deactivated to record state transitions and values: in
on_sprint_activated log that the throughput adaptive strategy was activated and
include the resolved _drop_threshold_pct, _spike_threshold_pct, _window_size,
and initial _baseline_rate/_blocked_count/_last_anomaly_state; in
on_sprint_deactivated log that the strategy was reset and include the reset
values for _drop_threshold_pct, _spike_threshold_pct, _window_size,
_baseline_rate, _blocked_count, and _last_anomaly_state. Use the class logger
(e.g., self._logger or existing logger instance) and INFO level so these
mid-sprint activations/resets appear in workflow telemetry.
🪄 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: 95b69622-e1a5-4a40-8ad3-e9fbb45e7960

📥 Commits

Reviewing files that changed from the base of the PR and between bec44a8 and 9cf1953.

📒 Files selected for processing (3)
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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). (6)
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Build Backend
  • GitHub Check: Build Sandbox
  • GitHub Check: Build Web
  • GitHub Check: Dependency Review
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Do not use from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations.
Use except A, B: syntax without parentheses for exception handling — PEP 758 syntax, enforced by ruff on Python 3.14.
All public functions and classes must have type hints. Use mypy strict mode for type checking.
Use Google-style docstrings on all public classes and functions. Enforced by ruff D rules.
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction plus MappingProxyType wrapping for read-only enforcement. For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries.
Use frozen Pydantic v2 models (BaseModel with frozen=True) 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.
Use Pydantic v2 with: allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time; @computed_field for derived values instead of storing redundant fields; NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
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.
Lines must not exceed 88 characters. Enforced by ruff formatter.
Functions must be fewer than 50 lines, and files must be fewer than 800 lines.
Handle errors explicitly and never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).

Files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Always include @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, or @pytest.mark.slow markers on test functions.
Maintain 80% code coverage minimum (enforced in CI). 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, use asyncio.Event().wait() instead of large asyncio.sleep() values.
Always run pytest with -n auto for parallel execution via pytest-xdist. Never run tests sequentially.
Prefer @pytest.mark.parametrize for testing similar cases instead of writing separate test functions.
Use Hypothesis for property-based testing with @given and @settings decorators. Run dev profile tests with HYPOTHESIS_PROFILE=dev to use 1000 examples instead of the default 50.

Files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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 or print() in application code.
Always use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured logging: logger.info(EVENT, key=value) — never logger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG logs are used for object creation, internal flow, and entry/exit of key functions.
Never implement retry logic in provider driver subclasses or calling code. All provider calls go through BaseCompletionProvider which applies retry and rate limiting automatically. Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig.
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. Tests must use test-provider, test-small-001, etc.

Files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
🧠 Learnings (25)
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to tests/**/*.py : Maintain 80% code coverage minimum (enforced in CI). 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, use `asyncio.Event().wait()` instead of large `asyncio.sleep()` values.

Applied to files:

  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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 src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 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.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to **/*.py : Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries, BaseTool), use `copy.deepcopy()` at construction plus `MappingProxyType` wrapping for read-only enforcement. For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : All state transitions must log at INFO.

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : All state transitions must log at INFO level

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 src/**/*.py : All state transitions must log at INFO level

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : 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:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 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:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
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 logs are used for object creation, internal flow, and entry/exit of key functions.

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log object creation/internal flow/entry-exit at DEBUG

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
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:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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/engine/workflow/strategies/throughput_adaptive.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/observability/**/*.py : Observability must use structured logging with correlation tracking and log sinks

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py

Comment on lines +100 to +104
def __init__(self) -> None:
self._received_events: set[str] = set()
self._event_counts: dict[str, int] = {}
self._ceremony_last_fired_counts: dict[str, int] = {}
self._sources: tuple[MappingProxyType[str, Any], ...] = ()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use a tuple key for ceremony/event edge state.

f"{ceremony_name}:{event_name}" can collide when either identifier contains :, causing distinct ceremony/event pairs to share fire-count state and suppress each other.

🛠️ Suggested change
-        self._ceremony_last_fired_counts: dict[str, int] = {}
+        self._ceremony_last_fired_counts: dict[tuple[str, str], int] = {}
@@
-        key = f"{ceremony_name}:{event_name}"
+        key = (ceremony_name, event_name)

Also applies to: 400-404

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 100
- 104, The code stores ceremony/event state using a concatenated string key
(e.g., f"{ceremony_name}:{event_name}") which can collide if ceremony_name or
event_name contains ':'; update the storage and lookup to use a tuple key
(ceremony_name, event_name) instead of a formatted string for _event_counts and
_ceremony_last_fired_counts and anywhere else the string key is constructed or
accessed (refer to variables _event_counts, _ceremony_last_fired_counts and
places that build keys for ceremony/event edges) so all insertions, increments,
reads, and deletions use the tuple form consistently.

Implement two new ceremony scheduling strategies:

- ThroughputAdaptiveStrategy (#973): fires ceremonies when rolling
  throughput rate drops or spikes beyond configured thresholds.
  Stateful -- tracks completion timestamps in a rolling window,
  establishes baseline from first full window, frozen per sprint.
  Default velocity calculator: TaskDrivenVelocityCalculator.

- ExternalTriggerStrategy (#974): fires ceremonies on named external
  signals (webhooks, git events, MCP invocations). Stateful -- buffers
  received events via lifecycle hooks. Source registration is
  declarative (transport integration handled externally).
  Default velocity calculator: PointsPerSprintVelocityCalculator.

Both strategies implement the CeremonySchedulingStrategy protocol with
full lifecycle hook support, strict config validation, lenient runtime
resolution, and structured observability logging.

Closes #973
Closes #974
Pre-reviewed by 6 agents, 7 findings addressed:

- Add missing logger.warning before raise in _validate_sources loop
- Extract _check_velocity_drop/_check_velocity_spike helpers (50-line limit)
- Extract _match_event helper in ExternalTriggerStrategy (50-line limit)
- Add SPRINT_AUTO_TRANSITION log in throughput-adaptive transition
- Update design spec: add roadmap subsection, observability events table
- Export BudgetDrivenStrategy/EventDrivenStrategy from workflow __init__
- Add test coverage: whitespace matching, non-dict source filtering
…abbit

- Edge-triggered matching: ThroughputAdaptiveStrategy tracks per-ceremony
  anomaly state, fires only on transition into drop/spike (not repeatedly
  during sustained anomaly). ExternalTriggerStrategy uses count-based
  edge-triggering so buffered events fire once per occurrence.
- Extract _validate_single_source to bring _validate_sources under 50 lines
- Wrap TypeError as ValueError in both validate_strategy_config methods
  (protocol contract says ValueError)
- Deep-copy + MappingProxyType for source dicts in on_sprint_activated
- Truncate logged config values to prevent log flooding
- Add math.isfinite() guard for threshold validation
- Document _blocked_count informational purpose
- Improve class docstrings (baseline freezing, both event entry points)
- Add 15 new tests: edge-triggering behavior (5), boundary inputs (4),
  NaN/Inf validation (2), default transition threshold (1), non-string
  event names (3)
- Extract _determine_anomaly_state and _update_and_check_edge_trigger
  from should_fire_ceremony to bring it under the 50-line function limit
- Consume transition event from _received_events in
  should_transition_sprint for one-shot consistency
… bad sources

- Remove _KEY_ON_DROP/_KEY_ON_SPIKE from _KNOWN_CONFIG_KEYS since they
  are per-ceremony keys (read via get_ceremony_config), not sprint-level
  keys. Putting them in sprint config now correctly raises 'Unknown
  config keys' instead of being silently accepted and ignored.
- Add warning log when on_sprint_activated filters non-dict entries
  from the sources list, consistent with event_driven's lenient-path
  warning pattern.
…from sprint config

- Change _ceremony_last_fired_counts key from f-string to tuple
  (ceremony_name, event_name) to prevent collision when names
  contain colons
- Remove _KEY_ON_EXTERNAL from _KNOWN_CONFIG_KEYS -- it is a
  per-ceremony key (read via get_ceremony_config), not a sprint-level
  key. Same fix as on_drop/on_spike in throughput_adaptive.
@Aureliolo Aureliolo force-pushed the feat/ceremony-strategies-batch2 branch from 9cf1953 to 61e9ff9 Compare April 2, 2026 19:00
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview April 2, 2026 19:01 — with GitHub Actions Inactive
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/synthorg/engine/workflow/strategies/external_trigger.py (1)

214-232: 🧹 Nitpick | 🔵 Trivial

Non-dict source entries are silently filtered instead of failing fast.

The current implementation filters out non-dict entries with a warning (lines 219-225), but per coding guidelines, validation at system boundaries should fail fast. If sources is provided but contains invalid entries, that's a configuration error that should be surfaced immediately.

However, this is on_sprint_activated (runtime), not validate_strategy_config (config validation). The validation path in _validate_sources (lines 470-496) does fail fast. This lenient runtime behavior may be intentional for graceful degradation.

Consider adding a comment clarifying this design choice, or align both paths to fail fast.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 214
- 232, The runtime handling in on_sprint_activated silently filters non-dict
entries from strategy_config.get(_KEY_SOURCES) and only logs
SPRINT_CEREMONY_SKIPPED, which diverges from the fail-fast behavior in
_validate_sources; change this to validate and raise a clear exception instead
of filtering: detect non-dict entries in raw_sources (the same bad_indices
logic), and if any exist raise a ValueError (or ConfigError) with context
(including indices and strategy="external_trigger") rather than calling
logger.warning, then set self._sources from the validated list; alternatively,
if the lenient behavior is intentional, add a clarifying comment in
on_sprint_activated referencing _validate_sources to explain why runtime
tolerates non-dict entries.
src/synthorg/engine/workflow/strategies/throughput_adaptive.py (1)

303-344: 🧹 Nitpick | 🔵 Trivial

Missing INFO logs for lifecycle state transitions.

Per coding guidelines, "All state transitions must log at INFO." Both on_sprint_activated and on_sprint_deactivated modify the full anomaly-tracking state but don't emit INFO-level logs.

Consider adding structured INFO logs on activation (with configured thresholds/window) and deactivation for observability when debugging scheduler behavior.

📋 Suggested additions
     async def on_sprint_activated(
         self,
         sprint: Sprint,  # noqa: ARG002
         config: SprintConfig,
     ) -> None:
         ...
         self._last_anomaly_state = {}
+        logger.info(
+            SPRINT_CEREMONY_SCHEDULER_STARTED,  # or a new event constant
+            drop_threshold_pct=self._drop_threshold_pct,
+            spike_threshold_pct=self._spike_threshold_pct,
+            window_size=self._window_size,
+            strategy="throughput_adaptive",
+        )

     async def on_sprint_deactivated(self) -> None:
         """Clear all internal state."""
+        logger.info(
+            SPRINT_CEREMONY_SCHEDULER_STOPPED,  # or a new event constant
+            strategy="throughput_adaptive",
+        )
         self._completion_timestamps = deque(maxlen=_DEFAULT_WINDOW_SIZE)
         ...

Based on learnings: "All state transitions in Python must log at INFO level."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py` around lines
303 - 344, Both on_sprint_activated and on_sprint_deactivated update the
anomaly-tracking state but don't emit INFO logs; add structured INFO-level
logging at the start or end of on_sprint_activated that logs the resolved values
_drop_threshold_pct, _spike_threshold_pct, and _window_size (and optionally
initial _baseline_rate/_blocked_count) and add an INFO log in
on_sprint_deactivated that logs that state was cleared along with the values
being reset (e.g., previous
_drop_threshold_pct/_spike_threshold_pct/_window_size), using the module/class
logger so lifecycle transitions are always visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/external_trigger.py`:
- Around line 214-232: The runtime handling in on_sprint_activated silently
filters non-dict entries from strategy_config.get(_KEY_SOURCES) and only logs
SPRINT_CEREMONY_SKIPPED, which diverges from the fail-fast behavior in
_validate_sources; change this to validate and raise a clear exception instead
of filtering: detect non-dict entries in raw_sources (the same bad_indices
logic), and if any exist raise a ValueError (or ConfigError) with context
(including indices and strategy="external_trigger") rather than calling
logger.warning, then set self._sources from the validated list; alternatively,
if the lenient behavior is intentional, add a clarifying comment in
on_sprint_activated referencing _validate_sources to explain why runtime
tolerates non-dict entries.

In `@src/synthorg/engine/workflow/strategies/throughput_adaptive.py`:
- Around line 303-344: Both on_sprint_activated and on_sprint_deactivated update
the anomaly-tracking state but don't emit INFO logs; add structured INFO-level
logging at the start or end of on_sprint_activated that logs the resolved values
_drop_threshold_pct, _spike_threshold_pct, and _window_size (and optionally
initial _baseline_rate/_blocked_count) and add an INFO log in
on_sprint_deactivated that logs that state was cleared along with the values
being reset (e.g., previous
_drop_threshold_pct/_spike_threshold_pct/_window_size), using the module/class
logger so lifecycle transitions are always visible.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e0a8d69a-7d93-4a0b-8eae-b23304b7adbb

📥 Commits

Reviewing files that changed from the base of the PR and between 9cf1953 and 61e9ff9.

📒 Files selected for processing (8)
  • docs/design/ceremony-scheduling.md
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/observability/events/workflow.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.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). (2)
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Build Backend
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations in Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: use except A, B: (no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
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_field for derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/observability/events/workflow.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every Python 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 Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always be logger (not _logger, not log)
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
Use structured logging with kwargs in Python: always logger.info(EVENT, key=value) -- never logger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python 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

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/observability/events/workflow.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/engine/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

RetryExhaustedError in Python signals that all retries failed; the engine layer catches this to trigger fallback chains

Files:

  • src/synthorg/engine/workflow/strategies/__init__.py
  • src/synthorg/engine/workflow/__init__.py
  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Documentation files in docs/ are Markdown, built with Zensical, configured in mkdocs.yml; design spec in docs/design/ (12 pages), Architecture in docs/architecture/, Roadmap in docs/roadmap/

Files:

  • docs/design/ceremony-scheduling.md
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: All Python test files must use @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, or @pytest.mark.slow markers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer @pytest.mark.parametrize for testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given + @settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions

Files:

  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • tests/unit/engine/workflow/strategies/test_throughput_adaptive.py
🧠 Learnings (35)
📚 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/engine/workflow/__init__.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/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)

Applied to files:

  • src/synthorg/engine/workflow/__init__.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
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/engine/workflow/__init__.py
  • src/synthorg/observability/events/workflow.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/observability/events/workflow.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/observability/events/workflow.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/observability/events/workflow.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/workflow.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/observability/events/workflow.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
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/observability/events/workflow.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 src/synthorg/**/*.py : Always 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 rather than using string literals

Applied to files:

  • src/synthorg/observability/events/workflow.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/workflow.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/observability/events/workflow.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : All state transitions must log at INFO.

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : All state transitions must log at INFO level

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to src/synthorg/**/*.py : All state transitions in Python must log at INFO level

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 src/**/*.py : All state transitions must log at INFO level

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : 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:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 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:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 : Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log object creation/internal flow/entry-exit at DEBUG

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
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:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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/engine/workflow/strategies/throughput_adaptive.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/observability/**/*.py : Observability must use structured logging with correlation tracking and log sinks

Applied to files:

  • src/synthorg/engine/workflow/strategies/throughput_adaptive.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 src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 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.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
🔇 Additional comments (12)
src/synthorg/engine/workflow/strategies/external_trigger.py (3)

74-74: Missing "mcp" in valid source types.

The docstring (line 4) and PR objectives mention MCP invocations as a valid external trigger source, but _VALID_SOURCE_TYPES only includes "webhook" and "git_event". MCP-originated triggers will fail validation.

-_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event"})
+_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event", "mcp"})

102-106: LGTM!

The initialization correctly uses a tuple key (ceremony_name, event_name) for _ceremony_last_fired_counts, avoiding the colon-collision issue from the previous review.


377-408: LGTM! Edge-triggered matching correctly implemented.

The _match_event method implements proper consume-once semantics:

  • Context events fire immediately (one-shot by nature)
  • Buffered events use count comparison: each on_external_event increments the count, and a ceremony only fires when count > last-fired count

This addresses the previous review concern about buffered events staying "hot" for the rest of the sprint.

src/synthorg/engine/workflow/strategies/__init__.py (1)

17-19: LGTM!

The new strategy imports and exports follow the established pattern and maintain alphabetical ordering in __all__.

Also applies to: 26-28, 34-34, 37-37

src/synthorg/engine/workflow/__init__.py (1)

58-66: LGTM!

The workflow package correctly re-exports the new strategies alongside existing ones, maintaining alphabetical ordering in __all__.

Also applies to: 83-83, 91-92, 105-105

docs/design/ceremony-scheduling.md (1)

666-673: LGTM!

Documentation correctly enumerates the new observability event constants and describes the implemented strategies. The roadmap section accurately notes that scheduler lifecycle-hook wiring remains pending.

Also applies to: 714-720

src/synthorg/observability/events/workflow.py (1)

156-188: LGTM!

The new observability event constants follow the established naming convention (workflow.sprint.<event_name>) and include descriptive docstrings. The constants align with usage in the strategy implementations.

tests/unit/engine/workflow/strategies/test_throughput_adaptive.py (1)

1-734: LGTM!

Comprehensive test suite covering:

  • Protocol conformance and metadata
  • Cold start behavior and baseline establishment
  • Drop/spike detection at various thresholds
  • Edge-triggered firing semantics (fire once, recovery, re-fire)
  • Sprint transition gating conditions
  • Lifecycle state management
  • Configuration validation with parametrized edge cases

Time-sensitive tests correctly mock time.monotonic() per coding guidelines.

tests/unit/engine/workflow/strategies/test_external_trigger.py (1)

1-591: LGTM!

Comprehensive test suite covering:

  • Protocol conformance and metadata
  • Context events vs buffered events matching
  • Edge-triggered firing semantics (fire once per occurrence)
  • Independent ceremony consumption of same event
  • Sprint transition with buffered event consumption
  • Lifecycle state clearing and source registration
  • Input validation with parametrized edge cases
src/synthorg/engine/workflow/strategies/throughput_adaptive.py (3)

129-179: LGTM!

The should_fire_ceremony method has been refactored per previous review feedback:

  • Delegates anomaly classification to _determine_anomaly_state
  • Delegates edge-trigger bookkeeping to _update_and_check_edge_trigger
  • Main method is now concise and readable

Edge-triggered semantics correctly fire only on transition into anomaly state.

Also applies to: 181-203


420-448: LGTM!

The validate_strategy_config correctly rejects on_drop and on_spike at sprint level (they're ceremony-level keys), addressing the previous review concern about validation gaps.


629-646: Remove unused _validate_bool_key static method.

This method is defined at lines 629–646 but never called anywhere in the file or codebase. It appears to be dead code since on_drop and on_spike validation happens elsewhere (ceremony-level validation via _resolve_bool).

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/external_trigger.py (1)

74-75: ⚠️ Potential issue | 🟠 Major

Accept MCP sources in _VALID_SOURCE_TYPES.

Line 74 still omits "mcp", so a documented sources: [{"type": "mcp"}] config will be rejected in _validate_single_source() even though the module docstring and PR objectives both say MCP-triggered ceremonies are supported.

Suggested fix
-_VALID_SOURCE_TYPES: frozenset[str] = frozenset({"webhook", "git_event"})
+_VALID_SOURCE_TYPES: frozenset[str] = frozenset(
+    {"webhook", "git_event", "mcp"}
+)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 74
- 75, The _VALID_SOURCE_TYPES constant is missing the "mcp" entry which causes
_validate_single_source() to reject valid MCP sources; update the frozenset
assigned to _VALID_SOURCE_TYPES (the symbol _VALID_SOURCE_TYPES in
external_trigger.py) to include "mcp" so that _validate_single_source() accepts
sources with type "mcp". Ensure only the set literal is changed (add "mcp" to
frozenset({"webhook", "git_event", "mcp"})) and leave validation logic in
_validate_single_source() untouched.
🤖 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/engine/workflow/strategies/external_trigger.py`:
- Around line 514-528: Add an explicit string-type check for the variable
source_type immediately after the existing None check in the validation block
for sources (before testing membership in _VALID_SOURCE_TYPES); if not
isinstance(source_type, str) log the same SPRINT_STRATEGY_CONFIG_INVALID with
strategy="external_trigger", key=f"sources[{index}].type" and a helpful value
(e.g., the actual type) via logger.warning and raise a ValueError with a clear
message so unhashable types (dict/list) produce the intended validation error
rather than a TypeError from the membership test.

---

Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/external_trigger.py`:
- Around line 74-75: The _VALID_SOURCE_TYPES constant is missing the "mcp" entry
which causes _validate_single_source() to reject valid MCP sources; update the
frozenset assigned to _VALID_SOURCE_TYPES (the symbol _VALID_SOURCE_TYPES in
external_trigger.py) to include "mcp" so that _validate_single_source() accepts
sources with type "mcp". Ensure only the set literal is changed (add "mcp" to
frozenset({"webhook", "git_event", "mcp"})) and leave validation logic in
_validate_single_source() untouched.
🪄 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: 067d8103-e9a3-4eb3-ac55-b7f1248eb81b

📥 Commits

Reviewing files that changed from the base of the PR and between 61e9ff9 and 3e1a671.

📒 Files selected for processing (1)
  • src/synthorg/engine/workflow/strategies/external_trigger.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 (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations in Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: use except A, B: (no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
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_field for derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)

Files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every Python 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 Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always be logger (not _logger, not log)
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
Use structured logging with kwargs in Python: always logger.info(EVENT, key=value) -- never logger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python 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

Files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/engine/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

RetryExhaustedError in Python signals that all retries failed; the engine layer catches this to trigger fallback chains

Files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
🧠 Learnings (10)
📚 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 src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 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.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py

Comment on lines +514 to +528
source_type = entry.get("type")
if source_type is None:
msg = f"sources[{index}] must have a 'type' key"
logger.warning(
SPRINT_STRATEGY_CONFIG_INVALID,
strategy="external_trigger",
key=f"sources[{index}].type",
value=None,
)
raise ValueError(msg)
if source_type not in _VALID_SOURCE_TYPES:
msg = (
f"sources[{index}].type must be one of "
f"{sorted(_VALID_SOURCE_TYPES)}, got {source_type!r}"
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
valid = frozenset({"webhook", "git_event"})
for value in ({}, []):
    try:
        print(type(value).__name__, value in valid)
    except Exception as exc:
        print(type(value).__name__, type(exc).__name__, exc)
PY

Repository: Aureliolo/synthorg

Length of output: 139


🏁 Script executed:

cat -n src/synthorg/engine/workflow/strategies/external_trigger.py | sed -n '510,535p'

Repository: Aureliolo/synthorg

Length of output: 1213


🏁 Script executed:

rg "_VALID_SOURCE_TYPES" src/synthorg/engine/workflow/strategies/external_trigger.py -B 2 -A 2

Repository: Aureliolo/synthorg

Length of output: 489


🏁 Script executed:

# Check what container type _VALID_SOURCE_TYPES is
rg "_VALID_SOURCE_TYPES\s*=" src/synthorg/engine/workflow/strategies/external_trigger.py -A 3

Repository: Aureliolo/synthorg

Length of output: 44


Add string type validation before set membership test.

Line 524 checks if source_type not in _VALID_SOURCE_TYPES: without first validating that source_type is hashable. If the config supplies a dict or list value for sources[i].type, Python raises TypeError: unhashable type before the validation branch can log the proper error message, surfacing an opaque error instead of the intended validation feedback.

Add an isinstance(source_type, str) check immediately after the None check:

         source_type = entry.get("type")
         if source_type is None:
             msg = f"sources[{index}] must have a 'type' key"
             logger.warning(
                 SPRINT_STRATEGY_CONFIG_INVALID,
                 strategy="external_trigger",
                 key=f"sources[{index}].type",
                 value=None,
             )
             raise ValueError(msg)
+        if not isinstance(source_type, str):
+            msg = f"sources[{index}].type must be a string"
+            logger.warning(
+                SPRINT_STRATEGY_CONFIG_INVALID,
+                strategy="external_trigger",
+                key=f"sources[{index}].type",
+                value=type(source_type).__name__,
+            )
+            raise ValueError(msg)
         if source_type not in _VALID_SOURCE_TYPES:

This satisfies the coding guidelines: "Validate at system boundaries in Python (user input, external APIs, config files)" and "Handle errors explicitly in Python; never silently swallow exceptions."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 514
- 528, Add an explicit string-type check for the variable source_type
immediately after the existing None check in the validation block for sources
(before testing membership in _VALID_SOURCE_TYPES); if not
isinstance(source_type, str) log the same SPRINT_STRATEGY_CONFIG_INVALID with
strategy="external_trigger", key=f"sources[{index}].type" and a helpful value
(e.g., the actual type) via logger.warning and raise a ValueError with a clear
message so unhashable types (dict/list) produce the intended validation error
rather than a TypeError from the membership test.

Unhashable types (list, dict) as source_type would raise an unhandled
TypeError from frozenset.__contains__. Add explicit isinstance(str)
check with a clear error message.
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/synthorg/engine/workflow/strategies/external_trigger.py (2)

74-74: ⚠️ Potential issue | 🟠 Major

Accept the MCP source type here too.

The PR objective and this module docstring both advertise MCP-triggered sources, but _VALID_SOURCE_TYPES still only allows webhook and git_event. A valid MCP source config will still fail validation and never be registered.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` at line 74,
_VALID_SOURCE_TYPES currently omits the "mcp" type so MCP-triggered sources fail
validation; update the frozenset _VALID_SOURCE_TYPES in external_trigger.py to
include "mcp" (i.e., add the string "mcp" alongside "webhook" and "git_event")
so that functions and validation logic that reference _VALID_SOURCE_TYPES will
accept MCP source configs.

317-331: ⚠️ Potential issue | 🟠 Major

This cap is counting event history, not pending buffer size.

_received_events only grows here and is never pruned when ceremony matches consume buffered occurrences, so after 256 unique names the strategy will reject every new name for the rest of the sprint even if nothing is still pending. That turns _MAX_RECEIVED_EVENTS into an ever_seen_this_sprint limit rather than a real buffer bound.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/workflow/strategies/external_trigger.py` around lines 317
- 331, The current check uses _received_events (which only grows) to enforce
_MAX_RECEIVED_EVENTS, causing a permanent unique-event cap; change the logic to
measure pending buffer size instead: base the cap on the number of events with
positive counts in _event_counts (or maintain a separate pending set updated
when counts go >0/==0), so that when a ceremony consumes occurrences you
decrement _event_counts[cleaned] and remove that name from the pending
set/_received_events when its count hits zero; update the pre-add check (the
block using cleaned, _received_events and _MAX_RECEIVED_EVENTS) to use this
pending count and ensure you still increment _event_counts[cleaned] and add to
the pending set only when count transitions from 0 to 1.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/external_trigger.py`:
- Line 74: _VALID_SOURCE_TYPES currently omits the "mcp" type so MCP-triggered
sources fail validation; update the frozenset _VALID_SOURCE_TYPES in
external_trigger.py to include "mcp" (i.e., add the string "mcp" alongside
"webhook" and "git_event") so that functions and validation logic that reference
_VALID_SOURCE_TYPES will accept MCP source configs.
- Around line 317-331: The current check uses _received_events (which only
grows) to enforce _MAX_RECEIVED_EVENTS, causing a permanent unique-event cap;
change the logic to measure pending buffer size instead: base the cap on the
number of events with positive counts in _event_counts (or maintain a separate
pending set updated when counts go >0/==0), so that when a ceremony consumes
occurrences you decrement _event_counts[cleaned] and remove that name from the
pending set/_received_events when its count hits zero; update the pre-add check
(the block using cleaned, _received_events and _MAX_RECEIVED_EVENTS) to use this
pending count and ensure you still increment _event_counts[cleaned] and add to
the pending set only when count transitions from 0 to 1.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 72ae9939-7c54-4d1f-b9ac-c62dabef09b7

📥 Commits

Reviewing files that changed from the base of the PR and between 3e1a671 and eeb59f4.

📒 Files selected for processing (2)
  • src/synthorg/engine/workflow/strategies/external_trigger.py
  • tests/unit/engine/workflow/strategies/test_external_trigger.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: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations in Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: use except A, B: (no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
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_field for derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)

Files:

  • tests/unit/engine/workflow/strategies/test_external_trigger.py
  • src/synthorg/engine/workflow/strategies/external_trigger.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: All Python test files must use @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, or @pytest.mark.slow markers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer @pytest.mark.parametrize for testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given + @settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions

Files:

  • tests/unit/engine/workflow/strategies/test_external_trigger.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every Python 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 Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always be logger (not _logger, not log)
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
Use structured logging with kwargs in Python: always logger.info(EVENT, key=value) -- never logger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python 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

Files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
src/synthorg/engine/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

RetryExhaustedError in Python signals that all retries failed; the engine layer catches this to trigger fallback chains

Files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
🧠 Learnings (15)
📚 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 src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 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.

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Validate at system boundaries in Python (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.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 src/synthorg/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators

Applied to files:

  • src/synthorg/engine/workflow/strategies/external_trigger.py
🔇 Additional comments (1)
src/synthorg/engine/workflow/strategies/external_trigger.py (1)

41-48: ⚠️ Potential issue | 🟠 Major

Move annotation-only imports out of TYPE_CHECKING.

Mapping, CeremonyEvalContext, SprintCeremonyConfig, and SprintConfig are used in public method annotations but imported only within the TYPE_CHECKING block. Under Python 3.14's lazy-annotation model, runtime access to __annotations__ or typing.get_type_hints() will fail with NameError because these names are not bound in the module namespace.

Move these imports to the module level to ensure they are available at runtime.

⛔ Skipped due to learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : No `from __future__ import annotations` in Python code; Python 3.14 has PEP 649 native lazy annotations
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations

@Aureliolo Aureliolo merged commit bb5c9a4 into main Apr 2, 2026
34 checks passed
@Aureliolo Aureliolo deleted the feat/ceremony-strategies-batch2 branch April 2, 2026 20:22
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview April 2, 2026 20:22 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: external-trigger ceremony scheduling strategy feat: throughput-adaptive ceremony scheduling strategy

2 participants