feat: event-driven and budget-driven ceremony scheduling strategies#995
feat: event-driven and budget-driven ceremony scheduling strategies#995
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds two new strategies: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Code Review
This pull request implements the Event-Driven and Budget-Driven ceremony scheduling strategies, alongside the PointsPerSprintVelocityCalculator and BudgetVelocityCalculator. The EventDrivenStrategy triggers ceremonies based on engine events with a debounce threshold, while the BudgetDrivenStrategy uses budget consumption percentages. The changes include new observability events, documentation updates, and comprehensive unit tests. Feedback was provided to ensure that the debounce_default value in EventDrivenStrategy is strictly checked for positive values during sprint activation to align with existing configuration validation.
| if isinstance(debounce_default, int) and not isinstance(debounce_default, bool): | ||
| self._debounce_default = debounce_default | ||
| else: | ||
| self._debounce_default = _DEFAULT_DEBOUNCE |
There was a problem hiding this comment.
The logic for setting self._debounce_default in on_sprint_activated is less strict than the validation in validate_strategy_config. The validation ensures debounce_default is a positive integer, but this method only checks that it's an integer (and not a boolean).
This could lead to unexpected behavior if an invalid value (e.g., 0 or a negative number) is present in the configuration and validation is somehow bypassed. A non-positive debounce value would cause ceremonies to fire more frequently than intended.
To improve robustness, I suggest making this check consistent with the validation logic by ensuring debounce_default is positive.
| if isinstance(debounce_default, int) and not isinstance(debounce_default, bool): | |
| self._debounce_default = debounce_default | |
| else: | |
| self._debounce_default = _DEFAULT_DEBOUNCE | |
| if ( | |
| isinstance(debounce_default, int) | |
| and not isinstance(debounce_default, bool) | |
| and debounce_default > 0 | |
| ): | |
| self._debounce_default = debounce_default | |
| else: | |
| self._debounce_default = _DEFAULT_DEBOUNCE |
There was a problem hiding this comment.
Pull request overview
This PR extends the pluggable sprint ceremony scheduling system with two additional scheduling strategies (event-driven and budget-driven) and adds corresponding velocity calculators, along with exports, observability event constants, tests, and design doc updates.
Changes:
- Add
EventDrivenStrategy+PointsPerSprintVelocityCalculatorfor reactive, debounced ceremony triggering. - Add
BudgetDrivenStrategy+BudgetVelocityCalculatorfor budget-threshold-based ceremony triggering andpts/EURvelocity. - Update exports, observability workflow event constants, shared test helpers, and design documentation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/synthorg/engine/workflow/strategies/event_driven.py | Implements event subscription + debounce-based ceremony firing and event-based sprint transition. |
| src/synthorg/engine/workflow/strategies/budget_driven.py | Implements budget consumption threshold ceremony firing and budget-threshold sprint transition. |
| src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py | Adds a simple pts/sprint velocity calculator. |
| src/synthorg/engine/workflow/velocity_calculators/budget.py | Adds a pts/EUR velocity calculator with budget-aware rolling average. |
| src/synthorg/engine/workflow/strategies/init.py | Exports the new strategies. |
| src/synthorg/engine/workflow/velocity_calculators/init.py | Exports the new velocity calculators. |
| src/synthorg/observability/events/workflow.py | Adds workflow observability event constants for the new strategy/calculator behaviors. |
| tests/unit/engine/workflow/strategies/test_event_driven.py | Unit tests for event-driven strategy behavior and config validation. |
| tests/unit/engine/workflow/strategies/test_budget_driven.py | Unit tests for budget-driven strategy behavior and config validation. |
| tests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.py | Unit tests for the points-per-sprint calculator. |
| tests/unit/engine/workflow/velocity_calculators/test_budget.py | Unit tests for the budget velocity calculator. |
| tests/unit/engine/workflow/strategies/conftest.py | Extends shared test context builder to support budget + external events fields. |
| docs/design/ceremony-scheduling.md | Updates “shipped implementations”, observability constants table, and roadmap sections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if budget is None: | ||
| logger.debug( | ||
| VELOCITY_BUDGET_NO_BUDGET_CONSUMED, | ||
| sprint_id=record.sprint_id, | ||
| ) |
There was a problem hiding this comment.
VELOCITY_BUDGET_NO_BUDGET_CONSUMED is documented as covering both None and zero budget, and this method treats budget == 0.0 the same as None, but the debug log only fires for the None case. Either emit the event for both None and 0.0 (recommended for observability consistency) or adjust the event name/docs so they match the actual logging behavior.
| if budget is None: | |
| logger.debug( | |
| VELOCITY_BUDGET_NO_BUDGET_CONSUMED, | |
| sprint_id=record.sprint_id, | |
| ) | |
| logger.debug( | |
| VELOCITY_BUDGET_NO_BUDGET_CONSUMED, | |
| sprint_id=record.sprint_id, | |
| ) |
| # -- Budget velocity calculator events ---------------------------------------- | ||
|
|
||
| VELOCITY_BUDGET_NO_BUDGET_CONSUMED: str = "workflow.velocity.budget_no_budget_consumed" | ||
| """BudgetVelocityCalculator: record has None or zero budget_consumed.""" |
There was a problem hiding this comment.
The docstring for VELOCITY_BUDGET_NO_BUDGET_CONSUMED says it covers None or zero budget_consumed, but BudgetVelocityCalculator.compute() currently only emits this event when budget_consumed is None. Please align the event description with the actual behavior, or update the calculator to log for the zero case as well.
| """BudgetVelocityCalculator: record has None or zero budget_consumed.""" | |
| """BudgetVelocityCalculator: record has None for budget_consumed.""" |
| if on_event is None: | ||
| return False | ||
|
|
||
| debounce: int = config.get(_KEY_DEBOUNCE, self._debounce_default) |
There was a problem hiding this comment.
on_event/debounce are read from policy_override.strategy_config without any runtime type checking, and per-ceremony configs are not validated by CeremonyScheduler.activate_sprint() (it only validates sprint-level ceremony_policy.strategy_config). As a result, a mis-typed config (e.g., debounce: "5" or on_event: 123) can raise at runtime during comparisons/lookup. Consider adding defensive type checks/fallbacks here (or ensuring per-ceremony configs are validated upstream before evaluation).
| if on_event is None: | |
| return False | |
| debounce: int = config.get(_KEY_DEBOUNCE, self._debounce_default) | |
| # Validate on_event type early; mis-typed configs should not raise. | |
| if on_event is None: | |
| return False | |
| if not isinstance(on_event, str): | |
| logger.warning( | |
| "Invalid on_event type in ceremony config; expected str", | |
| ceremony=ceremony.name, | |
| on_event=on_event, | |
| strategy="event_driven", | |
| ) | |
| return False | |
| # Validate and normalize debounce; fall back to default on invalid values. | |
| raw_debounce: Any = config.get(_KEY_DEBOUNCE, self._debounce_default) | |
| debounce: int | |
| if isinstance(raw_debounce, int): | |
| debounce = raw_debounce if raw_debounce > 0 else self._debounce_default | |
| else: | |
| try: | |
| # Attempt a best-effort coercion (e.g., from "5"). | |
| coerced = int(raw_debounce) # type: ignore[arg-type] | |
| except (TypeError, ValueError): | |
| logger.warning( | |
| "Invalid debounce value in ceremony config; using default", | |
| ceremony=ceremony.name, | |
| debounce=raw_debounce, | |
| debounce_default=self._debounce_default, | |
| strategy="event_driven", | |
| ) | |
| debounce = self._debounce_default | |
| else: | |
| debounce = coerced if coerced > 0 else self._debounce_default |
| if not thresholds: | ||
| return False | ||
|
|
||
| budget_pct = context.budget_consumed_fraction * _MAX_THRESHOLD_PCT | ||
| fired = self._fired_thresholds.setdefault(ceremony.name, set()) | ||
|
|
||
| for threshold in sorted(thresholds): |
There was a problem hiding this comment.
thresholds is pulled from policy_override.strategy_config and then iterated via sorted(thresholds) without runtime type checking, but per-ceremony configs are not validated by CeremonyScheduler.activate_sprint() (it only validates sprint-level ceremony_policy.strategy_config). If a user supplies a non-list/iterable value (e.g. budget_thresholds: 50), this will raise at runtime. Consider guarding thresholds to list[int|float] (or validating per-ceremony configs upstream) to avoid taking down the scheduler on bad config.
| if not thresholds: | |
| return False | |
| budget_pct = context.budget_consumed_fraction * _MAX_THRESHOLD_PCT | |
| fired = self._fired_thresholds.setdefault(ceremony.name, set()) | |
| for threshold in sorted(thresholds): | |
| # Normalize and validate thresholds from config to avoid runtime errors | |
| if thresholds is None: | |
| return False | |
| # Allow a single numeric value to be provided instead of a list | |
| if isinstance(thresholds, (int, float)): | |
| thresholds = [thresholds] | |
| if not isinstance(thresholds, (list, tuple, set)): | |
| logger.error( | |
| "Invalid %s for ceremony %s: expected list/tuple/set of numbers, got %r", | |
| _KEY_BUDGET_THRESHOLDS, | |
| ceremony.name, | |
| thresholds, | |
| ) | |
| return False | |
| numeric_thresholds: list[float] = [] | |
| for t in thresholds: | |
| if isinstance(t, (int, float)): | |
| numeric_thresholds.append(float(t)) | |
| else: | |
| logger.error( | |
| "Invalid threshold value in %s for ceremony %s: expected number, got %r", | |
| _KEY_BUDGET_THRESHOLDS, | |
| ceremony.name, | |
| t, | |
| ) | |
| return False | |
| if not numeric_thresholds: | |
| return False | |
| budget_pct = context.budget_consumed_fraction * _MAX_THRESHOLD_PCT | |
| fired = self._fired_thresholds.setdefault(ceremony.name, set()) | |
| for threshold in sorted(numeric_thresholds): |
docs/design/ceremony-scheduling.md
Outdated
| ### Shipped in #971 + #972 (Event-Driven + Budget-Driven Strategies) | ||
|
|
||
| - `EventDrivenStrategy` -- reactive ceremony firing on engine events with configurable debounce | ||
| - `PointsPerSprintVelocityCalculator` -- `pts/sprint` for event-driven and external-trigger strategies | ||
| - `BudgetDrivenStrategy` -- ceremony firing at cost-consumption thresholds |
There was a problem hiding this comment.
This section marks Event-Driven and Budget-Driven strategies as "Shipped", but the current CeremonyScheduler still hardcodes budget_consumed_fraction=0.0 / external_events=() and only wires on_task_completed (no on_budget_updated, on_task_added, on_task_blocked, or on_external_event). Without that runtime wiring, these strategies won’t actually fire/transition in production. Either adjust the roadmap wording to reflect "implemented but not yet wired" or include the missing scheduler integrations.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workflow/strategies/event_driven.py`:
- Around line 195-218: In on_sprint_activated, add a debug log when the
retrieved debounce_default fails the int (non-bool) check and you fall back to
_DEFAULT_DEBOUNCE; specifically, after evaluating debounce_default and before
assigning self._debounce_default = _DEFAULT_DEBOUNCE, call the module/class
logger (or self.logger) to emit a debug message including the invalid
debounce_default value and that _DEFAULT_DEBOUNCE is being used, referencing
debounce_default, self._debounce_default and _DEFAULT_DEBOUNCE so it's easy to
locate in the on_sprint_activated method.
In `@src/synthorg/engine/workflow/velocity_calculators/budget.py`:
- Around line 109-116: The secondary metric "sprints_averaged" currently uses
len(recent) (the window size) which is incorrect because some records in recent
are filtered out when their budget_consumed is missing or non-positive; update
the logic that builds the VelocityMetrics return in the function/method that
returns this object so that "sprints_averaged" counts only the records actually
included in the average (i.e., the number of recent entries with a valid
budget_consumed > 0 or whatever inclusion predicate you already use when
computing total_budget and total_pts) and set secondary["sprints_averaged"] to
that included count instead of len(recent), keeping the rest of the return shape
(primary_value, primary_unit, secondary) the same.
🪄 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: 07bfaee9-cae7-4fc7-9991-0d46fb5d29fc
📒 Files selected for processing (13)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.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: Agent
- 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 (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) per PEP 758 except syntax — ruff enforces this on Python 3.14
Type hints required on all public functions, enforced by mypy strict mode
Docstrings must use Google style and are required on public classes and functions, enforced by ruff D rules
Never mutate existing objects — create new objects; for non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models usingmodel_copy(update=...)for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) for structured concurrency instead of barecreate_task
Line length must be 88 characters, enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Logging variable name must always belogger(not_logger, notlog)
Always use structured logging withlogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WAR...
Files:
src/synthorg/engine/workflow/strategies/__init__.pytests/unit/engine/workflow/strategies/conftest.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/test_budget_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pytests/unit/engine/workflow/strategies/test_event_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code, except inobservability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.py
Always use event name constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
src/synthorg/{engine,providers}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto classify tests
Useasyncio_mode = "auto"for async tests — no manual@pytest.mark.asyncioneeded
Global timeout is 30 seconds per test inpyproject.toml— do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing in Python with@given+@settingsdecorators
Use Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()instead of widening timing margins; for tasks that must block indefinitely, useasyncio.Event().wait()
No-redundant-timeout pre-commit hook enforced
Files:
tests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Docs in
docs/(Markdown, built with Zensical, config:mkdocs.yml); design spec:docs/design/(12 pages); architecture:docs/architecture/; roadmap:docs/roadmap/
Files:
docs/design/ceremony-scheduling.md
🧠 Learnings (13)
📚 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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pytests/unit/engine/workflow/strategies/test_budget_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pytests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.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-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-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-04-02T09:13:44.997Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T09:13:44.997Z
Learning: Applies to src/synthorg/**/*.py : Always use event name 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> 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-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-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-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-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-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 : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.py
🔇 Additional comments (19)
src/synthorg/engine/workflow/strategies/__init__.py (1)
8-10: Re-export updates are correct and coherent.The new strategy imports and
__all__entries are aligned and keep the public API explicit.Also applies to: 14-16, 25-27
tests/unit/engine/workflow/strategies/conftest.py (1)
62-64: Fixture extension is well-scoped.The added budget and external-event fields are correctly threaded through to
CeremonyEvalContext, enabling the new strategy tests cleanly.Also applies to: 86-87, 89-89
src/synthorg/engine/workflow/velocity_calculators/__init__.py (1)
7-9: Velocity calculator exports are correctly integrated.The new calculator classes are imported and exposed consistently via
__all__.Also applies to: 16-18, 24-24, 27-27
docs/design/ceremony-scheduling.md (1)
444-452: Design doc updates are consistent with the delivered feature set.The shipped calculators, observability constants, and roadmap status are documented clearly and in the right sections.
Also applies to: 660-665, 696-703
tests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.py (1)
30-119: Comprehensive unit coverage for points-per-sprint calculator.The tests validate properties, core computation, rolling averages, and key edge conditions (
empty records,window=0,zero committed) effectively.src/synthorg/observability/events/workflow.py (1)
123-151: Observability event constants are cleanly extended.The new constants fit the existing workflow event namespace and support strategy-specific instrumentation without introducing literal-event drift.
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py (1)
25-96: Calculator implementation is solid and protocol-aligned.
compute()androlling_average()behavior is clear, handles boundary inputs safely, and returns consistentVelocityMetricsunits/secondary fields.tests/unit/engine/workflow/velocity_calculators/test_budget.py (1)
32-191: Budget velocity tests are thorough and well-targeted.The suite covers expected outputs, exclusion rules for invalid budget consumption, and rolling-window edge cases comprehensively.
src/synthorg/engine/workflow/velocity_calculators/budget.py (1)
28-72: LGTM!The
BudgetVelocityCalculatorclass correctly implements theVelocityCalculatorprotocol. Thecompute()method properly handles edge cases (budget_consumedbeingNoneor0.0), avoids division by zero, and includes appropriate secondary metrics.tests/unit/engine/workflow/strategies/test_budget_driven.py (2)
1-99: LGTM!The test module provides comprehensive coverage for
BudgetDrivenStrategy, including protocol conformance, threshold crossing/non-firing, independent per-ceremony tracking, lifecycle resets, and configuration validation. The helper functions cleanly abstract ceremony and config creation.
100-399: Thorough edge-case coverage.The validation tests properly verify rejection of boolean inputs for thresholds (lines 382-399), which guards against Python's
isinstance(True, int)returningTrue. The lifecycle tests correctly verify state reset behavior.src/synthorg/engine/workflow/strategies/budget_driven.py (3)
60-76: LGTM!The
BudgetDrivenStrategyclass is well-structured with__slots__for memory efficiency and clear state tracking via_fired_thresholds. The class docstring accurately describes the one-at-a-time threshold firing behavior.
80-145: LGTM!The
should_fire_ceremonyimplementation correctly handles the "fire lowest unfired threshold" semantics. The explicitNotedocumenting state mutation is helpful for callers. Logging follows guidelines with INFO for state transitions and DEBUG for skipped evaluations.
287-338: LGTM!The
validate_strategy_configmethod properly guards against boolean misuse withisinstance(t, bool)checks before theint | floattype check. The duplicate threshold detection and range validation are correctly implemented.tests/unit/engine/workflow/strategies/test_event_driven.py (2)
1-81: LGTM!The test module provides comprehensive coverage for
EventDrivenStrategy. The helper functions are well-designed, and protocol conformance tests verify the strategy type and default velocity calculator correctly.
84-478: Excellent test coverage for event-driven behavior.The tests thoroughly verify debounce mechanics (counter reset after fire, per-ceremony tracking), multiple event types (
task_completed,task_blocked,task_added,budget_updated, external events), and sprint transitions via both external events and internal counters. The validation tests properly cover boolean rejection for debounce values.src/synthorg/engine/workflow/strategies/event_driven.py (3)
71-88: LGTM!The
EventDrivenStrategyclass correctly initializes state tracking dictionaries and uses__slots__for memory efficiency. The default debounce of 5 is reasonable.
91-138: LGTM!The
should_fire_ceremonyimplementation correctly tracks per-ceremony firing state via_ceremony_last_fire_atand resets relative to the global event counter. The debounce resolution logic (per-ceremony override →debounce_default→_DEFAULT_DEBOUNCE) is clean.
307-362: LGTM!The
validate_strategy_configmethod properly validates all config keys with appropriate type and value checks. The_incrementhelper is clean with consistent logging. The_get_ceremony_configstatic method correctly handles missingpolicy_override.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #995 +/- ##
==========================================
+ Coverage 91.67% 91.69% +0.01%
==========================================
Files 645 649 +4
Lines 35141 35488 +347
Branches 3448 3493 +45
==========================================
+ Hits 32216 32539 +323
- Misses 2314 2330 +16
- Partials 611 619 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/ceremony-scheduling.md`:
- Around line 696-705: EventDrivenStrategy is marked as shipped but depends on
lifecycle hooks the CeremonyScheduler doesn't dispatch (on_task_added,
on_task_blocked, on_budget_updated, on_external_event), so its event-counter
increments and ceremony triggers never run; either wire these hooks into the
CeremonyScheduler (ensure CeremonyScheduler calls
EventDrivenStrategy.on_task_added, on_task_blocked, on_budget_updated,
on_external_event at the appropriate task/budget/external-event lifecycle points
and add tests exercising SPRING_CEREMONY_EVENT_* observability constants) or
revert EventDrivenStrategy from the shipped list and move it back to follow-up
until integration is complete; reference EventDrivenStrategy, CeremonyScheduler,
and the hook names when making the change.
In `@src/synthorg/engine/workflow/strategies/budget_driven.py`:
- Around line 110-183: The thresholds list and transition threshold must be
re-validated before numeric use: in should_fire_ceremony (after fetching
thresholds via get_ceremony_config and _KEY_BUDGET_THRESHOLDS) ensure thresholds
is a list of numeric values (reject/bypass booleans, convert numeric strings to
int/float when safe, drop or coerce invalid entries), log a warning via logger
when any entries are ignored or coerced, and only pass a cleaned list to
_find_crossed_threshold (or return False if no valid thresholds remain);
similarly in should_transition_sprint validate
strategy_config.get(_KEY_TRANSITION_THRESHOLD) is not bool and is a numeric in a
sensible range (>0 and <=_MAX_THRESHOLD_PCT), log and fall back to
_DEFAULT_TRANSITION_THRESHOLD_PCT when invalid (including negative, zero, >100,
or non-numeric strings) before computing transition_pct and budget_pct.
- Around line 86-136: Extract the repeated threshold normalization/validation
into a private helper (e.g. _normalize_budget_thresholds) that accepts the
ceremony or its config and returns either a validated list of thresholds or None
while emitting the same SPRINT_CEREMONY_SKIPPED logs and reasons
("no_budget_thresholds" and "invalid_budget_thresholds_type"); then replace the
duplicated logic in should_fire_ceremony and the other evaluator that processes
budget thresholds (the method directly adjacent that currently duplicates this
fallback/validation) to call that helper and pass its result into
_find_crossed_threshold, keeping the helper and updated methods under 50 lines
and preserving existing logging messages and behavior.
In `@src/synthorg/engine/workflow/strategies/event_driven.py`:
- Around line 356-393: The validate_strategy_config function raises ValueError
at multiple checks without logging; before each raise in
validate_strategy_config (the unknown keys check and validations for
_KEY_ON_EVENT, _KEY_TRANSITION_EVENT, _KEY_DEBOUNCE, _KEY_DEBOUNCE_DEFAULT and
max debounce), log the same error message at WARNING (using the module logger
instance, e.g., logger.warning(...)) including the offending key/value and
context (e.g., the config or key name and value) and then raise the ValueError
as before; ensure messages match the ValueError text so logs and exceptions are
consistent.
- Around line 98-166: The method should_fire_ceremony is over the 50-line limit;
extract the debounce validation into a private helper (e.g., _resolve_debounce)
that takes the ceremony config and ceremony.name and returns an int. Move the
logic that reads config.get(_KEY_DEBOUNCE, self._debounce_default), validates
(reject bool, non-int, <1), logs the SPRINT_CEREMONY_SKIPPED warning with
ceremony, reason="invalid_debounce", value, fallback=self._debounce_default and
strategy="event_driven", and returns the resolved int into _resolve_debounce,
then call it from should_fire_ceremony to assign debounce. Keep references to
_KEY_DEBOUNCE, _debounce_default, and the SPRINT_CEREMONY_SKIPPED logger call
consistent.
🪄 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: d71e75ab-9013-4ebd-841c-a873609e2f23
📒 Files selected for processing (10)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.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: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) per PEP 758 except syntax — ruff enforces this on Python 3.14
Type hints required on all public functions, enforced by mypy strict mode
Docstrings must use Google style and are required on public classes and functions, enforced by ruff D rules
Never mutate existing objects — create new objects; for non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models usingmodel_copy(update=...)for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) for structured concurrency instead of barecreate_task
Line length must be 88 characters, enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Logging variable name must always belogger(not_logger, notlog)
Always use structured logging withlogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WAR...
Files:
src/synthorg/engine/workflow/strategies/task_driven.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pytests/unit/engine/workflow/strategies/test_event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code, except inobservability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.py
Always use event name constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Files:
src/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/{engine,providers}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Docs in
docs/(Markdown, built with Zensical, config:mkdocs.yml); design spec:docs/design/(12 pages); architecture:docs/architecture/; roadmap:docs/roadmap/
Files:
docs/design/ceremony-scheduling.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto classify tests
Useasyncio_mode = "auto"for async tests — no manual@pytest.mark.asyncioneeded
Global timeout is 30 seconds per test inpyproject.toml— do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing in Python with@given+@settingsdecorators
Use Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()instead of widening timing margins; for tasks that must block indefinitely, useasyncio.Event().wait()
No-redundant-timeout pre-commit hook enforced
Files:
tests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/strategies/test_event_driven.py
🧠 Learnings (2)
📚 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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
tests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
tests/unit/engine/workflow/strategies/test_budget_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
🔇 Additional comments (11)
src/synthorg/engine/workflow/strategies/task_driven.py (2)
19-19: LGTM — shared helper import is correctly placed.The import of
get_ceremony_configfrom the new_helpersmodule is appropriately grouped with other local strategy imports and follows the project's import conventions.
102-103: LGTM — clean refactor to shared helper.The replacement of the static method
_get_ceremony_configwith the sharedget_ceremony_confighelper preserves identical behavior (returns{}forNonepolicy_override, otherwisestrategy_config or {}) while consolidating duplicated logic. The existing control flow readingtriggerfrom the config remains unchanged.src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py (1)
52-56: No issues found. The protocol atsrc/synthorg/engine/workflow/velocity_calculator.pydefinesrolling_average(self, records: Sequence[VelocityRecord], window: int)with thewindowparameter, and bothpoints_per_sprint.pyandbudget.pyimplementations match this signature exactly. The implementations are correctly substitutable for the protocol.> Likely an incorrect or invalid review comment.src/synthorg/engine/workflow/strategies/event_driven.py (8)
1-46: LGTM!Module docstring, imports, and logger setup follow guidelines correctly. Event constants are imported from the domain-specific observability module, and
get_logger(__name__)is used appropriately.
48-75: Good defensive limits and constant organization.The caps on debounce values, event name length, and distinct events prevent unbounded memory growth. Using
frozensetfor known config keys ensures immutability.
77-94: LGTM!Good use of
__slots__for memory efficiency. The mutable internal dictionaries for runtime event tracking are appropriate for a strategy that maintains state across a sprint lifecycle.
168-201: LGTM!Clean implementation with appropriate guard for non-ACTIVE sprints and good delegation to the helper method.
203-227: LGTM!Good observability with distinct log sources for external vs internal event detection. The tuple membership test is correct per the
CeremonyEvalContexttype definition.
231-267: LGTM!State reset on sprint activation is correct. The WARNING log for invalid
debounce_defaultvalues (lines 258-264) provides good diagnostics while the silent fallback forNoneis appropriate for the default case.
269-343: LGTM!Clean implementation of lifecycle hooks and event handlers. The delegation to
_incrementkeeps the handlers minimal while correctly conforming to the protocol interface.
397-424: LGTM!Good defensive limits with appropriate logging. The truncation of long event names in warning logs (line 403) prevents log bloat while still providing diagnostics.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/budget_driven.py (1)
156-169:⚠️ Potential issue | 🟡 MinorMissing range validation for
transition_thresholdat runtime.The defensive type check (lines 158-168) rejects booleans and non-numerics but doesn't validate the range. If sprint-level config bypasses
validate_strategy_config, values like-1,0, or150will be used as-is:
0or negative values → immediate transition on any budget update- Values
> 100→ sprint never auto-transitions🔧 Suggested fix to add range validation
if isinstance(raw_threshold, bool) or not isinstance( raw_threshold, int | float + ) or not (0 < raw_threshold <= _MAX_THRESHOLD_PCT): - ): logger.warning( SPRINT_CEREMONY_SKIPPED, - reason="invalid_transition_threshold_type", + reason="invalid_transition_threshold", value=raw_threshold, fallback=_DEFAULT_TRANSITION_THRESHOLD_PCT, strategy="budget_driven", ) raw_threshold = _DEFAULT_TRANSITION_THRESHOLD_PCT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/budget_driven.py` around lines 156 - 169, The code currently only type-checks raw_threshold but not its numeric range; add a runtime range validation after the existing type guard in budget_driven.py to ensure raw_threshold is between 0 and 100 inclusive, and if out of range log a SPRINT_CEREMONY_SKIPPED warning (same fields: reason="invalid_transition_threshold_range", value=raw_threshold, fallback=_DEFAULT_TRANSITION_THRESHOLD_PCT, strategy="budget_driven") and set raw_threshold = _DEFAULT_TRANSITION_THRESHOLD_PCT before assigning transition_pct; update any tests/usage of transition_pct to expect a clamped/defaulted value.
🤖 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/budget_driven.py`:
- Around line 380-392: The threshold validator currently accepts any int|float
but doesn't reject NaN/Inf; update the loop in the function that builds `valid`
from `raw` to also check finiteness (use math.isfinite on `t`) and skip
non-finite values, logging the same SPRINT_CEREMONY_SKIPPED event with a new
reason like "non_finite_threshold" (keep existing fields:
ceremony=ceremony_name, value=t, strategy="budget_driven"), so only finite
floats are appended to `valid` before returning `valid or None`.
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/budget_driven.py`:
- Around line 156-169: The code currently only type-checks raw_threshold but not
its numeric range; add a runtime range validation after the existing type guard
in budget_driven.py to ensure raw_threshold is between 0 and 100 inclusive, and
if out of range log a SPRINT_CEREMONY_SKIPPED warning (same fields:
reason="invalid_transition_threshold_range", value=raw_threshold,
fallback=_DEFAULT_TRANSITION_THRESHOLD_PCT, strategy="budget_driven") and set
raw_threshold = _DEFAULT_TRANSITION_THRESHOLD_PCT before assigning
transition_pct; update any tests/usage of transition_pct to expect a
clamped/defaulted value.
🪄 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: b04a027b-816c-408d-a0e4-11f68dd922f1
📒 Files selected for processing (2)
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.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: Deploy Preview
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- 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: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) per PEP 758 except syntax — ruff enforces this on Python 3.14
Type hints required on all public functions, enforced by mypy strict mode
Docstrings must use Google style and are required on public classes and functions, enforced by ruff D rules
Never mutate existing objects — create new objects; for non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models usingmodel_copy(update=...)for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) for structured concurrency instead of barecreate_task
Line length must be 88 characters, enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Logging variable name must always belogger(not_logger, notlog)
Always use structured logging withlogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WAR...
Files:
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code, except inobservability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.py
Always use event name constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Files:
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/{engine,providers}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
🧠 Learnings (15)
📚 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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.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/event_driven.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 : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.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/event_driven.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/event_driven.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/event_driven.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.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 error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.py
🔇 Additional comments (13)
src/synthorg/engine/workflow/strategies/event_driven.py (7)
1-46: Module setup and imports look good.The module docstring is comprehensive, logger is properly initialized using
get_logger(__name__), and imports follow the project conventions withTYPE_CHECKINGguards for type-only imports.
77-95: Class definition and initialization are well-structured.Good use of
__slots__for memory efficiency. The docstring clearly explains the debounce mechanism and state management.
98-151: The debounce helper extraction addresses the prior concern.The
_resolve_debouncehelper (lines 387-405) was extracted as suggested, keeping this method focused on the core firing logic. The debounce-based firing mechanism is correctly implemented.
153-212: Sprint transition logic is correct.The method properly checks only
ACTIVEsprints, supports both external events and internal counters as transition triggers, and logs transitions at INFO level as required.
216-258: Lifecycle hooks correctly manage state and validate configuration.The
on_sprint_activatedmethod now logs a warning whendebounce_defaultis invalid (lines 242-250), addressing the prior review feedback. State is properly cleared on both activation and deactivation.
353-383: Validation now logs before raising, addressing prior feedback.All
ValueErrorraises are now preceded bylogger.warningcalls with appropriate context. This satisfies the coding guideline that all error paths must log at WARNING or ERROR before raising.
407-434: Good defensive programming in_increment.The method protects against resource exhaustion by limiting event name length and distinct event count, logging warnings when limits are hit. The counter increment logs at DEBUG level appropriately.
src/synthorg/engine/workflow/strategies/budget_driven.py (6)
1-42: Module setup is correct.Module docstring clearly documents config keys. Logger is properly initialized using
get_logger(__name__).
86-123: Threshold validation was extracted to_resolve_thresholds, addressing prior feedback.The helper (lines 354-392) validates threshold types, filters out invalid elements with warnings, and returns
Nonefor entirely invalid configs. This keeps the method focused and under the 50-line limit.
185-270: Lifecycle hooks and event handlers are correctly implemented.State is properly cleared on activation/deactivation. The no-op handlers are documented and signatures match the protocol.
282-315: Validation correctly logs before raising.All error paths log at WARNING level with context before raising
ValueErrororTypeError, complying with the coding guidelines.
319-352: Thorough threshold validation with proper logging.The method validates list type, count limit, element types/ranges, and uniqueness. All error paths log before raising.
394-427: Threshold crossing logic is well-implemented.The method correctly finds the lowest unfired threshold, marks it as fired, and returns
True. The one-at-a-time firing prevents cascades as documented. Logging at INFO for threshold crossings is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/budget_driven.py (1)
357-404:⚠️ Potential issue | 🟠 MajorReapply the threshold range check in
_resolve_thresholds().This runtime path only filters
bool, non-numeric, and non-finite values. If persisted config bypassesvalidate_strategy_config(), thresholds like0or-1still reach_find_crossed_threshold()and will fire immediately becauseCeremonyEvalContext.budget_consumed_fractioncan legitimately be0.0when budget tracking is inactive. Likewise,>100thresholds silently become unreachable.🛡️ Proposed hardening
for t in raw: if isinstance(t, bool) or not isinstance(t, int | float): logger.warning( SPRINT_CEREMONY_SKIPPED, ceremony=ceremony_name, @@ if not math.isfinite(t): logger.warning( SPRINT_CEREMONY_SKIPPED, ceremony=ceremony_name, reason="non_finite_threshold", value=t, strategy="budget_driven", ) continue - valid.append(float(t)) + threshold = float(t) + if not (0 < threshold <= _MAX_THRESHOLD_PCT): + logger.warning( + SPRINT_CEREMONY_SKIPPED, + ceremony=ceremony_name, + reason="invalid_threshold_element", + value=t, + strategy="budget_driven", + ) + continue + valid.append(threshold)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/budget_driven.py` around lines 357 - 404, _resolve_thresholds currently only filters non-numeric and non-finite values; reintroduce a range check to reject thresholds that would immediately fire or be unreachable. In _resolve_thresholds (and referencing validate_strategy_config, _find_crossed_threshold, and CeremonyEvalContext.budget_consumed_fraction), skip and log any threshold t that is <= 0 or > 100 (e.g., reason="threshold_out_of_range", value=t) so only 0 < t <= 100 are added to valid; return None if no thresholds remain.
🤖 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/budget_driven.py`:
- Around line 303-305: The public validator validate_strategy_config() must
raise ValueError for invalid configs instead of letting TypeError from
_validate_thresholds() propagate; change the code that reads thresholds =
config.get(_KEY_BUDGET_THRESHOLDS) to explicitly validate the type (e.g., if
thresholds is not None and not isinstance(thresholds, list): raise
ValueError(...)) or catch TypeError from self._validate_thresholds(thresholds)
and re-raise it as ValueError with a clear message; apply the same pattern to
the other budget_thresholds validation site that calls _validate_thresholds().
- Around line 297-318: Replace free-form logger.warning calls with structured
workflow event logging using an event constant from
synthorg.observability.events: import the event enum/constant (e.g.,
events.WORKFLOW_CONFIG_INVALID) and replace logger.warning(msg,
strategy="budget_driven") with a structured call like
logger.info(events.WORKFLOW_CONFIG_INVALID, strategy="budget_driven",
detail=msg) or more granular fields (e.g., unknown_keys=sorted(unknown) for the
unknown-key branch and key=_KEY_TRANSITION_THRESHOLD, value=transition for the
transition validation). Do this for the unknown-config branch and the transition
validation (and the analogous warnings in the 323-355 range), keeping the
ValueError raises but moving human-readable data into the structured event
fields; ensure you add the synthorg.observability.events import and use the
exact event constant from that module.
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/budget_driven.py`:
- Around line 357-404: _resolve_thresholds currently only filters non-numeric
and non-finite values; reintroduce a range check to reject thresholds that would
immediately fire or be unreachable. In _resolve_thresholds (and referencing
validate_strategy_config, _find_crossed_threshold, and
CeremonyEvalContext.budget_consumed_fraction), skip and log any threshold t that
is <= 0 or > 100 (e.g., reason="threshold_out_of_range", value=t) so only 0 < t
<= 100 are added to valid; return None if no thresholds remain.
🪄 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: 79d153c0-ed13-4e99-b452-b2bb234054b7
📒 Files selected for processing (1)
src/synthorg/engine/workflow/strategies/budget_driven.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: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) per PEP 758 except syntax — ruff enforces this on Python 3.14
Type hints required on all public functions, enforced by mypy strict mode
Docstrings must use Google style and are required on public classes and functions, enforced by ruff D rules
Never mutate existing objects — create new objects; for non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models usingmodel_copy(update=...)for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) for structured concurrency instead of barecreate_task
Line length must be 88 characters, enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Logging variable name must always belogger(not_logger, notlog)
Always use structured logging withlogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WAR...
Files:
src/synthorg/engine/workflow/strategies/budget_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code, except inobservability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.py
Always use event name constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Files:
src/synthorg/engine/workflow/strategies/budget_driven.py
src/synthorg/{engine,providers}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/budget_driven.py
🧠 Learnings (8)
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.py
📚 Learning: 2026-04-02T09:13:44.997Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T09:13:44.997Z
Learning: Applies to **/*.py : 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
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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 Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.py
🔇 Additional comments (1)
src/synthorg/engine/workflow/strategies/budget_driven.py (1)
87-131: > Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/event_driven.py`:
- Around line 342-409: The validate_strategy_config method is too long; extract
the per-key validation blocks into small private helpers to keep the function
under 50 lines. Create helpers like _validate_on_event(config),
_validate_transition_event(config), and _validate_debounce_keys(config) (or
similar names) and move the logic that checks _KEY_ON_EVENT,
_KEY_TRANSITION_EVENT, and the loop over (_KEY_DEBOUNCE, _KEY_DEBOUNCE_DEFAULT)
into them; keep the unknown-keys check in validate_strategy_config and call
these new helpers in sequence, preserving the same logger warnings, ValueError
messages, and use of the existing constants (_KEY_ON_EVENT,
_KEY_TRANSITION_EVENT, _KEY_DEBOUNCE, _KEY_DEBOUNCE_DEFAULT, _MAX_DEBOUNCE,
SPRINT_STRATEGY_CONFIG_INVALID).
🪄 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: 9b4f7c88-d9b7-4444-9206-0e251a67abf1
📒 Files selected for processing (4)
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/test_budget_driven.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) per PEP 758 except syntax — ruff enforces this on Python 3.14
Type hints required on all public functions, enforced by mypy strict mode
Docstrings must use Google style and are required on public classes and functions, enforced by ruff D rules
Never mutate existing objects — create new objects; for non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models usingmodel_copy(update=...)for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) for structured concurrency instead of barecreate_task
Line length must be 88 characters, enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Logging variable name must always belogger(not_logger, notlog)
Always use structured logging withlogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WAR...
Files:
src/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pytests/unit/engine/workflow/strategies/test_budget_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code, except inobservability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.py
Always use event name constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Files:
src/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/{engine,providers}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto classify tests
Useasyncio_mode = "auto"for async tests — no manual@pytest.mark.asyncioneeded
Global timeout is 30 seconds per test inpyproject.toml— do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing in Python with@given+@settingsdecorators
Use Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()instead of widening timing margins; for tasks that must block indefinitely, useasyncio.Event().wait()
No-redundant-timeout pre-commit hook enforced
Files:
tests/unit/engine/workflow/strategies/test_budget_driven.py
🧠 Learnings (28)
📚 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-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-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-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-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-04-02T09:13:44.997Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T09:13:44.997Z
Learning: Applies to src/synthorg/**/*.py : Always use event name 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> 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-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-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.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 : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
src/synthorg/observability/events/workflow.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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.py
📚 Learning: 2026-04-02T09:13:44.997Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T09:13:44.997Z
Learning: Applies to **/*.py : 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
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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 Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.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/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.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/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.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/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.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 : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.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/budget_driven.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 : Always use structured logging: `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.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 error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.py
🔇 Additional comments (13)
src/synthorg/observability/events/workflow.py (1)
75-77: LGTM!The new workflow event constants are well-organized, follow the existing naming convention (
workflow.sprint.*andworkflow.velocity.*), and include clear docstrings. The sectioning with comment headers improves readability.Also applies to: 125-154
tests/unit/engine/workflow/strategies/test_budget_driven.py (1)
1-448: LGTM!Comprehensive test coverage for
BudgetDrivenStrategy:
- Protocol conformance verification
- Threshold firing behavior including edge cases (multiple thresholds, jumps, refire prevention)
- Independent tracking per ceremony
- Lifecycle reset behavior
- Validation edge cases (types, ranges, duplicates, bools)
The test organization into classes by functionality improves maintainability.
src/synthorg/engine/workflow/strategies/budget_driven.py (6)
1-84: LGTM!The
BudgetDrivenStrategyimplementation is well-structured:
- Clear module docstring explaining config keys
- Proper use of
__slots__for internal state- Correct logger setup via
get_logger(__name__)- Event constants imported from the domain-specific module
88-125: LGTM!The
should_fire_ceremonymethod correctly:
- Retrieves ceremony config via helper
- Defensively resolves/filters thresholds at runtime
- Converts fraction to percentage for comparison
- Delegates threshold tracking to
_find_crossed_threshold
127-185: LGTM!The
should_transition_sprintmethod correctly:
- Guards against non-ACTIVE sprint status
- Defensively validates
transition_thresholdwith type and range checks- Logs at INFO level for state transitions (compliant with guidelines)
- Falls back to default threshold when config is invalid
189-273: LGTM!Lifecycle hooks correctly:
- Clear internal state on sprint activation/deactivation
- Implement no-ops for task/budget/external events (budget-driven strategy reads from context, not events)
- Follow the async signature required by the protocol
286-331: LGTM!The
validate_strategy_configmethod:
- Properly validates unknown keys
- Catches
TypeErrorfrom_validate_thresholdsand re-raises asValueError(maintaining the documented contract)- Logs at WARNING before each raise (compliant with guidelines)
440-473: LGTM!The
_find_crossed_thresholdmethod correctly:
- Iterates sorted thresholds to fire the lowest unfired first
- Tracks fired thresholds per ceremony via
setdefault- Logs at INFO for threshold crossings (state transition)
- Logs at DEBUG for already-fired and skipped cases
src/synthorg/engine/workflow/strategies/event_driven.py (5)
99-152: LGTM!The
should_fire_ceremonymethod correctly:
- Retrieves ceremony config and validates
on_eventpresence- Resolves debounce via extracted helper (addressing past review feedback)
- Compares event delta against debounce threshold
- Updates firing baseline on successful fire
- Logs at appropriate levels (INFO for triggered, DEBUG for debounce not met)
154-213: LGTM!The
should_transition_sprintmethod correctly:
- Guards against non-ACTIVE sprint status
- Checks for transition event in both
context.external_eventsand internal counters- Logs at INFO level for state transitions with source attribution
217-258: LGTM!The lifecycle hooks correctly:
- Clear internal state (
_event_counts,_ceremony_last_fire_at) on activation/deactivation- Validate and load
debounce_defaultfrom config with proper fallback- Log at WARNING when falling back to default (addressing past review feedback)
260-329: LGTM!The event handlers correctly:
- Implement async signatures matching the protocol
- Increment appropriate counters via
_incrementhelper- Handle external events by incrementing the provided event name
433-460: LGTM!The
_incrementmethod has good defensive guards:
- Rejects event names exceeding
_MAX_EVENT_NAME_LEN(128)- Limits distinct events to
_MAX_DISTINCT_EVENTS(64) to prevent unbounded memory growth- Logs at WARNING for rejected events
- Logs at DEBUG for successful increments
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workflow/strategies/event_driven.py`:
- Around line 416-434: In _resolve_debounce, add an upper-bound validation
against _MAX_DEBOUNCE: after confirming raw is an int and >=1, also check if raw
> _MAX_DEBOUNCE and, if so, emit the same SPRINT_CEREMONY_SKIPPED logger.warning
(include ceremony=ceremony_name, reason="invalid_debounce", value=raw,
fallback=self._debounce_default, strategy="event_driven") and return
self._debounce_default; otherwise cast raw to int (result) and return it,
preserving the existing bool/type checks and use of _debounce_default.
- Around line 237-253: The activation path for debounce_default only checks it
is an int >= 1; update the on_sprint_activated activation logic that sets
self._debounce_default (the block handling debounce_default) to also enforce an
upper bound by checking debounce_default <= _MAX_DEBOUNCE (the same constant
used in validate_strategy_config), and if out of range log the
SPRINT_CEREMONY_SKIPPED warning with reason="invalid_debounce_default" and
fallback=_DEFAULT_DEBOUNCE, then set self._debounce_default = _DEFAULT_DEBOUNCE;
this mirrors validate_strategy_config and defends against oversized values.
🪄 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: 9040218b-0ef6-464d-9631-2bb7f82db101
📒 Files selected for processing (1)
src/synthorg/engine/workflow/strategies/event_driven.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 Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) per PEP 758 except syntax — ruff enforces this on Python 3.14
Type hints required on all public functions, enforced by mypy strict mode
Docstrings must use Google style and are required on public classes and functions, enforced by ruff D rules
Never mutate existing objects — create new objects; for non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; separate mutable-via-copy models usingmodel_copy(update=...)for runtime state that evolves; never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) for structured concurrency instead of barecreate_task
Line length must be 88 characters, enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Logging variable name must always belogger(not_logger, notlog)
Always use structured logging withlogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WAR...
Files:
src/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code, except inobservability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.py
Always use event name constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Files:
src/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/{engine,providers}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/event_driven.py
🧠 Learnings (2)
📚 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/event_driven.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/event_driven.py
🔇 Additional comments (7)
src/synthorg/engine/workflow/strategies/event_driven.py (7)
1-76: Well-structured module setup with proper observability integration.Imports follow the project conventions, logger is correctly initialized via
get_logger(__name__), and event constants are imported from the domain-specific module. Constants are clearly named with appropriate bounds.
78-95: Good use of__slots__and clear state initialization.The mutable internal state for event tracking is appropriate for a strategy class that maintains per-sprint counters. State is properly reset during lifecycle transitions.
99-152: Debounce tracking logic is correct.The firing baseline advances after each trigger (
self._ceremony_last_fire_at[ceremony.name] = global_count), ensuring the next debounce window starts from the current count. Logging appropriately distinguishes between trigger (INFO) and skip (DEBUG) events.
154-213: Sprint transition logic correctly handles both external and internal events.The method properly guards against non-ACTIVE sprints, checks external events first, then falls back to internal counters. State transitions are logged at INFO level as required.
331-340: Metadata methods correctly expose strategy type and default calculator.
342-414: Validation methods are well-structured with proper error logging.The extraction into
_validate_string_keyand_validate_debounce_keyskeeps the mainvalidate_strategy_configmethod concise. All validation failures log at WARNING with context before raisingValueError.
436-463: Good defensive bounds checking in_increment.The method correctly protects against excessively long event names and too many distinct events, preventing potential memory exhaustion from malicious or malformed input. Logging provides visibility into rejected events.
de2178f to
de43ac6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workflow/strategies/budget_driven.py`:
- Around line 409-438: The _resolve_thresholds routine currently accepts
arbitrarily long budget_thresholds lists and lets oversized lists reach the
hot-path in _find_crossed_threshold; enforce the existing _MAX_THRESHOLD_COUNT
here by rejecting lists longer than _MAX_THRESHOLD_COUNT: if len(raw) >
_MAX_THRESHOLD_COUNT, log SPRINT_CEREMONY_SKIPPED via logger.warning with
ceremony=ceremony_name, reason="threshold_count_exceeded", value=len(raw),
strategy="budget_driven" and return None (same skip behavior as other invalid
cases) before iterating; keep the rest of the type/finite/range checks intact so
oversized inputs are treated as invalid at the boundary.
🪄 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: 2ccdf554-4aa8-441e-bbad-484258d14603
📒 Files selected for processing (14)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.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: Deploy Preview
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
🧠 Learnings (29)
📚 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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/strategies/test_budget_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/engine/coordination/**/*.py : Task coordination uses multi-agent pipeline with 4 dispatchers (SAS/centralized/decentralized/context-dependent), wave execution, and workspace lifecycle integration.
Applied to files:
docs/design/ceremony-scheduling.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/hr/**/*.py : HR engine must provide: hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, trend detection), promotion/demotion
Applied to files:
docs/design/ceremony-scheduling.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/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:
docs/design/ceremony-scheduling.md
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
tests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/workflow/strategies/test_event_driven.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-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-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-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-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-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-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 : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/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/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.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/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.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}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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 Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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 : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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 : Always use structured logging: `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.py
🔇 Additional comments (11)
tests/unit/engine/workflow/strategies/conftest.py (1)
62-64: Good fixture extension for new strategy inputs.
make_context()now cleanly supports budget and external-event scenarios and forwards them directly intoCeremonyEvalContext, which keeps strategy tests realistic and explicit.Also applies to: 86-89
src/synthorg/engine/workflow/strategies/__init__.py (1)
8-10: Exports are correctly extended for new strategies.The module-level API now includes both new strategy implementations in imports and
__all__, which keeps discovery/usage consistent.Also applies to: 14-16, 25-27
src/synthorg/engine/workflow/strategies/task_driven.py (1)
19-19: Nice deduplication via shared config helper.Switching to
get_ceremony_config()centralizes strategy-config extraction without changing trigger evaluation behavior.Also applies to: 102-102
src/synthorg/engine/workflow/velocity_calculators/__init__.py (1)
7-9: Public calculator surface is updated correctly.Both new calculators are imported and re-exported consistently, matching the expanded velocity strategy support.
Also applies to: 16-18, 24-27
docs/design/ceremony-scheduling.md (2)
660-665: Observability table update is clear and complete for new strategy events.The added event constants are documented with useful descriptions and improve traceability for the new behavior paths.
696-705: Good addition of the integration caveat.Calling out pending
CeremonySchedulerhook wiring prevents overstatement and sets accurate expectations for current runtime behavior.src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py (1)
22-94: Implementation is solid and protocol-aligned.The calculator behavior is consistent (
pts/sprintprimary metric), and edge handling for empty/zero-window rolling averages is appropriately defensive.tests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.py (1)
28-129: Great targeted coverage for the new calculator.The suite validates protocol compliance, primary/secondary metrics, and rolling-average edge cases (
empty,window=0, and subset windows).src/synthorg/observability/events/workflow.py (1)
75-76: Event constant additions are consistent and well-scoped.The new workflow-domain constants follow the existing naming scheme and support structured logging for event-driven, budget-driven, and budget-velocity paths.
Also applies to: 126-154
src/synthorg/engine/workflow/strategies/event_driven.py (1)
38-45: No action needed. The code correctly uses Python 3.14's native PEP 649 lazy annotations. Withrequires-python = ">=3.14", the TYPE_CHECKING pattern for these imports is the standard approach and does not requirefrom __future__ import annotations. The project's linting configuration explicitly warns against addingfrom __future__ import annotationsfor Python 3.14+.> Likely an incorrect or invalid review comment.src/synthorg/engine/workflow/velocity_calculators/budget.py (1)
18-21: This annotation pattern is compatible with the project's Python 3.14+ requirement.The project explicitly requires
Python >=3.14(perpyproject.toml). In Python 3.14, PEP 649 makes all annotations deferred by default, soVelocityRecordandSequenceinsideTYPE_CHECKINGwill not causeNameErrorwhen the module is imported. Thefrom __future__ import annotationsworkaround is unnecessary for this codebase.> Likely an incorrect or invalid review comment.
…r transition_threshold
…apping, range check in _resolve_thresholds - Add SPRINT_STRATEGY_CONFIG_INVALID event constant - Replace bare-string logger.warning with structured event logging in validate_strategy_config (both strategies) and _validate_thresholds - Catch TypeError from _validate_thresholds and re-raise as ValueError so validate_strategy_config only raises ValueError per its contract - Add range check (0 < t <= 100) in _resolve_thresholds to skip out-of-range thresholds at read time (covers NaN/Inf too)
…om validate_strategy_config
…, _coerce_threshold helpers - should_transition_sprint: 62 -> 38 lines (extract config resolution) - _validate_thresholds: 56 -> 43 lines (use shared element check, add math.isfinite) - _resolve_thresholds: 57 -> 37 lines (use shared coerce helper, add dedup)
….isfinite to transition checks
…nology - Add type + length check for on_event in should_fire_ceremony - Add length check in _validate_string_key for consistency - Add str validation in on_external_event before _increment - Change 'Shipped' to 'Implemented (integration pending)' in docs - Expand scheduler integration note with all missing hooks - Parametrize validation tests in both test files per CLAUDE.md
…s to correct classes - Add length check to on_external_event, remove redundant one from _increment - Move test_no_policy_override_returns_false to TestShouldFireCeremony - Move test_transition_with_strategy_config_none to TestShouldTransitionSprint
0b4e756 to
37e2f2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py (1)
7-19:⚠️ Potential issue | 🟡 MinorAdd required module logger initialization.
This business-logic module is missing the mandated logger setup per coding guidelines.
Proposed fix
from typing import TYPE_CHECKING +from synthorg.observability import get_logger from synthorg.engine.workflow.velocity_types import ( VelocityCalcType, VelocityMetrics, ) if TYPE_CHECKING: from collections.abc import Sequence from synthorg.engine.workflow.sprint_velocity import VelocityRecord _UNIT: str = "pts/sprint" + +logger = get_logger(__name__)As per coding guidelines:
src/synthorg/**/*.py: "Every module with business logic MUST havefrom synthorg.observability import get_loggerfollowed bylogger = get_logger(__name__)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py` around lines 7 - 19, This module is missing the mandated logger setup; add the required import and initialize a module-level logger by importing get_logger from synthorg.observability and creating logger = get_logger(__name__) near the top of the file (e.g., alongside the existing imports and before module constants like _UNIT) so functions/classes in this module (e.g., any in points_per_sprint.py referencing _UNIT or VelocityCalcType/VelocityMetrics) can use the logger.src/synthorg/engine/workflow/strategies/budget_driven.py (1)
110-113:⚠️ Potential issue | 🟠 MajorVerify that callers drain crossed thresholds before moving on.
This implementation intentionally returns after marking the first crossed threshold. If
CeremonySchedulerstill callsshould_fire_ceremony()only once per ceremony per evaluation cycle, a 20%→80% jump will fire 25% and leave 50%/75% pending until another budget update arrives. That does not match the repeated-call behavior documented here or exercised by the unit tests.Use this read-only check to confirm the current caller contract:
#!/bin/bash set -euo pipefail scheduler_file="$(fd -i 'ceremony_scheduler\.py$' src tests | head -n 1)" if [[ -z "${scheduler_file}" ]]; then echo "ceremony_scheduler.py not found" exit 1 fi echo "== should_fire_ceremony call sites ==" rg -nC6 --type=py '\bshould_fire_ceremony\s*\(' "${scheduler_file}" echo echo "== surrounding evaluation control flow ==" rg -nC10 --type=py '_evaluate_ceremonies|should_fire_ceremony|while|for' "${scheduler_file}"Expected result: either the scheduler loops until
should_fire_ceremony()returnsFalse, or the strategy/tests need to be adjusted to match the actual single-call behavior.Also applies to: 130-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/budget_driven.py` around lines 110 - 113, The strategy's should_fire_ceremony currently marks and returns after the first crossed threshold, which relies on CeremonyScheduler repeatedly calling it; instead either make the caller loop until should_fire_ceremony() is False or (preferable) change the budget-driven implementation in should_fire_ceremony (and the helper that marks thresholds, e.g., _mark_threshold_fired) to drain all currently crossed/unfired thresholds in one call by iterating over the thresholds list and marking each crossed threshold before returning so no pending thresholds are left behind; verify behavior against CeremonyScheduler usage to ensure tests remain correct.
🤖 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/event_driven.py`:
- Around line 118-123: The current validation for event names (in the on_event
checks inside event_driven.py) accepts whitespace-only strings because it only
tests truthiness; update the guards (e.g., the on_event validation block and the
similar check around lines ~390-407) to reject strings that are empty after
trimming by using on_event.strip() (or delegating to _validate_string_key()
updated to use .strip()) so that whitespace-only names are treated as invalid,
keeping behavior consistent with on_external_event().
- Around line 183-191: Validate that transition_event is a non-empty string
before passing it to _check_transition_event to avoid TypeError from
self._event_counts.get(transition_event, 0); if transition_event is not an
instance of str or is an empty string, return None. Apply the same guard used in
should_fire_ceremony for on_event: check
strategy_config.get(_KEY_TRANSITION_EVENT) (and the similar retrieval in the
199-208 block) for isinstance(..., str) and len(...) > 0 before calling
_check_transition_event or accessing event counters.
In `@src/synthorg/engine/workflow/velocity_calculators/budget.py`:
- Around line 50-67: compute() currently only guards against None and exactly
0.0 for record.budget_consumed so negative budgets produce a negative pts/EUR
while rolling_average() excludes budget_consumed <= 0.0; update compute() to
treat any non-positive budget the same as None (i.e., if budget is None or
budget <= 0.0) and log reason accordingly (e.g., "none" vs "non-positive")
before returning the zeroed VelocityMetrics; reference record.budget_consumed,
record.story_points_completed, compute(), rolling_average(), and VelocityMetrics
when making the change.
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/budget_driven.py`:
- Around line 110-113: The strategy's should_fire_ceremony currently marks and
returns after the first crossed threshold, which relies on CeremonyScheduler
repeatedly calling it; instead either make the caller loop until
should_fire_ceremony() is False or (preferable) change the budget-driven
implementation in should_fire_ceremony (and the helper that marks thresholds,
e.g., _mark_threshold_fired) to drain all currently crossed/unfired thresholds
in one call by iterating over the thresholds list and marking each crossed
threshold before returning so no pending thresholds are left behind; verify
behavior against CeremonyScheduler usage to ensure tests remain correct.
In `@src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py`:
- Around line 7-19: This module is missing the mandated logger setup; add the
required import and initialize a module-level logger by importing get_logger
from synthorg.observability and creating logger = get_logger(__name__) near the
top of the file (e.g., alongside the existing imports and before module
constants like _UNIT) so functions/classes in this module (e.g., any in
points_per_sprint.py referencing _UNIT or VelocityCalcType/VelocityMetrics) can
use the logger.
🪄 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: f342ee7c-b763-4ab1-a460-e68f1754857d
📒 Files selected for processing (14)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/budget_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.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: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
Useexcept A, B:syntax (no parentheses) for exception handling -- PEP 758 except syntax, enforced by ruff on Python 3.14
All public functions must have type hints; enforce with mypy strict mode
Use Google-style docstrings, required on public classes and functions; enforced by ruff D rules
Create new objects for immutability; never mutate existing objects. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic models for config and identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); adoptallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier and name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Maintain line length of 88 characters; enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, conf...
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pytests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerfollowed bylogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr)
Always use variable namelogger(not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly from domain modules
Use structured kwargs in logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG logging for object creation, internal flow, and entry/exit of key functions
All provider calls go throughBaseCompletionProviderwhich applies retry and rate limiting automatically; never implement retry logic in driver subclasses or calling code
SetRetryConfigandRateLimiterConfigper-provider inProviderConfig
Retryable errors (is_retryable=True):RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError
Non-retryable errors raise immediately without retry
Rate limiter respectsRateLimitError.retry_afterfrom providers and automatically pauses future requests
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names likeexample-provider,example-large-001,example-medium-001,large/medium/smallaliases; vendor names allowed only in: (1) Operations design page (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths, (4) provid...
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
src/synthorg/engine/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Catch
RetryExhaustedErrorat the engine layer to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/points_per_sprint.pysrc/synthorg/engine/workflow/strategies/task_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Maintain 80% code coverage minimum; enforced in CI
Useasyncio_mode = "auto"in pytest configuration; no manual@pytest.mark.asyncioneeded
Global test timeout: 30 seconds per test (configured inpyproject.toml); non-default overrides liketimeout(60)are allowed but do not add per-filepytest.mark.timeout(30)markers
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing in Python with@givenand@settingsdecorators; profiles:ci(50 examples, default) anddev(1000 examples) controlled viaHYPOTHESIS_PROFILEenv var
NEVER skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()for determinism; for tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/velocity_calculators/test_budget.pytests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.py
🧠 Learnings (42)
📚 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/engine/workflow/strategies/__init__.pysrc/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/**/*.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/strategies/__init__.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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pytests/unit/engine/workflow/velocity_calculators/test_budget.pysrc/synthorg/engine/workflow/velocity_calculators/budget.pytests/unit/engine/workflow/strategies/test_budget_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.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 : Every module with business logic MUST have `from synthorg.observability import get_logger` followed by `logger = get_logger(__name__)`.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have `from synthorg.observability import get_logger` followed by `logger = get_logger(__name__)`
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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 : Every module with business logic must import logger via `from synthorg.observability import get_logger` and initialize with `logger = get_logger(__name__)`
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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 : Every module with business logic must have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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 : Every module with business logic must import `from synthorg.observability import get_logger` and define `logger = get_logger(__name__)`
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use import logging / logging.getLogger() / print() in application code.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.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-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-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-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/budget_driven.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 tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.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 : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/engine/workflow/strategies/test_budget_driven.pytests/unit/engine/workflow/strategies/test_event_driven.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/workflow/strategies/test_event_driven.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 tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; mock `time.monotonic()` and `asyncio.sleep()` for determinism; use `asyncio.Event().wait()` for indefinite blocking instead of `asyncio.sleep(large_number)`
Applied to files:
tests/unit/engine/workflow/strategies/test_event_driven.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 tests/**/*.py : 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 instead of widening timing margins
Applied to files:
tests/unit/engine/workflow/strategies/test_event_driven.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` for determinism; for tasks that must block indefinitely until cancelled, use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)`
Applied to files:
tests/unit/engine/workflow/strategies/test_event_driven.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 tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/engine/workflow/strategies/test_event_driven.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_event_driven.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/engine/coordination/**/*.py : Task coordination uses multi-agent pipeline with 4 dispatchers (SAS/centralized/decentralized/context-dependent), wave execution, and workspace lifecycle integration.
Applied to files:
docs/design/ceremony-scheduling.md
📚 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/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/strategies/budget_driven.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}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`); adopt `allow_inf_nan=False` in all `ConfigDict` declarations to reject `NaN`/`Inf` in numeric fields at validation time
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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 Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.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/budget_driven.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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 : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/workflow/strategies/budget_driven.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/budget_driven.py
🔇 Additional comments (9)
src/synthorg/engine/workflow/strategies/task_driven.py (1)
19-19: LGTM! Good refactor to shared helper.Consolidating
_get_ceremony_configinto a sharedget_ceremony_confighelper alignsTaskDrivenStrategywith the new strategies and follows DRY principles.Also applies to: 102-102
src/synthorg/engine/workflow/strategies/__init__.py (1)
8-10: LGTM!Clean package re-exports for the new strategy implementations with proper alphabetical ordering in
__all__.Also applies to: 14-16, 24-30
src/synthorg/engine/workflow/velocity_calculators/__init__.py (1)
7-9: LGTM!Clean package re-exports for the new velocity calculator implementations.
Also applies to: 16-18, 23-29
tests/unit/engine/workflow/strategies/conftest.py (1)
54-93: LGTM!Clean backward-compatible extension of
make_context()to support budget and external event testing scenarios. Default values ensure existing tests continue to work.docs/design/ceremony-scheduling.md (2)
696-705: LGTM! Documentation accurately reflects integration status.The explicit note about pending
CeremonySchedulerhook wiring (Lines 704-705) properly addresses the concern thatEventDrivenStrategy's event counters won't increment until scheduler integration is complete. This transparency helps users understand the current state.
660-666: LGTM!Observability event constants are well-documented and align with the implementations in the strategy modules.
src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py (1)
35-84: LGTM! Clean velocity calculator implementation.The
compute()androlling_average()methods correctly handle edge cases (empty records,window < 1) and provide appropriate secondary metrics. The implementation follows the established pattern from other velocity calculators.tests/unit/engine/workflow/velocity_calculators/test_points_per_sprint.py (1)
1-129: LGTM! Comprehensive test coverage.Good coverage of protocol conformance, metadata properties,
compute()behavior, androlling_average()edge cases (empty records, zero window, window smaller than records). Tests properly usepytest.approxfor float comparisons.tests/unit/engine/workflow/velocity_calculators/test_budget.py (1)
1-225: LGTM! Thorough test coverage for budget-specific edge cases.Excellent coverage including:
- Protocol conformance and metadata
compute()with valid budget, zero budget, andNonebudgetrolling_average()skipping records withNoneor zerobudget_consumed- Verification that
sprints_averagedcounts only valid recordsThe tests properly validate the graceful degradation behavior documented in the PR objectives.
… type, guard negative budget - Use .strip() for on_event, transition_event, and _validate_string_key - Add isinstance+strip check for transition_event before _check_transition_event - Change budget.py compute() guard from == 0.0 to <= 0.0 (match rolling_average)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/event_driven.py (1)
158-191: 🧹 Nitpick | 🔵 TrivialConsider adding length check for consistency with
_validate_string_key.The
transition_eventvalidation at line 185 checksisinstanceandstrip()but omits the length check (len(transition_event) > _MAX_EVENT_NAME_LEN) that_validate_string_keyenforces. Whilevalidate_strategy_configshould catch this at config load time, adding the length check here would provide consistent defense-in-depth.This is a minor inconsistency—the code won't crash with a long event name, it just won't match any events.
♻️ Optional enhancement for consistency
strategy_config = config.ceremony_policy.strategy_config or {} transition_event = strategy_config.get(_KEY_TRANSITION_EVENT) - if not isinstance(transition_event, str) or not transition_event.strip(): + if ( + not isinstance(transition_event, str) + or not transition_event.strip() + or len(transition_event) > _MAX_EVENT_NAME_LEN + ): return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/event_driven.py` around lines 158 - 191, The transition_event check in should_transition_sprint currently only verifies type and strip() but omits the max-length guard used elsewhere; update should_transition_sprint to also reject values where len(transition_event) > _MAX_EVENT_NAME_LEN (same constant used by _validate_string_key) before calling _check_transition_event for consistent defense-in-depth around _KEY_TRANSITION_EVENT handling.
🤖 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/event_driven.py`:
- Around line 116-132: The validation uses on_event.strip() but later uses the
original unstripped on_event for lookups, causing mismatches; normalize the
event name by trimming whitespace once after reading config (e.g., replace
on_event = config.get(_KEY_ON_EVENT) with a stripped variable like
normalized_on_event = on_event.strip()) and then use normalized_on_event for the
_event_counts lookup, debounce resolution (self._resolve_debounce), and any
increment/compare logic; apply the same normalization in on_external_event and
should_transition_sprint so all comparisons against _event_counts and EVENT_*
constants use the trimmed event name.
In `@src/synthorg/engine/workflow/velocity_calculators/budget.py`:
- Around line 57-63: There are three duplicated constructions of
VelocityMetrics(primary_value=0.0, primary_unit=_UNIT, secondary=...) — extract
a small helper (e.g., a module-level function named _zero_metrics or
make_zero_velocity) that accepts the secondary dict (or optional kwargs) and
returns VelocityMetrics with primary_value 0.0 and primary_unit _UNIT; then
replace the three inline constructions (the ones currently returning
VelocityMetrics with primary_value=0.0 in budget.py at the spots referencing
record.story_points_completed and the other two occurrences) with calls to that
helper to remove duplication.
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/event_driven.py`:
- Around line 158-191: The transition_event check in should_transition_sprint
currently only verifies type and strip() but omits the max-length guard used
elsewhere; update should_transition_sprint to also reject values where
len(transition_event) > _MAX_EVENT_NAME_LEN (same constant used by
_validate_string_key) before calling _check_transition_event for consistent
defense-in-depth around _KEY_TRANSITION_EVENT handling.
🪄 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: b3fc1ee3-d8be-419e-9bd8-d80e8a7a4ee5
📒 Files selected for processing (2)
src/synthorg/engine/workflow/strategies/event_driven.pysrc/synthorg/engine/workflow/velocity_calculators/budget.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: Build Backend
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
Useexcept A, B:syntax (no parentheses) for exception handling -- PEP 758 except syntax, enforced by ruff on Python 3.14
All public functions must have type hints; enforce with mypy strict mode
Use Google-style docstrings, required on public classes and functions; enforced by ruff D rules
Create new objects for immutability; never mutate existing objects. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic models for config and identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); adoptallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier and name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Maintain line length of 88 characters; enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, conf...
Files:
src/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerfollowed bylogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr)
Always use variable namelogger(not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly from domain modules
Use structured kwargs in logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG logging for object creation, internal flow, and entry/exit of key functions
All provider calls go throughBaseCompletionProviderwhich applies retry and rate limiting automatically; never implement retry logic in driver subclasses or calling code
SetRetryConfigandRateLimiterConfigper-provider inProviderConfig
Retryable errors (is_retryable=True):RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError
Non-retryable errors raise immediately without retry
Rate limiter respectsRateLimitError.retry_afterfrom providers and automatically pauses future requests
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names likeexample-provider,example-large-001,example-medium-001,large/medium/smallaliases; vendor names allowed only in: (1) Operations design page (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths, (4) provid...
Files:
src/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/engine/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Catch
RetryExhaustedErrorat the engine layer to trigger fallback chains
Files:
src/synthorg/engine/workflow/velocity_calculators/budget.pysrc/synthorg/engine/workflow/strategies/event_driven.py
🧠 Learnings (5)
📚 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 src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/budget.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/budget.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/event_driven.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.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/event_driven.py
🔇 Additional comments (12)
src/synthorg/engine/workflow/velocity_calculators/budget.py (3)
13-23: Observability setup is correctly wired.
get_logger+loggernaming and event-constant logging style align well with the module standards.
41-74: Budget guard and single-record velocity computation look correct.The
budget is None or budget <= 0.0guard prevents invalid division and keeps behavior consistent with the rolling path.
98-119: Rolling average inclusion logic is correct and clear.Filtering invalid budgets and reporting
sprints_averagedfromvalid_countcorrectly reflects contributing records.src/synthorg/engine/workflow/strategies/event_driven.py (9)
1-76: Well-structured module setup.The module docstring clearly documents the config keys for both ceremony-level and sprint-level configuration. Logger setup follows the required pattern, and observability event constants are properly imported from the domain-specific module.
78-96: LGTM!Good use of
__slots__for memory efficiency. The class docstring clearly explains the debounce firing mechanism and state lifecycle.
193-217: LGTM!Clean separation checking external events first, then falling back to internal counters. Both paths are properly logged with source attribution.
221-262: LGTM!Lifecycle hooks properly manage state. The
debounce_defaultvalidation now includes the full bounds check (1 <= debounce_default <= _MAX_DEBOUNCE) with appropriate fallback logging when invalid values are detected.
264-318: LGTM!Task event handlers correctly delegate to
_incrementwith well-defined constant event names. Each handler has proper type hints and docstrings.
353-360: LGTM!Metadata methods correctly return
EVENT_DRIVENstrategy type andPOINTS_PER_SPRINTas the default velocity calculator, matching the PR objectives.
362-386: LGTM!Validation is well-factored into helper methods, keeping this method under the 50-line limit. Unknown key detection with sorted output aids debugging.
442-464: LGTM!The debounce resolution now includes the full bounds check (
1 <= raw <= _MAX_DEBOUNCE), addressing the previous review concern about ceremony-level overrides bypassing limits.Minor style note: Lines 463-464 could be simplified to
return rawsince the type is already validated, but this is purely cosmetic.
466-490: Good defensive limit on distinct events.The
_MAX_DISTINCT_EVENTScap prevents unbounded memory growth from external event names. The docstring clearly documents that callers must validateevent_namebefore calling, with internal handlers using known constants.
| return VelocityMetrics( | ||
| primary_value=0.0, | ||
| primary_unit=_UNIT, | ||
| secondary={ | ||
| "pts_per_sprint": record.story_points_completed, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting a helper for repeated zero-metrics returns.
There are three near-identical VelocityMetrics(primary_value=0.0, primary_unit=_UNIT, ...) constructions; a small helper would reduce duplication.
♻️ Optional refactor
class BudgetVelocityCalculator:
@@
+ def _zero_metrics(
+ self,
+ *,
+ pts_per_sprint: float | None = None,
+ ) -> VelocityMetrics:
+ secondary = (
+ {"pts_per_sprint": pts_per_sprint}
+ if pts_per_sprint is not None
+ else None
+ )
+ return VelocityMetrics(
+ primary_value=0.0,
+ primary_unit=_UNIT,
+ secondary=secondary,
+ )
@@
- return VelocityMetrics(
- primary_value=0.0,
- primary_unit=_UNIT,
- secondary={
- "pts_per_sprint": record.story_points_completed,
- },
- )
+ return self._zero_metrics(
+ pts_per_sprint=record.story_points_completed,
+ )
@@
- return VelocityMetrics(
- primary_value=0.0,
- primary_unit=_UNIT,
- )
+ return self._zero_metrics()
@@
- return VelocityMetrics(
- primary_value=0.0,
- primary_unit=_UNIT,
- )
+ return self._zero_metrics()Also applies to: 94-97, 109-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/velocity_calculators/budget.py` around lines 57
- 63, There are three duplicated constructions of
VelocityMetrics(primary_value=0.0, primary_unit=_UNIT, secondary=...) — extract
a small helper (e.g., a module-level function named _zero_metrics or
make_zero_velocity) that accepts the secondary dict (or optional kwargs) and
returns VelocityMetrics with primary_value 0.0 and primary_unit _UNIT; then
replace the three inline constructions (the ones currently returning
VelocityMetrics with primary_value=0.0 in budget.py at the spots referencing
record.story_points_completed and the other two occurrences) with calls to that
helper to remove duplication.
…eck on transition_event
There was a problem hiding this comment.
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/event_driven.py`:
- Around line 471-489: The _increment method currently forbids adding new event
keys once _event_counts reaches _MAX_DISTINCT_EVENTS which can prevent
configured ceremonies from ever starting; change _increment (and related
initialization) so that configured subscribed event names (those referenced by
ceremony configs: on_event and transition_event) are exempt from the
distinct-event cap. Concretely, introduce or reuse a set like
self._subscribed_event_names (populate it from ceremony configs during strategy
initialization by collecting on_event and transition_event values), then in
_increment check: if event_name not in self._event_counts and
len(self._event_counts) >= _MAX_DISTINCT_EVENTS and event_name not in
self._subscribed_event_names then log the warning and return; otherwise allow
adding/incrementing the event_name so should_fire_ceremony can debounce
subscribed ceremonies correctly (references: _increment, _event_counts,
_MAX_DISTINCT_EVENTS, should_fire_ceremony, on_event, transition_event).
- Around line 56-63: The current _KNOWN_CONFIG_KEYS set mixes ceremony-level
keys (_KEY_ON_EVENT, _KEY_DEBOUNCE) and sprint-level keys
(_KEY_TRANSITION_EVENT, _KEY_DEBOUNCE_DEFAULT), allowing mis-scoped config to
pass validation; change the validator to enforce scope-specific keys by
splitting _KNOWN_CONFIG_KEYS into two distinct frozensets (e.g.,
_KNOWN_CEREMONY_KEYS and _KNOWN_SPRINT_KEYS) and update the validation function
(or its callers) to accept an explicit scope parameter (or call the appropriate
scope-specific validator) so only the proper set is used; also apply the same
split/parameterized validation to the other occurrence referenced around the
367-391 area to ensure misplaced keys fail fast.
- Around line 264-267: on_sprint_deactivated currently clears _event_counts and
_ceremony_last_fire_at but leaves sprint-local debounce config in
_debounce_default causing stale thresholds to persist; update
on_sprint_deactivated to also reset _debounce_default (and any related debounce
state) to its initial/empty value so that evaluations after deactivation do not
reuse the prior sprint's thresholds, referencing the on_sprint_deactivated
method and the _debounce_default attribute when making the change.
🪄 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: dd597b65-0d60-4f14-be1f-deb6861b7ac9
📒 Files selected for processing (1)
src/synthorg/engine/workflow/strategies/event_driven.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 Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
Useexcept A, B:syntax (no parentheses) for exception handling -- PEP 758 except syntax, enforced by ruff on Python 3.14
All public functions must have type hints; enforce with mypy strict mode
Use Google-style docstrings, required on public classes and functions; enforced by ruff D rules
Create new objects for immutability; never mutate existing objects. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic models for config and identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); adoptallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier and name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Maintain line length of 88 characters; enforced by ruff
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, conf...
Files:
src/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerfollowed bylogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr)
Always use variable namelogger(not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly from domain modules
Use structured kwargs in logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG logging for object creation, internal flow, and entry/exit of key functions
All provider calls go throughBaseCompletionProviderwhich applies retry and rate limiting automatically; never implement retry logic in driver subclasses or calling code
SetRetryConfigandRateLimiterConfigper-provider inProviderConfig
Retryable errors (is_retryable=True):RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError
Non-retryable errors raise immediately without retry
Rate limiter respectsRateLimitError.retry_afterfrom providers and automatically pauses future requests
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names likeexample-provider,example-large-001,example-medium-001,large/medium/smallaliases; vendor names allowed only in: (1) Operations design page (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths, (4) provid...
Files:
src/synthorg/engine/workflow/strategies/event_driven.py
src/synthorg/engine/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Catch
RetryExhaustedErrorat the engine layer to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/event_driven.py
🧠 Learnings (3)
📚 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/event_driven.py
📚 Learning: 2026-04-02T13:12:42.085Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T13:12:42.085Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/engine/workflow/strategies/event_driven.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/event_driven.py
| _KNOWN_CONFIG_KEYS: frozenset[str] = frozenset( | ||
| { | ||
| _KEY_ON_EVENT, | ||
| _KEY_DEBOUNCE, | ||
| _KEY_DEBOUNCE_DEFAULT, | ||
| _KEY_TRANSITION_EVENT, | ||
| } | ||
| ) |
There was a problem hiding this comment.
Scope-specific keys need scope-specific validation.
This validator accepts the union of ceremony-level keys (on_event, debounce) and sprint-level keys (transition_event, debounce_default), but the runtime reads those keys from different config layers. A misplaced key can therefore look valid here and then be ignored silently later. Split validation by scope, or have the caller provide the expected scope so bad placement fails fast.
Based on learnings: "Validate at system boundaries (user input, external APIs, config files)."
Also applies to: 367-391
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/event_driven.py` around lines 56 -
63, The current _KNOWN_CONFIG_KEYS set mixes ceremony-level keys (_KEY_ON_EVENT,
_KEY_DEBOUNCE) and sprint-level keys (_KEY_TRANSITION_EVENT,
_KEY_DEBOUNCE_DEFAULT), allowing mis-scoped config to pass validation; change
the validator to enforce scope-specific keys by splitting _KNOWN_CONFIG_KEYS
into two distinct frozensets (e.g., _KNOWN_CEREMONY_KEYS and _KNOWN_SPRINT_KEYS)
and update the validation function (or its callers) to accept an explicit scope
parameter (or call the appropriate scope-specific validator) so only the proper
set is used; also apply the same split/parameterized validation to the other
occurrence referenced around the 367-391 area to ensure misplaced keys fail
fast.
| def _increment(self, event_name: str) -> None: | ||
| """Increment the global event counter for the given event. | ||
|
|
||
| Callers must validate ``event_name`` before calling. | ||
| Internal lifecycle hooks pass known constants; external | ||
| events are validated in ``on_external_event``. | ||
| """ | ||
| if ( | ||
| event_name not in self._event_counts | ||
| and len(self._event_counts) >= _MAX_DISTINCT_EVENTS | ||
| ): | ||
| logger.warning( | ||
| SPRINT_CEREMONY_SKIPPED, | ||
| reason="too_many_distinct_events", | ||
| event_name=event_name, | ||
| limit=_MAX_DISTINCT_EVENTS, | ||
| strategy="event_driven", | ||
| ) | ||
| return |
There was a problem hiding this comment.
Don't let the distinct-event cap disable subscribed ceremonies.
Once _event_counts already contains 64 names, _increment() drops every new event key. If a ceremony's configured on_event is first observed after that point, its counter never starts and should_fire_ceremony() can no longer satisfy debounce for the rest of the sprint. Reserve capacity for configured on_event / transition_event names, or exempt subscribed events from this cap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/event_driven.py` around lines 471 -
489, The _increment method currently forbids adding new event keys once
_event_counts reaches _MAX_DISTINCT_EVENTS which can prevent configured
ceremonies from ever starting; change _increment (and related initialization) so
that configured subscribed event names (those referenced by ceremony configs:
on_event and transition_event) are exempt from the distinct-event cap.
Concretely, introduce or reuse a set like self._subscribed_event_names (populate
it from ceremony configs during strategy initialization by collecting on_event
and transition_event values), then in _increment check: if event_name not in
self._event_counts and len(self._event_counts) >= _MAX_DISTINCT_EVENTS and
event_name not in self._subscribed_event_names then log the warning and return;
otherwise allow adding/incrementing the event_name so should_fire_ceremony can
debounce subscribed ceremonies correctly (references: _increment, _event_counts,
_MAX_DISTINCT_EVENTS, should_fire_ceremony, on_event, transition_event).
🤖 I have created a release *beep* *boop* --- ## [0.5.7](v0.5.6...v0.5.7) (2026-04-02) ### Features * comparison page -- SynthOrg vs agent orchestration frameworks ([#994](#994)) ([6f937ef](6f937ef)), closes [#981](#981) * event-driven and budget-driven ceremony scheduling strategies ([#995](#995)) ([f88e7b0](f88e7b0)), closes [#971](#971) [#972](#972) * template packs for post-setup additive team expansion ([#996](#996)) ([b45e14a](b45e14a)), closes [#727](#727) ### Performance * preload JetBrains Mono font, remove unused api.github.com preconnect ([#998](#998)) ([2a189c2](2a189c2)) * run only affected modules in pre-push hooks ([#992](#992)) ([7956e23](7956e23)) ### Maintenance * bump astro from 6.1.2 to 6.1.3 in /site in the all group ([#988](#988)) ([17b58db](17b58db)) * bump the all group across 1 directory with 2 updates ([#989](#989)) ([1ff462a](1ff462a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
Add two new ceremony scheduling strategies and their velocity calculators to the pluggable ceremony scheduling system (#961).
Event-Driven Strategy (#971)
EventDrivenStrategy: ceremonies subscribe to engine events (task_completed,task_blocked,task_added,budget_updated, arbitrary external events) with configurable debouncetransition_event(external events or internal counters)PointsPerSprintVelocityCalculator: simplest calculator -- rawpts/sprintwith no normalizationBudget-Driven Strategy (#972)
BudgetDrivenStrategy: ceremonies fire at cost-consumption percentage thresholds (e.g. 25%, 50%, 75%)BudgetVelocityCalculator:pts/EURmeasuring cost efficiency, gracefully skips records without budget dataChanges
src/synthorg/engine/workflow/strategies/event_driven.py(362 lines)src/synthorg/engine/workflow/strategies/budget_driven.py(349 lines)src/synthorg/engine/workflow/velocity_calculators/points_per_sprint.py(96 lines)src/synthorg/engine/workflow/velocity_calculators/budget.py(126 lines)__init__.py(exports)observability/events/workflow.py(6 new event constants)docs/design/ceremony-scheduling.md(shipped status, observability table, roadmap)Test Plan
Closes #971
Closes #972