feat: calendar + hybrid ceremony scheduling strategies#985
Conversation
WalkthroughAdds two ceremony scheduling strategies (CalendarStrategy, HybridStrategy) and two velocity calculators (CalendarVelocityCalculator — pts/day; MultiDimensionalVelocityCalculator — pts/sprint with secondary metrics). Exposes these implementations from the strategies, velocity_calculators, and workflow package initializers. Introduces shared strategy helpers for interval/duration/trigger resolution and validation, adds observability event constants for velocity edge cases, updates documentation to mark Calendar and Hybrid as shipped, and adds comprehensive unit tests. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 introduces the CalendarStrategy and HybridStrategy for ceremony scheduling, along with corresponding velocity calculators (CalendarVelocityCalculator and MultiDimensionalVelocityCalculator). The review identified a critical issue in HybridStrategy where task-driven triggers can cause infinite firing loops, a high-severity bug in MultiDimensionalVelocityCalculator where pts_per_day calculation can trigger a ZeroDivisionError, and a medium-severity inconsistency regarding the TRIGGER_SPRINT_START trigger, which is listed as valid but not implemented.
| fires = calendar_fires or task_fires | ||
| if fires: | ||
| # Reset calendar timer regardless of which leg fired. | ||
| self._last_fire_elapsed[ceremony.name] = context.elapsed_seconds |
There was a problem hiding this comment.
The HybridStrategy is susceptible to an infinite firing loop for state-based task triggers such as TRIGGER_SPRINT_END or TRIGGER_SPRINT_PERCENTAGE.
Unlike the calendar leg, which uses an interval-based check against _last_fire_elapsed, the task leg only checks if a threshold is met (e.g., pct >= threshold). Once this threshold is crossed, _check_task_driven will return True on every subsequent evaluation (every event), causing the ceremony to fire repeatedly.
To fix this, the strategy should ensure that a task-driven trigger only fires if it hasn't already fired for the current threshold crossing, perhaps by checking context.elapsed_seconds > self._last_fire_elapsed.get(ceremony.name, -1.0) or by tracking fired thresholds.
| as secondary metrics. | ||
| """ | ||
| pts_sprint = record.story_points_completed | ||
| pts_per_day = pts_sprint / record.duration_days |
There was a problem hiding this comment.
The calculation of pts_per_day will raise a ZeroDivisionError if record.duration_days is zero. While CalendarVelocityCalculator includes a check for this condition, it is missing here. A safe fallback (e.g., 0.0) should be used when the duration is zero.
| pts_per_day = pts_sprint / record.duration_days | |
| pts_per_day = pts_sprint / record.duration_days if record.duration_days > 0 else 0.0 |
| if trigger == TRIGGER_SPRINT_START: | ||
| return False |
There was a problem hiding this comment.
The TRIGGER_SPRINT_START trigger is explicitly disabled in the task-driven leg of the HybridStrategy by always returning False. Since this trigger is listed as valid for the strategy (line 70), it should be implemented to fire once at the beginning of the sprint, or removed from the valid triggers list if intentionally unsupported for this strategy.
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 `@docs/design/ceremony-scheduling.md`:
- Around line 449-450: The default-strategy mapping for
MultiDimensionalVelocityCalculator is inconsistent: change the default
assignment so MultiDimensionalVelocityCalculator is the default for "hybrid"
only (remove it as default for "event_driven" and "milestone_driven") and ensure
CalendarVelocityCalculator remains mapped appropriately; update the
strategy-default table entries and any corresponding row that lists
`MultiDimensionalVelocityCalculator` (and the related `pts/sprint` descriptor)
so the default column lists "hybrid" instead of "event_driven, milestone_driven"
and keep other strategy rows consistent.
In `@src/synthorg/engine/workflow/strategies/hybrid.py`:
- Around line 283-345: Extract the duplicated helpers _get_ceremony_config,
_resolve_interval, and _resolve_duration_days into a new shared utilities module
and have both HybridStrategy and CalendarStrategy import and call those
utilities instead of keeping the copies; specifically, move the implementations
for _get_ceremony_config(ceremony: SprintCeremonyConfig),
_resolve_interval(self, ceremony: SprintCeremonyConfig) and
_resolve_duration_days(config: SprintConfig) into the new module, export them
with clear names, update the HybridStrategy methods (and the counterparts in
calendar.py) to delegate to the shared functions, and remove the duplicate
implementations so there is a single source of truth for these helpers.
In `@tests/unit/engine/workflow/strategies/test_calendar.py`:
- Around line 131-184: Replace the repetitive interval tests with a single
parametrized test: create one pytest.mark.parametrize test over (frequency,
elapsed_seconds) tuples and call CalendarStrategy().should_fire_ceremony for
each case, using _make_ceremony(frequency=...),
_make_context(elapsed_seconds=...), and _make_sprint() to build inputs; target
the current tests test_fires_when_daily_interval_elapsed, test_weekly_interval,
and test_bi_weekly_interval to be consolidated into this parametrized test and
keep the existing assertions that the result is True. Ensure you still keep
separate non-parametrized tests that check boundary behaviors (e.g., the “does
not fire before interval” and “does not double fire” tests using
_SECONDS_PER_DAY) while referencing CalendarStrategy, should_fire_ceremony,
_make_ceremony, _make_context, and MeetingFrequency in the new test.
🪄 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: d0d53782-2695-4af2-b6a6-83a7bcc74002
📒 Files selected for processing (12)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.pytests/unit/engine/workflow/velocity_calculators/test_calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Deploy Preview
- GitHub Check: Agent
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect approved deviations from the original spec
Files:
docs/design/ceremony-scheduling.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python; Python 3.14+ has native PEP 649 lazy annotations
Useexcept A, B:syntax without parentheses (PEP 758) in Python 3.14; ruff enforces this
Add type hints to all public functions; enforce with mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Set 88-character line length enforced by ruff for Python code
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/velocity_calculators/test_calendar.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.pytests/unit/engine/workflow/strategies/test_calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pytests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Create new objects instead of mutating existing ones; usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement in non-Pydantic collections
Usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for mutable field handling
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state usingmodel_copy(update=...); never mix static config with mutable runtime fields
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); includeallow_inf_nan=Falsein allConfigDictdeclarations
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and usefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py,observability/http_handler.py)
Always name logger variableslogger(not_loggerorlog)
Use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly from the domain module
Use structured logging formatlogger.info(EVENT, key=value)instead oflogger.info('msg %s', val)
Log at WARNING or ERROR with context before raising on all error paths; log state transitions at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not require logging
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names like
example-provider,example-large-001,test-provider
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/velocity_calculators/test_calendar.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.pytests/unit/engine/workflow/strategies/test_calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pytests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers for test categorization
Maintain 80% minimum code coverage (enforced in CI)
Useasyncio_mode = 'auto'for async tests; do not manually add@pytest.mark.asyncio
Run pytest with-n autofor parallelism via pytest-xdist; never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis property-based testing with@given+@settingsdecorators; control viaHYPOTHESIS_PROFILEenv var (ci: 50 examples default,dev: 1000 examples)
Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mockingtime.monotonic()andasyncio.sleep()for determinism or usingasyncio.Event().wait()for indefinite blocking
Use Hypothesisciprofile (50 examples, default) anddevprofile (1000 examples) for property tests; run dev profile withHYPOTHESIS_PROFILE=dev uv run python -m pytest
Files:
tests/unit/engine/workflow/velocity_calculators/test_calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.py
🧠 Learnings (11)
📚 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-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-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-04-01T21:35:00.759Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T21:35:00.759Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/observability/events/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-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-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/observability/events/workflow.py
🔇 Additional comments (37)
src/synthorg/engine/workflow/strategies/__init__.py (1)
8-13: Public strategy exports look correct.
CalendarStrategyandHybridStrategyare cleanly re-exported and aligned with the package surface.Also applies to: 19-20
src/synthorg/engine/workflow/velocity_calculators/__init__.py (1)
7-12: Velocity calculator exports are correctly wired.The new calculator classes are exposed consistently via module imports and
__all__.Also applies to: 18-19
src/synthorg/observability/events/workflow.py (1)
106-110: New workflow velocity event constants are well-scoped.These constants are consistent with the existing observability event naming scheme.
docs/design/ceremony-scheduling.md (1)
678-685: Shipped-section update is clear and useful.The added section cleanly documents what landed in
#969/#970, including strategy/calculator/event additions.tests/unit/engine/workflow/velocity_calculators/test_calendar.py (1)
31-125: Test coverage forCalendarVelocityCalculatoris solid.The suite validates metadata, core computation, and weighted rolling-average semantics with clear assertions.
src/synthorg/engine/workflow/velocity_calculators/calendar.py (1)
39-117:CalendarVelocityCalculatorimplementation is clean and protocol-aligned.Compute/rolling-average behavior and observability hooks are implemented consistently.
tests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.py (1)
32-202:MultiDimensionalVelocityCalculatortests are comprehensive for edge cases.Nice coverage for missing task counts, weighted aggregates, and rolling-window behavior.
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py (1)
44-113: Implementation is robust and readable.Primary/secondary metric computation and weighted helper logic are well-structured and easy to validate.
Also applies to: 126-137
tests/unit/engine/workflow/strategies/test_calendar.py (7)
1-26: LGTM!Well-structured test module with clear organization. Imports are correct, module docstring is present, and helper functions match the actual model definitions from the codebase.
32-99: LGTM!Helper functions are well-designed with proper type hints and correctly construct the domain objects matching the actual model definitions (verified against context snippets for
CeremonyEvalContext,SprintCeremonyConfig, andSprint).
105-122: LGTM!Protocol conformance tests are comprehensive, verifying the strategy implements
CeremonySchedulingStrategy, exposes the correctstrategy_type, and returns the expected velocity calculator type.
186-228: LGTM!Good coverage of edge cases: missing frequency handling, strategy_config fallback, and independent per-ceremony tracking. The tests correctly verify the expected behavior.
233-299: LGTM!Sprint transition tests comprehensively cover the time-based transition logic, including the critical verification that task completion does not trigger calendar-based transitions.
305-351: LGTM!Config validation tests thoroughly cover valid and invalid cases for
duration_daysandfrequency, plus rejection of unknown keys.
357-389: LGTM!Lifecycle hook tests correctly verify that internal state (
_last_fire_elapsed) is cleared on bothon_sprint_activatedandon_sprint_deactivated, ensuring ceremonies can fire again at the same elapsed time after a sprint lifecycle transition.tests/unit/engine/workflow/strategies/test_hybrid.py (6)
1-113: LGTM!Well-structured test module with helper functions appropriately extended to support hybrid strategy's dual-leg (calendar + task-driven) configuration. The
_make_ceremonyhelper correctly handles the construction ofpolicy_overridewith trigger and threshold settings.
119-136: LGTM!Protocol conformance tests correctly verify
HybridStrategyimplements the protocol and returns the expectedHYBRIDstrategy type andMULTI_DIMENSIONALvelocity calculator.
142-306: LGTM!Excellent coverage of the hybrid first-wins logic, including critical tests for:
- Calendar-only and task-only firing
- Calendar fires first / task fires first scenarios
- Timer reset when task fires (verifying cadence reset behavior)
- Both thresholds met simultaneously
- Edge case with no frequency and no trigger
312-438: LGTM!Sprint transition tests comprehensively cover the hybrid first-wins behavior for transitions, including calendar-only, task-only, and combined scenarios, plus important edge cases like empty sprints and custom thresholds.
444-505: LGTM!Config validation tests thoroughly cover the hybrid strategy's combined configuration space, including valid permutations and rejection of invalid values for all supported config keys.
511-541: LGTM!Lifecycle hook tests correctly verify state clearing behavior, mirroring the
CalendarStrategytests for consistent behavior verification.src/synthorg/engine/workflow/strategies/calendar.py (8)
1-46: LGTM!Clean module setup with proper imports,
TYPE_CHECKINGusage for type-only dependencies, and immutable frozensets for config validation. The constants are well-organized and appropriately scoped.
48-73: LGTM!Well-documented class with proper
__slots__usage for memory efficiency. The docstring clearly explains the calendar-based firing behavior, frequency resolution, auto-transition logic, and state management.
75-123: LGTM!The
should_fire_ceremonymethod correctly implements calendar-based firing with double-fire prevention. Logging follows guidelines: INFO level for ceremony triggers (state transition), DEBUG level for skipped evaluations.
125-151: LGTM!Sprint transition logic correctly implements time-only transitions from
ACTIVEtoIN_REVIEWwhen theduration_daysboundary is reached, with proper resolution of duration from strategy config or defaultSprintConfig.
153-203: LGTM!Lifecycle hooks correctly implement state management: clearing
_last_fire_elapsedon sprint lifecycle transitions while leaving task/budget/external event hooks as no-ops (appropriate for a time-only strategy).
205-212: LGTM!Metadata methods correctly expose the strategy type and default velocity calculator as required by the protocol.
214-252: LGTM!Config validation is thorough with clear error messages. Unknown keys are rejected first, then
duration_daysis validated for type and range, andfrequencyis validated against known values.
254-293: LGTM!Internal helpers are well-structured with clear separation of concerns. The frequency resolution correctly handles both
ceremony.frequency(primary) andstrategy_config["frequency"](fallback) with proper error handling for invalid frequency strings.src/synthorg/engine/workflow/strategies/hybrid.py (8)
1-78: LGTM!Well-organized module setup with proper imports. Trigger constants are correctly imported from
ceremony_policy, and validation sets are appropriately defined as frozensets.
81-109: LGTM!Class documentation clearly explains the hybrid first-wins behavior with calendar and task-driven legs. Structure is consistent with
CalendarStrategy.
111-151: LGTM!The
should_fire_ceremonymethod correctly implements first-wins logic with both legs evaluated and calendar timer reset on any fire. Logging includes helpful diagnostic flags for which leg(s) triggered.
185-189: Defensive null check may be unnecessary.Per the
SprintConfigdefinition (context snippet 4),ceremony_policyhas a default factory that always providestransition_threshold=1.0. Theis not Nonecheck is defensive but the value should never beNoneat runtime.This is fine to keep for safety, just noting for awareness.
195-254: LGTM!Lifecycle hooks and metadata methods are consistent with
CalendarStrategy, with appropriate state clearing and correct strategy type/velocity calculator exposure.
256-279: LGTM!Config validation correctly delegates to extracted helper functions for each parameter type, following the PR objective of extracted validators.
348-398: LGTM!Extracted validation functions are clean and well-documented. The
TRY004suppression on line 354 appropriately maintains consistency with other validators usingValueError.
401-436: LGTM!The
_evaluate_task_triggerfunction comprehensively handles all defined triggers with proper threshold evaluation. The warning log for unrecognized triggers (lines 430-435) aligns with the PR objective about adding a warning log for unrecognized hybrid triggers.
There was a problem hiding this comment.
Pull request overview
Adds two new ceremony scheduling strategies (calendar-based and hybrid first-wins) to the workflow engine’s pluggable scheduler system, along with strategy-appropriate velocity calculators and unit tests, plus documentation/observability updates.
Changes:
- Implement
CalendarStrategyandHybridStrategyfor ceremony firing + sprint auto-transition evaluation. - Add
CalendarVelocityCalculator(pts/day) andMultiDimensionalVelocityCalculator(pts/sprint+ secondary metrics). - Export new strategy/calculator types, add observability event constants, and update the design doc roadmap/status.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/synthorg/engine/workflow/strategies/calendar.py | New calendar/time-based ceremony scheduling strategy with minimal internal fire-tracking state. |
| src/synthorg/engine/workflow/strategies/hybrid.py | New hybrid “first-wins” strategy combining time + task-driven triggers with stateful calendar reset behavior. |
| src/synthorg/engine/workflow/velocity_calculators/calendar.py | New velocity calculator computing duration-weighted pts/day rolling averages. |
| src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py | New pts/sprint velocity calculator with per-task/per-day/completion-ratio secondaries and rolling averages. |
| src/synthorg/engine/workflow/strategies/init.py | Exports the new strategy implementations. |
| src/synthorg/engine/workflow/velocity_calculators/init.py | Exports the new velocity calculator implementations. |
| src/synthorg/observability/events/workflow.py | Adds new velocity-related event constants for calculator edge cases. |
| docs/design/ceremony-scheduling.md | Moves Calendar/Hybrid from planned to shipped and updates velocity calculator table/roadmap. |
| tests/unit/engine/workflow/strategies/test_calendar.py | Unit tests for CalendarStrategy behavior, transitions, config validation, and lifecycle state clearing. |
| tests/unit/engine/workflow/strategies/test_hybrid.py | Unit tests for HybridStrategy firing logic, transitions, config validation, and lifecycle state clearing. |
| tests/unit/engine/workflow/velocity_calculators/test_calendar.py | Unit tests for CalendarVelocityCalculator compute + rolling average behaviors. |
| tests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.py | Unit tests for MultiDimensionalVelocityCalculator compute + rolling average behaviors across dimensions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if record.duration_days == 0: | ||
| logger.debug( | ||
| VELOCITY_CALENDAR_NO_DURATION, | ||
| sprint_id=record.sprint_id, | ||
| ) | ||
| return VelocityMetrics( | ||
| primary_value=0.0, | ||
| primary_unit=_UNIT, | ||
| secondary={ | ||
| "pts_per_sprint": record.story_points_completed, | ||
| }, | ||
| ) | ||
| pts_per_day = record.story_points_completed / record.duration_days |
There was a problem hiding this comment.
VelocityRecord.duration_days is constrained to ge=1 (see sprint_velocity.VelocityRecord), so the duration_days == 0 branch is unreachable and the VELOCITY_CALENDAR_NO_DURATION debug event will never be emitted. Consider removing this branch (and related event constant/docs), or relaxing the model constraint if zero-duration records are meant to be representable.
| """VelocityRecord has no task_completion_count for task-driven calculation.""" | ||
|
|
||
| VELOCITY_CALENDAR_NO_DURATION: str = "workflow.velocity.calendar_no_duration" | ||
| """CalendarVelocityCalculator received a record with zero duration_days.""" |
There was a problem hiding this comment.
This event constant appears to describe a condition that cannot occur with the current models (VelocityRecord.duration_days is ge=1). If you keep the constant for defensive purposes, consider updating the docstring to reflect when it can actually happen (e.g., invalid/unvalidated input) or remove it along with the dead-code branch in CalendarVelocityCalculator.
| """CalendarVelocityCalculator received a record with zero duration_days.""" | |
| """CalendarVelocityCalculator received an invalid VelocityRecord (e.g., zero duration_days from unvalidated input or a violated invariant).""" |
| n: int = config.get(_KEY_EVERY_N, _DEFAULT_EVERY_N) | ||
| return context.completions_since_last_trigger >= n | ||
|
|
||
| if trigger == TRIGGER_SPRINT_PERCENTAGE: | ||
| threshold: float = config.get( | ||
| _KEY_SPRINT_PERCENTAGE, | ||
| _DEFAULT_SPRINT_PCT, | ||
| ) |
There was a problem hiding this comment.
_evaluate_task_trigger assumes every_n_completions is an int and sprint_percentage is numeric. Per-ceremony policy_override.strategy_config isn’t validated by CeremonyScheduler.activate_sprint() today, so invalid types coming from config can raise TypeError here during evaluation. Suggest either validating each ceremony’s policy_override.strategy_config at activation, or adding local type checks/coercion with a warning + safe False fallback.
| n: int = config.get(_KEY_EVERY_N, _DEFAULT_EVERY_N) | |
| return context.completions_since_last_trigger >= n | |
| if trigger == TRIGGER_SPRINT_PERCENTAGE: | |
| threshold: float = config.get( | |
| _KEY_SPRINT_PERCENTAGE, | |
| _DEFAULT_SPRINT_PCT, | |
| ) | |
| raw_n: Any = config.get(_KEY_EVERY_N, _DEFAULT_EVERY_N) | |
| try: | |
| n = int(raw_n) | |
| except (TypeError, ValueError): | |
| logger.warning( | |
| SPRINT_CEREMONY_SKIPPED, | |
| trigger=trigger, | |
| reason="invalid_every_n_config", | |
| value=raw_n, | |
| ) | |
| return False | |
| if n <= 0: | |
| logger.warning( | |
| SPRINT_CEREMONY_SKIPPED, | |
| trigger=trigger, | |
| reason="non_positive_every_n_config", | |
| value=raw_n, | |
| ) | |
| return False | |
| return context.completions_since_last_trigger >= n | |
| if trigger == TRIGGER_SPRINT_PERCENTAGE: | |
| raw_threshold: Any = config.get( | |
| _KEY_SPRINT_PERCENTAGE, | |
| _DEFAULT_SPRINT_PCT, | |
| ) | |
| try: | |
| threshold = float(raw_threshold) | |
| except (TypeError, ValueError): | |
| logger.warning( | |
| SPRINT_CEREMONY_SKIPPED, | |
| trigger=trigger, | |
| reason="invalid_sprint_percentage_config", | |
| value=raw_threshold, | |
| ) | |
| return False |
docs/design/ceremony-scheduling.md
Outdated
| - `CalendarVelocityCalculator` -- `pts/day` with duration-weighted rolling averages | ||
| - `HybridStrategy` -- first-wins between calendar (floor) and task-driven (ceiling) triggers | ||
| - `MultiDimensionalVelocityCalculator` -- `pts/sprint` with `pts_per_task`, `pts_per_day`, `completion_ratio` secondaries | ||
| - Observability event constants (`VELOCITY_CALENDAR_NO_DURATION`, `VELOCITY_MULTI_NO_TASK_COUNT`) |
There was a problem hiding this comment.
The design doc lists VELOCITY_CALENDAR_NO_DURATION as shipped observability surface, but VelocityRecord.duration_days is currently validated as >= 1, so this event won’t be emitted in normal operation. If you remove the dead-code path/event constant, update this shipped list accordingly; if you keep it, clarify that it only applies to invalid/unvalidated records.
| - Observability event constants (`VELOCITY_CALENDAR_NO_DURATION`, `VELOCITY_MULTI_NO_TASK_COUNT`) | |
| - Observability event constants (`VELOCITY_CALENDAR_NO_DURATION` — emitted only for invalid/unvalidated records, `VELOCITY_MULTI_NO_TASK_COUNT`) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #985 +/- ##
==========================================
- Coverage 91.94% 91.93% -0.01%
==========================================
Files 638 643 +5
Lines 34578 34897 +319
Branches 3377 3423 +46
==========================================
+ Hits 31792 32082 +290
- Misses 2193 2211 +18
- Partials 593 604 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 678-685: The design doc's Observability and lifecycle sections are
out of sync with the roadmap note: update the canonical `## Observability` table
to include the new velocity events (`VELOCITY_CALENDAR_NO_DURATION`,
`VELOCITY_MULTI_NO_TASK_COUNT`, `VELOCITY_MULTI_NO_DURATION`) and add entries
describing when they are emitted; update the lifecycle discussion to reflect
that `CalendarStrategy` is not a no-op (mention `CalendarVelocityCalculator`
behavior and the `HybridStrategy` first-wins semantics), and add a brief
description of `MultiDimensionalVelocityCalculator` (`pts/sprint` with
`pts_per_task`, `pts_per_day`, `completion_ratio`) so the page's reference
sections match the roadmap note.
In `@src/synthorg/engine/workflow/strategies/_helpers.py`:
- Around line 181-193: The current TRIGGER_SPRINT_PERCENTAGE branch only
type-checks raw_threshold (from KEY_SPRINT_PERCENTAGE) but does not bound-check
it, letting 0/negative values always pass and >MAX_SPRINT_PCT never pass; fix by
validating and normalizing the value before comparing: after the existing type
check, coerce raw_threshold to a numeric (float), then if raw_threshold <= 0 or
raw_threshold > MAX_SPRINT_PCT log the same SPRINT_CEREMONY_SKIPPED with reason
like "invalid_sprint_percentage_bounds" (include value) and return False;
otherwise compute threshold = raw_threshold / MAX_SPRINT_PCT and keep the
existing return has_tasks and pct >= threshold behavior.
In `@src/synthorg/engine/workflow/strategies/hybrid.py`:
- Around line 243-266: The validate_strategy_config function currently raises
ValueError without logging; add a warning log before raising for the
unknown-keys path (use _KNOWN_CONFIG_KEYS and the same message) and wrap each
call to validate_duration_days, validate_trigger, validate_every_n,
validate_sprint_percentage, and validate_frequency in a try/except ValueError
that logs a warning with contextual info (include the offending key/value and
function name) and then re-raises; also ensure a module-level logger (e.g.,
logger) is defined/used so logs follow project conventions and consider doing
the same logging pattern inside the helper validators in _helpers.py for
consistency.
In `@src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py`:
- Around line 74-82: Wrap the mutable secondary mapping in an immutable proxy
before constructing VelocityMetrics: import deepcopy and MappingProxyType at
module scope, build the dict for "secondary" (containing pts_per_task,
pts_per_day, completion_ratio from record) then call
MappingProxyType(deepcopy(secondary_dict)) and pass that proxy as the secondary
argument to VelocityMetrics; apply this change at both return sites that
construct VelocityMetrics (the one returning primary_value=pts_sprint and the
other at lines ~113-122) so callers cannot mutate metrics.secondary.
In `@tests/unit/engine/workflow/strategies/conftest.py`:
- Around line 11-35: Update the make_sprint factory to accept optional
story_points_committed and story_points_completed parameters (defaulting to
current derived values) and set them into the kwargs passed to Sprint so tests
can override point totals; modify the other shared factory mentioned (lines
38-58) the same way to expose the same two optional parameters; keep current
default behavior (story points = task_count * 3 and completed = completed_count
* 3) when the new args are not provided and do not change any other fields or
flow in make_sprint/Sprint creation.
In `@tests/unit/engine/workflow/strategies/test_hybrid.py`:
- Around line 599-613: The test calls strategy.should_fire_ceremony(ceremony,
sprint, ctx) to create tracked state but does not assert its return value, so if
it returns False the final assertion is meaningless; update
test_on_sprint_activated_clears_state to assert that the initial
should_fire_ceremony(...) call returns True (ensuring state was created) before
calling await strategy.on_sprint_activated(sprint, SprintConfig()) and then
re-checking should_fire_ceremony(...).
- Around line 615-626: The test fails to assert the state created by the initial
call to HybridStrategy.should_fire_ceremony before calling
on_sprint_deactivated; update the test to call
strategy.should_fire_ceremony(ceremony, sprint, ctx) and assert it returns True
(verifying the fired state was set) prior to awaiting
strategy.on_sprint_deactivated(), then assert the post-deactivation call returns
True as already present; reference HybridStrategy.should_fire_ceremony and
HybridStrategy.on_sprint_deactivated and the local variables ceremony, sprint,
ctx to locate the lines to change.
- Around line 548-559: Consolidate the three separate tests that assert invalid
sprint_percentage (the tests calling HybridStrategy().validate_strategy_config
with 0, -10, and 101) into a single parametrized test using
pytest.mark.parametrize; keep the assertion that a ValueError is raised and
match "between", and reference the HybridStrategy class and its
validate_strategy_config method so the new test iterates over values [0, -10,
101] (with descriptive ids like "zero", "negative", "over_100") to replace
test_invalid_sprint_percentage_zero, test_invalid_sprint_percentage_negative,
and the over-100 case.
🪄 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: abd17f91-9f34-4dbf-931d-de55158ebaef
📒 Files selected for processing (10)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python; Python 3.14+ has native PEP 649 lazy annotations
Useexcept A, B:syntax without parentheses (PEP 758) in Python 3.14; ruff enforces this
Add type hints to all public functions; enforce with mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Set 88-character line length enforced by ruff for Python code
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/_helpers.pytests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Create new objects instead of mutating existing ones; usecopy.deepcopy()at construction plusMappingProxyTypewrapping for read-only enforcement in non-Pydantic collections
Usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for mutable field handling
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state usingmodel_copy(update=...); never mix static config with mutable runtime fields
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict); includeallow_inf_nan=Falsein allConfigDictdeclarations
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and usefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py,observability/http_handler.py)
Always name logger variableslogger(not_loggerorlog)
Use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly from the domain module
Use structured logging formatlogger.info(EVENT, key=value)instead oflogger.info('msg %s', val)
Log at WARNING or ERROR with context before raising on all error paths; log state transitions at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not require logging
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples; use generic names like
example-provider,example-large-001,test-provider
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/_helpers.pytests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect approved deviations from the original spec
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, and@pytest.mark.slowmarkers for test categorization
Maintain 80% minimum code coverage (enforced in CI)
Useasyncio_mode = 'auto'for async tests; do not manually add@pytest.mark.asyncio
Run pytest with-n autofor parallelism via pytest-xdist; never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis property-based testing with@given+@settingsdecorators; control viaHYPOTHESIS_PROFILEenv var (ci: 50 examples default,dev: 1000 examples)
Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mockingtime.monotonic()andasyncio.sleep()for determinism or usingasyncio.Event().wait()for indefinite blocking
Use Hypothesisciprofile (50 examples, default) anddevprofile (1000 examples) for property tests; run dev profile withHYPOTHESIS_PROFILE=dev uv run python -m pytest
Files:
tests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.py
🧠 Learnings (20)
📚 Learning: 2026-04-01T21:35:00.759Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T21:35:00.759Z
Learning: Applies to {pyproject.toml,src/synthorg/__init__.py} : Update version in `pyproject.toml` (`[tool.commitizen].version`) and `src/synthorg/__init__.py` (`__version__`)
Applied to files:
src/synthorg/engine/workflow/__init__.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-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-01T21:35:00.759Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T21:35:00.759Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/observability/events/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-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: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-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-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-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_calendar.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_calendar.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_calendar.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_calendar.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_calendar.py
📚 Learning: 2026-04-01T21:35:00.759Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T21:35:00.759Z
Learning: Applies to tests/**/*.py : Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mocking `time.monotonic()` and `asyncio.sleep()` for determinism or using `asyncio.Event().wait()` for indefinite blocking
Applied to files:
tests/unit/engine/workflow/strategies/test_calendar.py
📚 Learning: 2026-04-01T21:35:00.759Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T21:35:00.759Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/engine/workflow/strategies/test_calendar.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/hr/**/*.py : HR package (hr/): hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, LLM calibration, collaboration overrides, trend detection), promotion/demotion (criteria evaluation, approval strategies, model mapping)
Applied to files:
src/synthorg/engine/workflow/strategies/_helpers.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/_helpers.py
🔇 Additional comments (10)
tests/unit/engine/workflow/strategies/test_hybrid.py (4)
29-60: LGTM - well-structured test helper.The
_make_ceremonyhelper cleanly abstracts ceremony configuration for both calendar-only and task-driven scenarios, keeping test bodies focused on assertions.
66-83: Protocol conformance tests look correct.Tests properly verify
CeremonySchedulingStrategyprotocol compliance,strategy_type, and default velocity calculator.
174-204: Good test for calendar timer reset semantics.This test correctly verifies that when the task leg fires first, the calendar timer resets, and subsequent calendar checks use the new baseline. The elapsed time calculations (50000 + 86400 = 136400) are accurate.
330-363: Solid coverage for infinite refire prevention.This test validates the critical fix for percentage-based triggers that stay true once crossed. The three-step pattern (initial fire → suppression → next interval fire) thoroughly exercises the suppression logic.
src/synthorg/engine/workflow/strategies/hybrid.py (6)
1-47: LGTM - proper module setup.Imports are correctly organized, logger is properly initialized using
get_logger(__name__), and observability event constants are imported from the domain module.
60-92: Well-documented class with clear state management.The docstring thoroughly explains the first-wins semantics, the two trigger legs, and the
_last_fire_elapsedstate tracking. Using__slots__is appropriate for this lightweight strategy object.
94-136: LGTM - correct first-wins implementation with proper logging.The short-circuit evaluation (
False if calendar_fires else ...) correctly implements the intended behavior where task-driven checking is skipped when calendar fires. Logging uses structured format with event constants.
180-192: Lifecycle hooks correctly clear state.Both
on_sprint_activatedandon_sprint_deactivatedproperly clear_last_fire_elapsedto prevent cross-sprint state carryover.
282-312: Suppression logic is sound.The task-driven leg correctly suppresses re-firing for percentage-based triggers that remain true once crossed, while still allowing the task leg to fire before the first calendar interval (preserving first-wins semantics). This addresses the infinite firing loop fix mentioned in PR objectives.
169-176: Theceremony_policyfield is guaranteed to be non-None and does not require a None check.The field is declared with type
CeremonyPolicyConfig(notOptional) and includes adefault_factorythat always creates a default instance. Pydantic v2 enforces that non-optional fields have values at instantiation, and the model is frozen, preventing runtime mutations toNone. The code at line 171 safely accessesconfig.ceremony_policy.transition_thresholdwithout risk.> Likely an incorrect or invalid review comment.
| def validate_strategy_config( | ||
| self, | ||
| config: Mapping[str, Any], | ||
| ) -> None: | ||
| """Validate hybrid strategy config. | ||
|
|
||
| Accepts keys from both calendar and task-driven strategies. | ||
|
|
||
| Args: | ||
| config: Strategy config to validate. | ||
|
|
||
| Raises: | ||
| ValueError: If the config contains invalid keys or values. | ||
| """ | ||
| unknown = set(config) - _KNOWN_CONFIG_KEYS | ||
| if unknown: | ||
| msg = f"Unknown config keys: {sorted(unknown)}" | ||
| raise ValueError(msg) | ||
|
|
||
| validate_duration_days(config.get(KEY_DURATION_DAYS)) | ||
| validate_trigger(config.get(KEY_TRIGGER)) | ||
| validate_every_n(config.get(KEY_EVERY_N)) | ||
| validate_sprint_percentage(config.get(KEY_SPRINT_PERCENTAGE)) | ||
| validate_frequency(config.get(KEY_FREQUENCY)) |
There was a problem hiding this comment.
Add warning log before raising ValueError.
The validation method raises ValueError on multiple paths without logging first. As per coding guidelines: "Log at WARNING or ERROR with context before raising on all error paths".
Proposed fix for the unknown keys case
unknown = set(config) - _KNOWN_CONFIG_KEYS
if unknown:
msg = f"Unknown config keys: {sorted(unknown)}"
+ logger.warning(msg, unknown_keys=sorted(unknown))
raise ValueError(msg)The individual validators in _helpers.py likely also raise without logging—consider adding logging there as well for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/hybrid.py` around lines 243 - 266,
The validate_strategy_config function currently raises ValueError without
logging; add a warning log before raising for the unknown-keys path (use
_KNOWN_CONFIG_KEYS and the same message) and wrap each call to
validate_duration_days, validate_trigger, validate_every_n,
validate_sprint_percentage, and validate_frequency in a try/except ValueError
that logs a warning with contextual info (include the offending key/value and
function name) and then re-raises; also ensure a module-level logger (e.g.,
logger) is defined/used so logs follow project conventions and consider doing
the same logging pattern inside the helper validators in _helpers.py for
consistency.
| return VelocityMetrics( | ||
| primary_value=pts_sprint, | ||
| primary_unit=_UNIT, | ||
| secondary={ | ||
| "pts_per_task": pts_per_task, | ||
| "pts_per_day": pts_per_day, | ||
| "completion_ratio": record.completion_ratio, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Freeze secondary before constructing VelocityMetrics.
VelocityMetrics is frozen, but these plain dicts are still mutable after construction, so callers can mutate metrics.secondary and change the calculator output out from under the model. Wrap the mapping in a read-only proxy at both return sites.
♻️ Suggested fix
- return VelocityMetrics(
- primary_value=pts_sprint,
- primary_unit=_UNIT,
- secondary={
- "pts_per_task": pts_per_task,
- "pts_per_day": pts_per_day,
- "completion_ratio": record.completion_ratio,
- },
- )
+ return VelocityMetrics(
+ primary_value=pts_sprint,
+ primary_unit=_UNIT,
+ secondary=MappingProxyType(
+ deepcopy(
+ {
+ "pts_per_task": pts_per_task,
+ "pts_per_day": pts_per_day,
+ "completion_ratio": record.completion_ratio,
+ }
+ )
+ ),
+ )- return VelocityMetrics(
- primary_value=total_pts / n,
- primary_unit=_UNIT,
- secondary={
- "pts_per_task": _weighted_pts_per_task(recent),
- "pts_per_day": (total_pts / total_days if total_days > 0 else 0.0),
- "completion_ratio": (sum(r.completion_ratio for r in recent) / n),
- "sprints_averaged": float(n),
- },
- )
+ return VelocityMetrics(
+ primary_value=total_pts / n,
+ primary_unit=_UNIT,
+ secondary=MappingProxyType(
+ deepcopy(
+ {
+ "pts_per_task": _weighted_pts_per_task(recent),
+ "pts_per_day": (
+ total_pts / total_days if total_days > 0 else 0.0
+ ),
+ "completion_ratio": (
+ sum(r.completion_ratio for r in recent) / n
+ ),
+ "sprints_averaged": float(n),
+ }
+ )
+ ),
+ )This also needs from copy import deepcopy and from types import MappingProxyType at module scope.
As per coding guidelines, "Create new objects instead of mutating existing ones; use copy.deepcopy() at construction plus MappingProxyType wrapping for read-only enforcement in non-Pydantic collections".
Also applies to: 113-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py`
around lines 74 - 82, Wrap the mutable secondary mapping in an immutable proxy
before constructing VelocityMetrics: import deepcopy and MappingProxyType at
module scope, build the dict for "secondary" (containing pts_per_task,
pts_per_day, completion_ratio from record) then call
MappingProxyType(deepcopy(secondary_dict)) and pass that proxy as the secondary
argument to VelocityMetrics; apply this change at both return sites that
construct VelocityMetrics (the one returning primary_value=pts_sprint and the
other at lines ~113-122) so callers cannot mutate metrics.secondary.
Implement CalendarStrategy and CalendarVelocityCalculator for time-based ceremony scheduling using MeetingFrequency intervals. CalendarStrategy fires ceremonies when wall-clock time crosses frequency interval boundaries (daily, weekly, bi_weekly, etc.). Auto-transitions ACTIVE to IN_REVIEW at the duration_days boundary. Uses minimal internal state (_last_fire_elapsed dict) to track per-ceremony fire times, preventing double-firing within intervals. CalendarVelocityCalculator measures velocity as pts/day with duration-weighted rolling averages. Also adds VELOCITY_CALENDAR_NO_DURATION and VELOCITY_MULTI_NO_TASK_COUNT observability event constants for the new calculators. Closes #969
Implement HybridStrategy and MultiDimensionalVelocityCalculator for combined calendar + task-driven ceremony scheduling. HybridStrategy evaluates both calendar (time-based floor) and task-driven (throughput ceiling) triggers. Whichever fires first wins and resets the cadence counter. Auto-transitions on whichever comes first: task completion threshold or calendar duration boundary. MultiDimensionalVelocityCalculator provides pts/sprint as primary metric with pts_per_task, pts_per_day, and completion_ratio as secondary dimensions for comprehensive velocity tracking. Closes #970
- Add warning log for unrecognized trigger in _evaluate_task_trigger (consistency with TaskDrivenStrategy reference implementation) - Extract validation helpers from HybridStrategy.validate_strategy_config to bring it under the 50-line function limit - Extract _weighted_pts_per_task helper from MultiDimensionalVelocityCalculator.rolling_average (50-line limit) - Update docs/design/ceremony-scheduling.md: remove (planned) labels from CalendarVelocityCalculator and MultiDimensionalVelocityCalculator, move #969/#970 to shipped section Pre-reviewed by 4 agents, 5 findings addressed
…, and Copilot Critical fixes: - Fix infinite firing loop for percentage-based task triggers in HybridStrategy (sprint_end, sprint_midpoint, sprint_percentage now suppressed until next calendar interval after firing) - Add missing re-exports to workflow/__init__.py (CalendarStrategy, HybridStrategy, CalendarVelocityCalculator, MultiDimensionalVelocityCalculator) Major fixes: - Add zero-duration guard in MultiDimensionalVelocityCalculator.compute() (consistency with CalendarVelocityCalculator) - Reject bool as int in duration_days/every_n validation (isinstance(True, int) is True in Python) - Log warning on invalid frequency in _resolve_interval (was silently swallowed) - Log warning on invalid duration_days fallback in _resolve_duration_days - Add runtime type checking in evaluate_task_trigger for per-ceremony config - Document TRIGGER_SPRINT_START as lifecycle-hook-only (intentional no-op) - Add tests for sprint_start, sprint_end, sprint_midpoint, unrecognized triggers Structural improvements: - Extract shared helpers to strategies/_helpers.py (eliminates duplication across CalendarStrategy, HybridStrategy, and TaskDrivenStrategy) - Extract shared test helpers to conftest.py - Parametrize interval tests with @pytest.mark.parametrize - Add sprint_percentage boundary validation tests (0, negative) - Fix design doc velocity calculator default mapping (hybrid only) - Fix module docstrings (cadence counter -> calendar timer, ceiling -> milestones) - Add completion_ratio weighting note to rolling_average docstring - Add VELOCITY_MULTI_NO_DURATION event constant - Clarify VELOCITY_CALENDAR_NO_DURATION docstring scope
- Add bounds check for sprint_percentage in evaluate_task_trigger (reject 0/negative/>100 at runtime, not just at config validation) - Add velocity event constants to design doc observability table - Add optional story_points_committed/completed params to test factories - Assert initial should_fire_ceremony return in lifecycle hook tests - Parametrize invalid sprint_percentage validation tests
817312b to
11e1ece
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py (1)
74-82:⚠️ Potential issue | 🟠 MajorFreeze
secondarybefore buildingVelocityMetrics.
secondaryis currently a plain dict at both return sites, so callers can mutatemetrics.secondarypost-construction.♻️ Suggested fix
+from copy import deepcopy +from types import MappingProxyType @@ return VelocityMetrics( primary_value=pts_sprint, primary_unit=_UNIT, - secondary={ - "pts_per_task": pts_per_task, - "pts_per_day": pts_per_day, - "completion_ratio": record.completion_ratio, - }, + secondary=MappingProxyType( + deepcopy( + { + "pts_per_task": pts_per_task, + "pts_per_day": pts_per_day, + "completion_ratio": record.completion_ratio, + } + ) + ), ) @@ return VelocityMetrics( primary_value=total_pts / n, primary_unit=_UNIT, - secondary={ - "pts_per_task": _weighted_pts_per_task(recent), - "pts_per_day": (total_pts / total_days if total_days > 0 else 0.0), - "completion_ratio": (sum(r.completion_ratio for r in recent) / n), - "sprints_averaged": float(n), - }, + secondary=MappingProxyType( + deepcopy( + { + "pts_per_task": _weighted_pts_per_task(recent), + "pts_per_day": ( + total_pts / total_days if total_days > 0 else 0.0 + ), + "completion_ratio": ( + sum(r.completion_ratio for r in recent) / n + ), + "sprints_averaged": float(n), + } + ) + ), )As per coding guidelines, "Create new objects instead of mutating existing ones... use
copy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement."Also applies to: 113-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py` around lines 74 - 82, The returned VelocityMetrics instances expose a mutable plain dict as secondary; make secondary immutable by creating a deep copy of the dict and wrapping it in MappingProxyType before passing it to VelocityMetrics. Update both return sites that build the secondary dict (the one using pts_sprint/pts_per_task/pts_per_day/record.completion_ratio and the similar block later) to import copy and types.MappingProxyType, call copy.deepcopy(...) on the dict, then wrap the result with MappingProxyType(...) so callers cannot mutate metrics.secondary after construction.src/synthorg/engine/workflow/strategies/hybrid.py (1)
257-266:⚠️ Potential issue | 🟡 MinorAdd warning logging before raising for invalid strategy config.
The unknown-key path raises
ValueErrordirectly, and validator failures propagated from helper calls are not logged in this method.As per coding guidelines, "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/hybrid.py` around lines 257 - 266, The function currently raises ValueError for unknown config keys and lets validation helpers (validate_duration_days, validate_trigger, validate_every_n, validate_sprint_percentage, validate_frequency) propagate errors without logging; update this block to log a warning with context before raising: when unknown := set(config) - _KNOWN_CONFIG_KEYS is non-empty, call the module logger.warn/error with a message that includes the sorted unknown keys and the config (or relevant id) and then raise ValueError(msg); for the calls to validate_duration_days/validate_trigger/validate_every_n/validate_sprint_percentage/validate_frequency, wrap them in try/except that logs a warning including which KEY_* constant failed (KEY_DURATION_DAYS, KEY_TRIGGER, KEY_EVERY_N, KEY_SPRINT_PERCENTAGE, KEY_FREQUENCY) and the config value, then re-raise the original exception.
🤖 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/_helpers.py`:
- Around line 215-270: The validators (validate_duration_days, validate_trigger,
validate_every_n, validate_sprint_percentage, validate_frequency) raise
ValueError without logging; add a structured warning log immediately before each
raise. Use the module logger (create one with logging.getLogger(__name__) if not
present) and call logger.warning with context that includes the config
key/identifier (e.g., KEY_DURATION_DAYS, KEY_EVERY_N, KEY_SPRINT_PERCENTAGE or
the value/allowed set like VALID_TRIGGERS/VALID_FREQUENCIES), the offending
value, and a short reason, then raise the same ValueError as before.
In `@src/synthorg/engine/workflow/strategies/calendar.py`:
- Around line 225-227: Before raising ValueError for unknown config keys, log a
warning with context: call the module/class logger (or
logging.getLogger(__name__) if no logger exists) to emit a logger.warning that
includes the same details as msg (e.g., sorted(unknown)) and any surrounding
context (function name or config being validated) so the unknown variable and
msg are logged before the raise; then raise the ValueError as before.
- Around line 145-147: The code returns SprintStatus.IN_REVIEW when
context.elapsed_seconds >= duration_seconds but does not log the state
transition; add an INFO-level log just before returning IN_REVIEW that records
the strategy name, sprint id, elapsed/duration (e.g., strategy,
context.sprint_id, context.elapsed_seconds, duration_seconds) to satisfy the
"All state transitions must log at INFO level" requirement — place this log in
the function that evaluates sprint status (the block using
context.elapsed_seconds and duration_seconds) so the transition to
SprintStatus.IN_REVIEW is always emitted to the logger.
In `@src/synthorg/engine/workflow/strategies/hybrid.py`:
- Around line 165-176: The sprint transition paths return SprintStatus.IN_REVIEW
without logging; update the decision points in the method that checks
context.elapsed_seconds and the task-driven branch (the blocks using
context.elapsed_seconds, context.total_tasks_in_sprint, threshold computed from
config.ceremony_policy.transition_threshold or DEFAULT_TRANSITION_THRESHOLD, and
context.sprint_percentage_complete) to emit an INFO-level log immediately before
returning, e.g., logger.info or processLogger.info, including the chosen
transition reason ("time_elapsed" or "task_threshold"), the relevant values
(elapsed_seconds, duration_seconds, sprint_percentage_complete, threshold) and
any identifying sprint/context id so the transition is recorded per the "All
state transitions must log at INFO level" guideline.
- Around line 293-312: The interval-based suppression currently runs before the
trigger is read and blocks all triggers (including non-sticky ones like
every_n_completions); move the interval check to after you extract trigger =
config.get(KEY_TRIGGER) and only apply it for sticky/percentage-style triggers
(e.g. "sprint_end", "sprint_midpoint", "sprint_percentage" or a config flag like
sticky=True if available). Concretely: fetch
ceremony.policy_override.strategy_config and trigger first (using KEY_TRIGGER),
then if trigger is one of the sticky triggers (or config indicates sticky)
consult self._last_fire_elapsed[ceremony.name] and suppress (return False) only
when elapsed < interval; otherwise proceed to return
evaluate_task_trigger(trigger, config, context).
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/hybrid.py`:
- Around line 257-266: The function currently raises ValueError for unknown
config keys and lets validation helpers (validate_duration_days,
validate_trigger, validate_every_n, validate_sprint_percentage,
validate_frequency) propagate errors without logging; update this block to log a
warning with context before raising: when unknown := set(config) -
_KNOWN_CONFIG_KEYS is non-empty, call the module logger.warn/error with a
message that includes the sorted unknown keys and the config (or relevant id)
and then raise ValueError(msg); for the calls to
validate_duration_days/validate_trigger/validate_every_n/validate_sprint_percentage/validate_frequency,
wrap them in try/except that logs a warning including which KEY_* constant
failed (KEY_DURATION_DAYS, KEY_TRIGGER, KEY_EVERY_N, KEY_SPRINT_PERCENTAGE,
KEY_FREQUENCY) and the config value, then re-raise the original exception.
In `@src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py`:
- Around line 74-82: The returned VelocityMetrics instances expose a mutable
plain dict as secondary; make secondary immutable by creating a deep copy of the
dict and wrapping it in MappingProxyType before passing it to VelocityMetrics.
Update both return sites that build the secondary dict (the one using
pts_sprint/pts_per_task/pts_per_day/record.completion_ratio and the similar
block later) to import copy and types.MappingProxyType, call copy.deepcopy(...)
on the dict, then wrap the result with MappingProxyType(...) so callers cannot
mutate metrics.secondary after construction.
🪄 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: e9bcb388-cdb2-478d-a1a8-edcbf7f1e079
📒 Files selected for processing (15)
docs/design/ceremony-scheduling.mdsrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.pytests/unit/engine/workflow/velocity_calculators/test_calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.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 Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python 3.14+ required (PEP 649 native lazy annotations).
Nofrom __future__ import annotationsin Python code - Python 3.14 has PEP 649.
Use PEP 758 except syntax:except A, B:(no parentheses) - ruff enforces this on Python 3.14.
All public functions and classes require type hints and Google-style docstrings (enforced by ruff D rules).
Use mypy strict mode for type checking on all Python code.
Line length: 88 characters (enforced by ruff).
Files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pytests/unit/engine/workflow/velocity_calculators/test_calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pytests/unit/engine/workflow/strategies/conftest.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/strategies/_helpers.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Create new objects instead of mutating existing ones. For non-Pydantic internal collections, usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement.
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state that evolves. Never mix static config fields with mutable runtime fields.
Use Pydantic v2 withallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.
Use@computed_fieldfor derived values in Pydantic models instead of storing and validating redundant fields.
UseNotBlankStr(fromcore.types) for all identifier and name fields in Pydantic models, including optional and tuple variants, instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code. Prefer structured concurrency over barecreate_task.
Keep functions under 50 lines and files under 800 lines.
Handle errors explicitly, never silently swallow them.
Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/strategies/_helpers.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(), orprint()in application code (exception: observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py may use stdlib logging).
Always useloggeras the variable name (not_logger, notlog) for logger instances.
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 synthorg.observability.events.<domain> import EVENT_CONSTANT.
Use structured logging withlogger.info(EVENT, key=value)- never use string formatting likelogger.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 level for object creation, internal flow, and entry/exit of key functions.
Pure data models, enums, and re-exports do NOT need logging.
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,test-provider,test-small-001.
Library reference indocs/api/is auto-generated via mkdocstrings and Griffe (AST-based) from Google-style docstrings.
Files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/strategies/_helpers.py
src/synthorg/engine/workflow/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Workflow orchestration includes Kanban board, Agile sprints, WIP limits, velocity tracking, and pluggable scheduling strategies in
src/synthorg/engine/workflow/.
Files:
src/synthorg/engine/workflow/velocity_calculators/__init__.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/strategies/_helpers.py
src/synthorg/observability/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Observability includes structured logging, correlation tracking, redaction, third-party logger taming, log shipping (syslog, HTTP), and compressed archival in
src/synthorg/observability/.
Files:
src/synthorg/observability/events/workflow.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/ceremony-scheduling.md
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation is in
docs/(Markdown, built with Zensical, config:mkdocs.yml). Includes design spec, architecture, roadmap, security, licensing, reference, and auto-generated API reference.
Files:
docs/design/ceremony-scheduling.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowto categorize test scope.
Maintain 80% minimum code coverage (enforced in CI). Run tests with-n autofor parallelism via pytest-xdist.
Useasyncio_mode = 'auto'- no manual@pytest.mark.asyncioneeded. Global timeout is 30 seconds per test.
Prefer@pytest.mark.parametrizefor testing similar cases.
Use Hypothesis for property-based testing in Python with@givenand@settingsdecorators. Run withHYPOTHESIS_PROFILE=dev uv run pytest tests/ -m unit -n auto -k propertiesfor dev profile (1000 examples).
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins. Useasyncio.Event().wait()for tasks blocking indefinitely.
Files:
tests/unit/engine/workflow/velocity_calculators/test_calendar.pytests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.pytests/unit/engine/workflow/strategies/conftest.pytests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.py
🧠 Learnings (43)
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to src/synthorg/engine/workflow/**/*.py : Workflow orchestration includes Kanban board, Agile sprints, WIP limits, velocity tracking, and pluggable scheduling strategies in `src/synthorg/engine/workflow/`.
Applied to files:
src/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/velocity_calculators/calendar.pysrc/synthorg/engine/workflow/velocity_calculators/multi_dimensional.pytests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.pysrc/synthorg/engine/workflow/strategies/_helpers.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-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-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
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: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/engine/workflow/__init__.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/engine/workflow/__init__.py
📚 Learning: 2026-03-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 docs/design/*.md : Update the relevant `docs/design/` page when approved deviations occur to reflect the new reality
Applied to files:
docs/design/ceremony-scheduling.md
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to docs/design/**/*.md : When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/ceremony-scheduling.md
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
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 docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/ceremony-scheduling.md
📚 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: Always read the relevant `docs/design/` page before implementing any feature or planning any issue — DESIGN_SPEC.md is a pointer file linking to 7 design pages (Agents, Organization, Communication, Engine, Memory, Operations)
Applied to files:
docs/design/ceremony-scheduling.md
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to src/**/*.py : Create new objects instead of mutating existing ones. For non-Pydantic internal collections, use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to src/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state that evolves. Never mix static config fields with mutable runtime fields.
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.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_calendar.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_calendar.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/workflow/strategies/test_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to tests/**/*.py : For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins. Use `asyncio.Event().wait()` for tasks blocking indefinitely.
Applied to files:
tests/unit/engine/workflow/strategies/test_calendar.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_calendar.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_calendar.pytests/unit/engine/workflow/strategies/test_hybrid.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 : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/engine/workflow/strategies/test_calendar.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_hybrid.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to tests/**/*.py : Maintain 80% minimum code coverage (enforced in CI). Run tests with `-n auto` for parallelism via pytest-xdist.
Applied to files:
tests/unit/engine/workflow/strategies/test_hybrid.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 : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
tests/unit/engine/workflow/strategies/test_hybrid.pysrc/synthorg/engine/workflow/strategies/_helpers.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/hybrid.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
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/hybrid.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/hybrid.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/hybrid.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/hybrid.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/hybrid.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/hybrid.py
📚 Learning: 2026-03-15T21:49:53.264Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:49:53.264Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
src/synthorg/engine/workflow/strategies/_helpers.py
🔇 Additional comments (13)
src/synthorg/engine/workflow/strategies/__init__.py (1)
8-21: LGTM!Clean re-export additions for
CalendarStrategyandHybridStrategy. The__all__list maintains alphabetical order, and the imports are correctly structured.src/synthorg/engine/workflow/velocity_calculators/__init__.py (1)
7-21: LGTM!Clean re-export additions for the new velocity calculators. The
__all__list is properly alphabetized.src/synthorg/engine/workflow/__init__.py (1)
58-68: LGTM!Public API correctly extended with the new strategy and velocity calculator exports. The
__all__list maintains alphabetical ordering.src/synthorg/observability/events/workflow.py (1)
105-121: LGTM!New velocity event constants are well-documented with clear docstrings explaining their purpose as defensive guards. The naming follows the established
VELOCITY_*pattern and matches the imports in the velocity calculator implementations.docs/design/ceremony-scheduling.md (3)
449-450: LGTM!Calculator table correctly updated:
CalendarVelocityCalculatormapped tocalendarstrategy andMultiDimensionalVelocityCalculatormapped tohybridstrategy, consistent with the implementation.
656-659: LGTM!Observability table now includes all velocity event constants. This addresses the previous review comment about promoting updates into reference sections.
682-689: LGTM!Shipped section accurately documents the delivered components for
#969and#970, including strategies, calculators, and observability events with appropriate notes about defensive guards.tests/unit/engine/workflow/velocity_calculators/test_calendar.py (1)
1-125: LGTM!Comprehensive test coverage for
CalendarVelocityCalculator:
- Metadata properties verified
compute()tested with basic case, single-day, zero points, and secondary metricsrolling_average()tested with basic case, windowing, empty input, and duration-weighted averagingThe defensive
duration_days == 0branch in production code is intentionally unreachable with validatedVelocityRecord(which enforcesge=1), so omitting a test for it is acceptable.src/synthorg/engine/workflow/velocity_calculators/calendar.py (1)
1-117: LGTM!Well-structured implementation of
CalendarVelocityCalculator:
- Proper observability setup with
get_logger(__name__)and domain-specific event constant- Defensive handling for
duration_days == 0with debug logging- Duration-weighted rolling averages correctly computed
- Clean
__slots__usage for memory efficiency- Type hints and docstrings are complete
tests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.py (1)
1-202: LGTM!Excellent test coverage for
MultiDimensionalVelocityCalculator:
- Metadata properties verified
compute()tested for primary value and all three secondary metrics (pts_per_task,pts_per_day,completion_ratio)- Edge cases covered:
Nonetask count, zero task count, zero committed pointsrolling_average()tested with weighted secondary metrics, windowing, andNonetask count handling- Correctly uses
pytest.approxfor float comparisonstests/unit/engine/workflow/strategies/conftest.py (1)
11-90: Factory helpers look solid and flexible.The optional story-point overrides in both shared factories improve test expressiveness while preserving current defaults.
tests/unit/engine/workflow/strategies/test_calendar.py (1)
78-342: Great coverage for calendar strategy behavior.The suite exercises interval firing, transition boundaries, config validation, and lifecycle state reset comprehensively.
tests/unit/engine/workflow/strategies/test_hybrid.py (1)
542-621: Nice improvements in validation and lifecycle test rigor.The parameterized invalid percentage test and asserted pre-clear fire checks strengthen reliability.
| def validate_duration_days(value: object) -> None: | ||
| """Validate optional duration_days config value.""" | ||
| if value is None: | ||
| return | ||
| if not isinstance(value, int) or isinstance(value, bool): | ||
| msg = f"{KEY_DURATION_DAYS} must be a positive integer, got {value!r}" | ||
| raise ValueError(msg) # noqa: TRY004 | ||
| if value < MIN_DURATION_DAYS or value > MAX_DURATION_DAYS: | ||
| msg = ( | ||
| f"{KEY_DURATION_DAYS} must be between " | ||
| f"{MIN_DURATION_DAYS} and {MAX_DURATION_DAYS}, " | ||
| f"got {value!r}" | ||
| ) | ||
| raise ValueError(msg) | ||
|
|
||
|
|
||
| def validate_trigger(value: object) -> None: | ||
| """Validate optional trigger config value.""" | ||
| if value is not None and value not in VALID_TRIGGERS: | ||
| msg = f"Invalid trigger {value!r}. Valid triggers: {sorted(VALID_TRIGGERS)}" | ||
| raise ValueError(msg) | ||
|
|
||
|
|
||
| def validate_every_n(value: object) -> None: | ||
| """Validate optional every_n_completions config value.""" | ||
| if value is not None and ( | ||
| not isinstance(value, int) or isinstance(value, bool) or value < 1 | ||
| ): | ||
| msg = f"{KEY_EVERY_N} must be a positive integer, got {value!r}" | ||
| raise ValueError(msg) | ||
|
|
||
|
|
||
| def validate_sprint_percentage(value: object) -> None: | ||
| """Validate optional sprint_percentage config value.""" | ||
| if value is not None and ( | ||
| not isinstance(value, int | float) | ||
| or isinstance(value, bool) | ||
| or value <= 0 | ||
| or value > MAX_SPRINT_PCT | ||
| ): | ||
| msg = ( | ||
| f"{KEY_SPRINT_PERCENTAGE} must be between " | ||
| f"0 (exclusive) and {MAX_SPRINT_PCT} (inclusive)," | ||
| f" got {value!r}" | ||
| ) | ||
| raise ValueError(msg) | ||
|
|
||
|
|
||
| def validate_frequency(value: object) -> None: | ||
| """Validate optional frequency config value.""" | ||
| if value is not None and value not in VALID_FREQUENCIES: | ||
| msg = ( | ||
| f"Invalid frequency {value!r}. " | ||
| f"Valid frequencies: {sorted(VALID_FREQUENCIES)}" | ||
| ) | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Emit structured warning logs before ValueError in shared validators.
All five validator helpers currently raise without prior logging; since these are shared, this creates observability blind spots across strategies.
As per coding guidelines, "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/_helpers.py` around lines 215 - 270,
The validators (validate_duration_days, validate_trigger, validate_every_n,
validate_sprint_percentage, validate_frequency) raise ValueError without
logging; add a structured warning log immediately before each raise. Use the
module logger (create one with logging.getLogger(__name__) if not present) and
call logger.warning with context that includes the config key/identifier (e.g.,
KEY_DURATION_DAYS, KEY_EVERY_N, KEY_SPRINT_PERCENTAGE or the value/allowed set
like VALID_TRIGGERS/VALID_FREQUENCIES), the offending value, and a short reason,
then raise the same ValueError as before.
| if unknown: | ||
| msg = f"Unknown config keys: {sorted(unknown)}" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Add warning log before raising on unknown config keys.
This error path raises immediately with no structured warning context.
As per coding guidelines, "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/calendar.py` around lines 225 - 227,
Before raising ValueError for unknown config keys, log a warning with context:
call the module/class logger (or logging.getLogger(__name__) if no logger
exists) to emit a logger.warning that includes the same details as msg (e.g.,
sorted(unknown)) and any surrounding context (function name or config being
validated) so the unknown variable and msg are logged before the raise; then
raise the ValueError as before.
- Log SPRINT_AUTO_TRANSITION at INFO before returning IN_REVIEW in both CalendarStrategy and HybridStrategy (time_elapsed / task_threshold) - Only suppress sticky triggers (sprint_end, sprint_midpoint, sprint_percentage) within the calendar interval; every_n_completions resets naturally and is no longer suppressed - Add STICKY_TRIGGERS frozenset to _helpers.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/_helpers.py (1)
227-282:⚠️ Potential issue | 🟡 MinorAdd warning logs before raising
ValueErrorin validators.These five validators raise
ValueErrorwithout prior logging. Per coding guidelines and retrieved learnings, all error paths must log at WARNING or ERROR with context before raising.🛡️ Proposed fix for validate_duration_days (apply similar pattern to others)
def validate_duration_days(value: object) -> None: """Validate optional duration_days config value.""" if value is None: return if not isinstance(value, int) or isinstance(value, bool): msg = f"{KEY_DURATION_DAYS} must be a positive integer, got {value!r}" + logger.warning( + SPRINT_CEREMONY_SKIPPED, + reason="invalid_duration_days", + value=value, + ) raise ValueError(msg) # noqa: TRY004 if value < MIN_DURATION_DAYS or value > MAX_DURATION_DAYS: msg = ( f"{KEY_DURATION_DAYS} must be between " f"{MIN_DURATION_DAYS} and {MAX_DURATION_DAYS}, " f"got {value!r}" ) + logger.warning( + SPRINT_CEREMONY_SKIPPED, + reason="duration_days_out_of_range", + value=value, + min=MIN_DURATION_DAYS, + max=MAX_DURATION_DAYS, + ) raise ValueError(msg)As per coding guidelines: "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/_helpers.py` around lines 227 - 282, Add a warning log call just before each ValueError is raised in the validator functions so the error paths emit context at WARNING level; specifically, in validate_duration_days, validate_trigger, validate_every_n, validate_sprint_percentage, and validate_frequency call the module logger (e.g., logger.warning(...)) with the same message you plan to raise (include identifying context like KEY_DURATION_DAYS, KEY_EVERY_N, KEY_SPRINT_PERCENTAGE, VALID_TRIGGERS, VALID_FREQUENCIES and the offending value) immediately before each raise ValueError(...) so the message is recorded and then raise as before.
🤖 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/calendar.py`:
- Around line 233-246: Before raising ValueError in the config validation block,
add warning logs that include context: log the unknown keys (use the computed
unknown variable and _KNOWN_CONFIG_KEYS) with logger.warning just before raising
for unknown config keys, and log the invalid frequency value (freq) along with
VALID_FREQUENCIES and KEY_FREQUENCY just before raising for the frequency check;
keep the existing validate_duration_days call unchanged and use the module/class
logger (or a logger named near this code) so all error paths are logged at
WARNING level before the ValueError is raised.
In `@src/synthorg/engine/workflow/strategies/hybrid.py`:
- Around line 275-278: Before raising the ValueError for unknown config keys in
the block that computes unknown = set(config) - _KNOWN_CONFIG_KEYS, log a
warning containing the sorted unknown keys and any relevant context (e.g.,
strategy name or incoming config) using the module logger (or the existing
logger variable) at WARNING level, then raise the ValueError as before; update
the code path that constructs msg and raises ValueError to first call
logger.warning(...) with that message/context so the error is recorded before
the exception is thrown.
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/_helpers.py`:
- Around line 227-282: Add a warning log call just before each ValueError is
raised in the validator functions so the error paths emit context at WARNING
level; specifically, in validate_duration_days, validate_trigger,
validate_every_n, validate_sprint_percentage, and validate_frequency call the
module logger (e.g., logger.warning(...)) with the same message you plan to
raise (include identifying context like KEY_DURATION_DAYS, KEY_EVERY_N,
KEY_SPRINT_PERCENTAGE, VALID_TRIGGERS, VALID_FREQUENCIES and the offending
value) immediately before each raise ValueError(...) so the message is recorded
and then raise as before.
🪄 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: 438c02a7-fe95-4dd4-b4b1-b85dc13e8d10
📒 Files selected for processing (3)
src/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python 3.14+ required (PEP 649 native lazy annotations).
Nofrom __future__ import annotationsin Python code - Python 3.14 has PEP 649.
Use PEP 758 except syntax:except A, B:(no parentheses) - ruff enforces this on Python 3.14.
All public functions and classes require type hints and Google-style docstrings (enforced by ruff D rules).
Use mypy strict mode for type checking on all Python code.
Line length: 88 characters (enforced by ruff).
Files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Create new objects instead of mutating existing ones. For non-Pydantic internal collections, usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement.
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state that evolves. Never mix static config fields with mutable runtime fields.
Use Pydantic v2 withallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.
Use@computed_fieldfor derived values in Pydantic models instead of storing and validating redundant fields.
UseNotBlankStr(fromcore.types) for all identifier and name fields in Pydantic models, including optional and tuple variants, instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code. Prefer structured concurrency over barecreate_task.
Keep functions under 50 lines and files under 800 lines.
Handle errors explicitly, never silently swallow them.
Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.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(), orprint()in application code (exception: observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py may use stdlib logging).
Always useloggeras the variable name (not_logger, notlog) for logger instances.
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 synthorg.observability.events.<domain> import EVENT_CONSTANT.
Use structured logging withlogger.info(EVENT, key=value)- never use string formatting likelogger.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 level for object creation, internal flow, and entry/exit of key functions.
Pure data models, enums, and re-exports do NOT need logging.
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,test-provider,test-small-001.
Library reference indocs/api/is auto-generated via mkdocstrings and Griffe (AST-based) from Google-style docstrings.
Files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.py
src/synthorg/engine/workflow/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Workflow orchestration includes Kanban board, Agile sprints, WIP limits, velocity tracking, and pluggable scheduling strategies in
src/synthorg/engine/workflow/.
Files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.py
🧠 Learnings (16)
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to src/synthorg/engine/workflow/**/*.py : Workflow orchestration includes Kanban board, Agile sprints, WIP limits, velocity tracking, and pluggable scheduling strategies in `src/synthorg/engine/workflow/`.
Applied to files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level.
Applied to files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.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/calendar.pysrc/synthorg/engine/workflow/strategies/_helpers.pysrc/synthorg/engine/workflow/strategies/hybrid.py
📚 Learning: 2026-03-15T21:49:53.264Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:49:53.264Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
src/synthorg/engine/workflow/strategies/_helpers.py
📚 Learning: 2026-04-02T07:17:23.636Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:17:23.636Z
Learning: Fix everything valid in `/pre-pr-review` reviews - never skip valid issues, suggestions, and adjacent findings. No deferring, no 'out of scope' skipping.
Applied to files:
src/synthorg/engine/workflow/strategies/_helpers.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 : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
src/synthorg/engine/workflow/strategies/_helpers.py
🔇 Additional comments (15)
src/synthorg/engine/workflow/strategies/_helpers.py (4)
1-32: LGTM!Module setup follows coding guidelines: uses
get_logger(__name__), imports event constants from the correct domain module, and properly usesTYPE_CHECKINGfor type-only imports.
34-73: LGTM!Constants are well-organized with appropriate types. The
STICKY_TRIGGERSfrozenset correctly identifies triggers that need interval-based suppression in the hybrid strategy.
79-146: LGTM!Config resolution functions correctly handle None cases, perform appropriate type/range validation, and log warnings with context before fallback. The priority resolution (ceremony-level > strategy_config) is clear.
151-221: LGTM!Trigger evaluation is comprehensive. The bounds-check for
sprint_percentage(lines 205-212) correctly rejects values ≤0 or >100, and all invalid config paths log warnings with appropriate context before returningFalse.src/synthorg/engine/workflow/strategies/calendar.py (5)
1-42: LGTM!Module setup follows coding guidelines with proper logger initialization and event constant imports.
45-70: LGTM!Clean class design with
__slots__for memory efficiency and minimal internal state. The docstring clearly explains the strategy behavior.
72-120: LGTM!Ceremony firing logic is correct with proper logging at INFO for triggers and DEBUG for skips. The elapsed time comparison and state update are handled correctly.
122-156: LGTM!Sprint transition logic correctly guards for ACTIVE status, computes duration in seconds, and logs the
SPRINT_AUTO_TRANSITIONevent at INFO level with full context before returningIN_REVIEW.
158-210: LGTM!Lifecycle hooks correctly clear fire tracking state on sprint transitions. No-op implementations for task/budget/external events are appropriate for a time-based strategy.
src/synthorg/engine/workflow/strategies/hybrid.py (6)
1-60: LGTM!Module setup follows coding guidelines with proper logger initialization and event constant imports. The
_KNOWN_CONFIG_KEYScorrectly includes all valid keys for hybrid configuration.
62-94: LGTM!Class design is consistent with
CalendarStrategy. The docstring clearly explains the "first-wins" semantics and sticky trigger suppression behavior.
96-138: LGTM!First-wins logic correctly prioritizes calendar check, with task-driven check only when calendar doesn't fire. Logging at INFO includes both
calendar_firedandtask_firedflags for clear observability.
140-196: LGTM!Sprint transition logic correctly handles both calendar and task-driven legs with proper INFO logging for the
SPRINT_AUTO_TRANSITIONevent. Thetransition_thresholdNone check (lines 181-183) is appropriate defensive coding.
198-250: LGTM!Lifecycle hooks follow the same pattern as
CalendarStrategywith state clearing on sprint transitions and no-op implementations for event hooks.
288-335: LGTM!Internal helpers are well-implemented. The sticky trigger suppression (lines 328-333) correctly applies only to percentage-based triggers after the first fire, preserving first-wins semantics and allowing
every_n_completionsto fire naturally.
| unknown = set(config) - _KNOWN_CONFIG_KEYS | ||
| if unknown: | ||
| msg = f"Unknown config keys: {sorted(unknown)}" | ||
| raise ValueError(msg) | ||
|
|
||
| validate_duration_days(config.get(KEY_DURATION_DAYS)) | ||
|
|
||
| freq = config.get(KEY_FREQUENCY) | ||
| if freq is not None and freq not in VALID_FREQUENCIES: | ||
| msg = ( | ||
| f"Invalid frequency {freq!r}. " | ||
| f"Valid frequencies: {sorted(VALID_FREQUENCIES)}" | ||
| ) | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Add warning log before raising ValueError for unknown keys and invalid frequency.
The validation raises ValueError on two paths without prior logging.
🛡️ Proposed fix
unknown = set(config) - _KNOWN_CONFIG_KEYS
if unknown:
msg = f"Unknown config keys: {sorted(unknown)}"
+ logger.warning(
+ SPRINT_CEREMONY_SKIPPED,
+ reason="unknown_config_keys",
+ unknown_keys=sorted(unknown),
+ strategy="calendar",
+ )
raise ValueError(msg)
validate_duration_days(config.get(KEY_DURATION_DAYS))
freq = config.get(KEY_FREQUENCY)
if freq is not None and freq not in VALID_FREQUENCIES:
msg = (
f"Invalid frequency {freq!r}. "
f"Valid frequencies: {sorted(VALID_FREQUENCIES)}"
)
+ logger.warning(
+ SPRINT_CEREMONY_SKIPPED,
+ reason="invalid_frequency",
+ frequency=freq,
+ strategy="calendar",
+ )
raise ValueError(msg)As per coding guidelines: "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/calendar.py` around lines 233 - 246,
Before raising ValueError in the config validation block, add warning logs that
include context: log the unknown keys (use the computed unknown variable and
_KNOWN_CONFIG_KEYS) with logger.warning just before raising for unknown config
keys, and log the invalid frequency value (freq) along with VALID_FREQUENCIES
and KEY_FREQUENCY just before raising for the frequency check; keep the existing
validate_duration_days call unchanged and use the module/class logger (or a
logger named near this code) so all error paths are logged at WARNING level
before the ValueError is raised.
| unknown = set(config) - _KNOWN_CONFIG_KEYS | ||
| if unknown: | ||
| msg = f"Unknown config keys: {sorted(unknown)}" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Add warning log before raising ValueError for unknown keys.
Same issue as in CalendarStrategy: raises without prior logging.
🛡️ Proposed fix
unknown = set(config) - _KNOWN_CONFIG_KEYS
if unknown:
msg = f"Unknown config keys: {sorted(unknown)}"
+ logger.warning(
+ SPRINT_CEREMONY_SKIPPED,
+ reason="unknown_config_keys",
+ unknown_keys=sorted(unknown),
+ strategy="hybrid",
+ )
raise ValueError(msg)As per coding guidelines: "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/hybrid.py` around lines 275 - 278,
Before raising the ValueError for unknown config keys in the block that computes
unknown = set(config) - _KNOWN_CONFIG_KEYS, log a warning containing the sorted
unknown keys and any relevant context (e.g., strategy name or incoming config)
using the module logger (or the existing logger variable) at WARNING level, then
raise the ValueError as before; update the code path that constructs msg and
raises ValueError to first call logger.warning(...) with that message/context so
the error is recorded before the exception is thrown.
🤖 I have created a release *beep* *boop* --- ## [0.5.6](v0.5.5...v0.5.6) (2026-04-02) ### Features * calendar + hybrid ceremony scheduling strategies ([#985](#985)) ([59a9b84](59a9b84)), closes [#969](#969) [#970](#970) * landing page interactive components ([#984](#984)) ([49868cb](49868cb)) * log aggregation and shipping (syslog, HTTP, compression) ([#964](#964)) ([84be9f8](84be9f8)) * restructure builtin templates into inheritance tree ([#982](#982)) ([3794c12](3794c12)) * sprint ceremony runtime scheduler with pluggable strategies ([#983](#983)) ([43564a9](43564a9)) ### Maintenance * add no-bash-file-writes rule to CLAUDE.md ([#968](#968)) ([a854dcc](a854dcc)) * bump web dependencies (lodash, eslint-react v4, storybook, playwright, esbuild, codemirror) ([#987](#987)) ([c344dfb](c344dfb)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Implement two ceremony scheduling strategies for the pluggable system from #961:
MeetingFrequencyintervals. Auto-transitions ACTIVE to IN_REVIEW at theduration_daysboundary. Default velocity:pts/day.pts/sprintwith multi-dimensional secondaries.Both strategies use minimal internal state (
_last_fire_elapseddict) to track per-ceremony fire times, preventing double-firing within the same interval. State is cleared on sprint lifecycle transitions.Changes
New files
src/synthorg/engine/workflow/strategies/calendar.py-- CalendarStrategysrc/synthorg/engine/workflow/strategies/hybrid.py-- HybridStrategysrc/synthorg/engine/workflow/velocity_calculators/calendar.py-- CalendarVelocityCalculator (pts/day)src/synthorg/engine/workflow/velocity_calculators/multi_dimensional.py-- MultiDimensionalVelocityCalculator (pts/sprint+pts_per_task,pts_per_day,completion_ratio)tests/unit/engine/workflow/strategies/test_calendar.py(28 tests)tests/unit/engine/workflow/strategies/test_hybrid.py(35 tests)tests/unit/engine/workflow/velocity_calculators/test_calendar.py(11 tests)tests/unit/engine/workflow/velocity_calculators/test_multi_dimensional.py(17 tests)Modified files
src/synthorg/engine/workflow/strategies/__init__.py-- export CalendarStrategy, HybridStrategysrc/synthorg/engine/workflow/velocity_calculators/__init__.py-- export CalendarVelocityCalculator, MultiDimensionalVelocityCalculatorsrc/synthorg/observability/events/workflow.py-- add VELOCITY_CALENDAR_NO_DURATION, VELOCITY_MULTI_NO_TASK_COUNTdocs/design/ceremony-scheduling.md-- remove (planned) labels, move feat: calendar ceremony scheduling strategy #969/feat: hybrid (first-wins) ceremony scheduling strategy #970 to shipped sectionTest plan
Review coverage
Pre-reviewed by 4 agents (code-reviewer, conventions-enforcer, issue-resolution-verifier, docs-consistency). 5 findings addressed:
_evaluate_task_triggerHybridStrategy.validate_strategy_configexceeded 50-line limit -- extracted validatorsMultiDimensionalVelocityCalculator.rolling_averageexceeded 50-line limit -- extracted helperCloses #969
Closes #970