Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (13)📚 Learning: 2026-03-15T18:28:13.207ZApplied to files:
📚 Learning: 2026-03-20T21:44:04.528ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-03-31T14:17:24.182ZApplied to files:
📚 Learning: 2026-03-16T07:22:28.134ZApplied to files:
📚 Learning: 2026-04-03T07:34:51.012ZApplied to files:
📚 Learning: 2026-03-15T18:28:13.207ZApplied to files:
📚 Learning: 2026-04-03T07:34:51.012ZApplied to files:
📚 Learning: 2026-03-14T15:43:05.601ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-03-16T07:22:28.134ZApplied to files:
📚 Learning: 2026-03-31T14:17:24.182ZApplied to files:
📚 Learning: 2026-03-17T06:43:14.114ZApplied to files:
🔇 Additional comments (16)
WalkthroughAdds a new MilestoneDrivenStrategy (implemented and exported at package level); introduces a per-department 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Code Review
This pull request introduces the MilestoneDrivenStrategy for ceremony scheduling, which triggers ceremonies based on the completion of tasks associated with specific milestones. The changes include updates to the Department model and template schema to support per-department ceremony policy overrides, as well as default strategy assignments for various builtin templates. Review feedback identifies a security vulnerability caused by the removal of origin verification in mockServiceWorker.js and suggests an improvement to the milestone task assignment logic to ensure idempotency when enforcing task limits.
| return | ||
| } | ||
|
|
||
| const clientId = Reflect.get(event.source || {}, 'id') |
There was a problem hiding this comment.
Removing the same-origin verification for incoming messages introduces a security vulnerability (CWE-020/CWE-940). This check ensures that the Service Worker only processes messages from its own origin, preventing malicious cross-origin scripts from interacting with the worker. Additionally, the file header explicitly states that this file should not be modified. Unless there is a documented requirement for cross-origin messaging that is restricted to non-production environments, this check should be restored.
| const clientId = Reflect.get(event.source || {}, 'id') | |
| // Verify same-origin (CWE-020/CWE-940) | |
| if (event.origin && event.origin !== self.location.origin) { | |
| return | |
| } | |
| const clientId = Reflect.get(event.source || {}, 'id') |
| if milestone not in self._milestone_tasks: | ||
| self._milestone_tasks[milestone] = set() | ||
|
|
||
| if len(self._milestone_tasks[milestone]) >= _MAX_TASKS_PER_MILESTONE: | ||
| logger.warning( | ||
| SPRINT_CEREMONY_SKIPPED, | ||
| reason="too_many_tasks_in_milestone", | ||
| milestone=milestone, | ||
| limit=_MAX_TASKS_PER_MILESTONE, | ||
| strategy="milestone_driven", | ||
| ) | ||
| return | ||
|
|
||
| self._milestone_tasks[milestone].add(task_id) |
There was a problem hiding this comment.
The task limit check in _handle_assign prevents idempotent operations when a milestone is at its capacity. If a milestone_assign event is received for a task that is already registered to the milestone, it will trigger a warning and return early without performing the addition. This can lead to misleading logs and unnecessary early exits for redundant events. It is better to check if the task is already present before enforcing the limit.
| if milestone not in self._milestone_tasks: | |
| self._milestone_tasks[milestone] = set() | |
| if len(self._milestone_tasks[milestone]) >= _MAX_TASKS_PER_MILESTONE: | |
| logger.warning( | |
| SPRINT_CEREMONY_SKIPPED, | |
| reason="too_many_tasks_in_milestone", | |
| milestone=milestone, | |
| limit=_MAX_TASKS_PER_MILESTONE, | |
| strategy="milestone_driven", | |
| ) | |
| return | |
| self._milestone_tasks[milestone].add(task_id) | |
| tasks = self._milestone_tasks.setdefault(milestone, set()) | |
| if task_id not in tasks and len(tasks) >= _MAX_TASKS_PER_MILESTONE: | |
| logger.warning( | |
| SPRINT_CEREMONY_SKIPPED, | |
| reason="too_many_tasks_in_milestone", | |
| milestone=milestone, | |
| limit=_MAX_TASKS_PER_MILESTONE, | |
| strategy="milestone_driven", | |
| ) | |
| return | |
| tasks.add(task_id) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1019 +/- ##
==========================================
+ Coverage 91.77% 91.82% +0.04%
==========================================
Files 669 670 +1
Lines 36739 36957 +218
Branches 3625 3666 +41
==========================================
+ Hits 33719 33937 +218
Misses 2389 2389
Partials 631 631 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds milestone-driven ceremony scheduling, sets default ceremony strategies in builtin templates, and introduces department-level ceremony policy overrides to support 3-level ceremony policy resolution across the workflow system.
Changes:
- Introduce
MilestoneDrivenStrategywith milestone assignment via external events plus new observability events. - Add
ceremony_policydefaults to all builtin templates and validate via new integration tests. - Extend template + runtime department configs to carry an optional
ceremony_policyoverride (passthrough validated during rendering).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/public/mockServiceWorker.js | Modifies MSW service worker message handling (generated artifact). |
| src/synthorg/engine/workflow/strategies/milestone_driven.py | New milestone-driven ceremony scheduling strategy implementation. |
| src/synthorg/engine/workflow/strategies/init.py | Exports MilestoneDrivenStrategy from strategies package. |
| src/synthorg/observability/events/workflow.py | Adds milestone-driven strategy event constants. |
| src/synthorg/templates/schema.py | Extends TemplateDepartmentConfig to accept optional ceremony_policy. |
| src/synthorg/templates/_render_helpers.py | Validates/copies department ceremony_policy during rendering. |
| src/synthorg/core/company.py | Extends runtime Department model with optional ceremony_policy. |
| src/synthorg/templates/builtins/solo_founder.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/startup.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/dev_shop.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/product_team.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/agency.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/full_company.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/research_lab.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/consultancy.yaml | Sets builtin default sprint ceremony strategy. |
| src/synthorg/templates/builtins/data_team.yaml | Sets builtin default sprint ceremony strategy. |
| tests/unit/engine/workflow/strategies/test_milestone_driven.py | Adds unit tests covering milestone-driven strategy behavior and validation. |
| tests/unit/templates/test_workflow_config_integration.py | Adds tests for builtin template ceremony strategy defaults and department policy passthrough. |
| tests/unit/templates/test_schema.py | Adds schema tests for department ceremony_policy field behavior. |
| tests/unit/core/test_company.py | Adds tests for runtime Department.ceremony_policy field behavior. |
| docs/design/ceremony-scheduling.md | Updates design doc with shipped milestone strategy + overrides and new events. |
Comments suppressed due to low confidence (1)
web/public/mockServiceWorker.js:27
mockServiceWorker.jsis a generated MSW artifact (header explicitly says “Please do NOT modify this file”). This PR changes the message handler and removes the same-origin guard, which is both a security regression (defense-in-depth against spoofed postMessage senders) and likely to be overwritten on the nextmsw init/upgrade. Prefer regenerating the worker or addressing the underlying lint/security concern without editing this generated file (or re-add a safe origin/client verification before processing messages).
addEventListener('message', async function (event) {
const clientId = Reflect.get(event.source || {}, 'id')
if (!clientId || !self.clients) {
return
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| default_factory=DepartmentPolicies, | ||
| description="Department-level operational policies", | ||
| ) | ||
| ceremony_policy: dict[str, Any] | None = Field( | ||
| default=None, | ||
| description="Per-department ceremony policy override", | ||
| ) |
There was a problem hiding this comment.
Department is declared with model_config = ConfigDict(frozen=True, ...), but ceremony_policy is stored as a mutable dict. That undermines the immutability guarantee of a frozen config object (the dict can still be mutated in-place after model creation). Consider storing this as an immutable mapping (e.g., wrap with MappingProxyType in a validator) or as a dedicated (frozen) Pydantic model to preserve the frozen contract while still avoiding circular imports.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/milestone_driven.py`:
- Around line 303-320: The handler accepts milestone assignments for any task id
and mutates _milestone_tasks even if the task is not part of the active sprint;
update on_external_event (or _handle_assign/_handle_unassign) to validate the
incoming payload["task_id"] against the current Sprint.task_ids (not
Sprint.completed_task_ids) and ignore or log-and-reject
assignments/unassignments for task_ids not present in sprint.task_ids; apply the
same check for EVENT_MILESTONE_UNASSIGN and ensure no mutation of
_milestone_tasks occurs for out-of-scope task_ids.
- Around line 408-413: Change the two milestone membership logs from DEBUG to
INFO and add the same strategy field used elsewhere so state transitions are
visible; specifically, in milestone_driven.py update the logger.debug call that
emits SPRINT_CEREMONY_MILESTONE_ASSIGNED (and the analogous
SPRINT_CEREMONY_MILESTONE_UNASSIGNED later) to logger.info and include
strategy=self.name (or the exact strategy variable used in the class/method)
alongside task_id, milestone, and task_count so these transitions conform to the
"log state transitions at INFO" guideline.
- Around line 92-153: The method should_fire_ceremony is too long and mixes
iteration, per-milestone evaluation and firing logic—extract the per-milestone
check into a small helper (e.g. _is_milestone_complete or
_evaluate_milestone_for_ceremony) that takes (milestone_name, ceremony,
completed_set) and returns a bool/side-effect flag, and similarly split
_validate_milestones into a single-entry validator (e.g.
_validate_single_milestone) that validates one milestone mapping; then refactor
should_fire_ceremony to loop over self._milestones and call the new
per-milestone helpers so both functions stay under 50 lines while preserving
behavior around self._milestone_tasks, self._fired_milestones and the logger
calls (SPRINT_CEREMONY_MILESTONE_NOT_READY, SPRINT_CEREMONY_MILESTONE_COMPLETED,
SPRINT_CEREMONY_TRIGGERED, SPRINT_CEREMONY_SKIPPED).
In `@tests/unit/engine/workflow/strategies/test_milestone_driven.py`:
- Around line 311-320: Tests call
MilestoneDrivenStrategy.should_transition_sprint() without initializing its
cache; call MilestoneDrivenStrategy.on_sprint_activated(...) first to populate
_transition_milestone before asserting behavior. Specifically, in the
non-active-sprint test (using make_sprint, _make_sprint_config, make_context)
invoke strategy.on_sprint_activated(sprint, config, ctx) prior to
strategy.should_transition_sprint(...), and for the non-ACTIVE case ensure the
sprint data includes a completed transition milestone (e.g., mark the "alpha"
milestone as completed on the sprint) so the test fails due to status guard
rather than an uninitialized _transition_milestone; apply the same
initialization change to the other test (lines ~338-349) that exercises
should_transition_sprint().
🪄 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: de606638-4a30-4678-a8a8-661bd7b639df
📒 Files selected for processing (21)
docs/design/ceremony-scheduling.mdsrc/synthorg/core/company.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/milestone_driven.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/templates/builtins/agency.yamlsrc/synthorg/templates/builtins/consultancy.yamlsrc/synthorg/templates/builtins/data_team.yamlsrc/synthorg/templates/builtins/dev_shop.yamlsrc/synthorg/templates/builtins/full_company.yamlsrc/synthorg/templates/builtins/product_team.yamlsrc/synthorg/templates/builtins/research_lab.yamlsrc/synthorg/templates/builtins/solo_founder.yamlsrc/synthorg/templates/builtins/startup.yamlsrc/synthorg/templates/schema.pytests/unit/core/test_company.pytests/unit/engine/workflow/strategies/test_milestone_driven.pytests/unit/templates/test_schema.pytests/unit/templates/test_workflow_config_integration.pyweb/public/mockServiceWorker.js
💤 Files with no reviewable changes (1)
- web/public/mockServiceWorker.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Agent
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dashboard Test
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Files:
tests/unit/core/test_company.pytests/unit/templates/test_schema.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/templates/schema.pysrc/synthorg/core/company.pytests/unit/templates/test_workflow_config_integration.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/test_milestone_driven.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All Python test files must use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer@pytest.mark.parametrizefor testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given+@settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/core/test_company.pytests/unit/templates/test_schema.pytests/unit/templates/test_workflow_config_integration.pytests/unit/engine/workflow/strategies/test_milestone_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every Python module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging with kwargs in Python: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/templates/schema.pysrc/synthorg/core/company.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
src/synthorg/engine/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedError in Python signals that all retries failed; the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation files in docs/ are Markdown, built with Zensical, configured in mkdocs.yml; design spec in docs/design/ (12 pages), Architecture in docs/architecture/, Roadmap in docs/roadmap/
Files:
docs/design/ceremony-scheduling.md
🧠 Learnings (15)
📚 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/strategies/__init__.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields
Applied to files:
src/synthorg/templates/_render_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 Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/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-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-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-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-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-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/workflow.py
🔇 Additional comments (17)
src/synthorg/templates/builtins/consultancy.yaml (1)
110-112: LGTM!The
ceremony_policyconfiguration withstrategy: "calendar"is valid and matches theCeremonyStrategyType.CALENDARenum value. This is appropriate for a client-facing consultancy where time-based scheduling aligns with client engagement cadences.src/synthorg/templates/builtins/research_lab.yaml (1)
125-127: LGTM!The
ceremony_policywithstrategy: "throughput_adaptive"is valid and appropriate for a research lab where ceremonies should adapt to varying research throughput rather than rigid schedules.src/synthorg/templates/builtins/solo_founder.yaml (1)
74-76: LGTM!The
ceremony_policywithstrategy: "task_driven"is valid and well-suited for a solo builder setup where ceremonies should trigger based on task completion rather than calendar schedules.src/synthorg/templates/builtins/dev_shop.yaml (1)
173-174: LGTM!The
ceremony_policywithstrategy: "hybrid"is valid and appropriate for an engineering squad that balances calendar-based sprints with task-driven milestones.src/synthorg/templates/builtins/data_team.yaml (1)
105-107: LGTM!The
ceremony_policywithstrategy: "task_driven"is valid. This appropriately overrides the parentresearch_labtemplate'sthroughput_adaptivestrategy with a task-focused approach better suited for data pipeline work.src/synthorg/templates/builtins/startup.yaml (1)
117-118: LGTM!The
ceremony_policywithstrategy: "task_driven"is valid and correctly positioned within the existing sprint configuration block.src/synthorg/templates/schema.py (1)
230-233: LGTM!The
ceremony_policyfield follows the same pattern as the existingpoliciesfield (rawdict[str, Any] | None), which is appropriate for avoiding circular imports with the ceremony strategy types. The deferred validation approach is consistent with the schema's role as a template-level structure definition.tests/unit/core/test_company.py (1)
234-260: LGTM!The three new tests provide good coverage of the
ceremony_policyfield behavior:
- Default
Nonevalue- Simple strategy dict
- Complex multi-field dict with nested values
The tests follow existing patterns in the file and use valid strategy names from the
CeremonyStrategyTypeenum.src/synthorg/templates/builtins/full_company.yaml (1)
376-377: Default strategy assignment looks correct.
full_companynow sets a sprintceremony_policydefault tohybrid, matching the intended built-in strategy mapping.src/synthorg/templates/builtins/agency.yaml (1)
189-191: Strategy default is correctly configured for Agency.The added sprint
ceremony_policywithevent_drivenis consistent with the intended template defaults.tests/unit/templates/test_schema.py (1)
277-299: Good schema coverage forceremony_policy.These tests validate defaulting, basic dict acceptance, and nested dict passthrough for department overrides.
src/synthorg/engine/workflow/strategies/__init__.py (1)
23-25: Export wiring forMilestoneDrivenStrategyis clean.Import +
__all__update correctly exposes the new strategy at package level.Also applies to: 39-40
src/synthorg/templates/builtins/product_team.yaml (1)
180-181:product_teamdefault policy is set correctly.The sprint-level
ceremony_policy.strategyupdate tohybridaligns with the expected defaults.src/synthorg/templates/_render_helpers.py (1)
127-139: Validation and defensive copy pattern is solid.
ceremony_policynow gets explicit type-checking, structured error logging, anddeepcopypassthrough consistent with existing optional-field handling.src/synthorg/core/company.py (1)
372-376: Department-level override support is modeled cleanly.The new
ceremony_policyfield and doc updates clearly encode inheritance semantics while keeping engine-type coupling out ofcore.Also applies to: 412-415
docs/design/ceremony-scheduling.md (2)
674-678: Milestone observability events are documented clearly.The new event rows make milestone lifecycle instrumentation explicit and easy to trace.
727-735: Roadmap update is clear and actionable.The shipped-scope section and integration note provide good status clarity for milestone strategy, template defaults, and department overrides.
| async def on_external_event( | ||
| self, | ||
| sprint: Sprint, # noqa: ARG002 | ||
| event_name: str, | ||
| payload: Mapping[str, Any], | ||
| ) -> None: | ||
| """Handle milestone_assign / milestone_unassign events. | ||
|
|
||
| Expected payload keys: ``task_id`` (str), ``milestone`` (str). | ||
|
|
||
| Args: | ||
| sprint: Current sprint state. | ||
| event_name: Name of the external event. | ||
| payload: Event payload data. | ||
| """ | ||
| if event_name == EVENT_MILESTONE_ASSIGN: | ||
| self._handle_assign(payload) | ||
| elif event_name == EVENT_MILESTONE_UNASSIGN: |
There was a problem hiding this comment.
Reject milestone assignments for tasks outside the active sprint.
on_external_event() drops sprint, and _handle_assign() accepts any task_id from the external stream. Since Sprint.completed_task_ids is only a subset of task_ids (src/synthorg/engine/workflow/sprint_lifecycle.py:172-175), a stray/global assignment can never become complete and will block that milestone for the rest of the sprint. Filter against sprint.task_ids before mutating _milestone_tasks, or track milestone membership outside sprint-local state.
💡 Minimal fix
async def on_external_event(
self,
- sprint: Sprint, # noqa: ARG002
+ sprint: Sprint,
event_name: str,
payload: Mapping[str, Any],
) -> None:
@@
"""
if event_name == EVENT_MILESTONE_ASSIGN:
- self._handle_assign(payload)
+ self._handle_assign(set(sprint.task_ids), payload)
elif event_name == EVENT_MILESTONE_UNASSIGN:
self._handle_unassign(payload)
@@
- def _handle_assign(self, payload: Mapping[str, Any]) -> None:
+ def _handle_assign(
+ self,
+ active_task_ids: set[str],
+ payload: Mapping[str, Any],
+ ) -> None:
"""Register a task under a milestone."""
task_id = payload.get("task_id")
milestone = payload.get("milestone")
@@
milestone = milestone.strip()
task_id = task_id.strip()
+
+ if task_id not in active_task_ids:
+ logger.debug(
+ SPRINT_CEREMONY_SKIPPED,
+ reason="task_not_in_active_sprint",
+ task_id=task_id,
+ milestone=milestone,
+ strategy="milestone_driven",
+ )
+ returnAlso applies to: 361-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workflow/strategies/milestone_driven.py` around lines 303
- 320, The handler accepts milestone assignments for any task id and mutates
_milestone_tasks even if the task is not part of the active sprint; update
on_external_event (or _handle_assign/_handle_unassign) to validate the incoming
payload["task_id"] against the current Sprint.task_ids (not
Sprint.completed_task_ids) and ignore or log-and-reject
assignments/unassignments for task_ids not present in sprint.task_ids; apply the
same check for EVENT_MILESTONE_UNASSIGN and ensure no mutation of
_milestone_tasks occurs for out-of-scope task_ids.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/synthorg/engine/workflow/strategies/milestone_driven.py (1)
334-352:⚠️ Potential issue | 🟠 MajorScope milestone events to the active sprint.
Line 336 drops
sprint, somilestone_assign/milestone_unassigncan mutate_milestone_tasksfor arbitrary task IDs. Because readiness is later computed only fromsprint.completed_task_ids, one stray out-of-scope assignment can block the mapped ceremony andtransition_milestonefor the rest of the sprint. Validate againstsprint.task_idsbefore mutating state, and apply the same guard to unassigns.🔧 Minimal fix
async def on_external_event( self, - sprint: Sprint, # noqa: ARG002 + sprint: Sprint, event_name: str, payload: Mapping[str, Any], ) -> None: + active_task_ids = set(sprint.task_ids) if event_name == EVENT_MILESTONE_ASSIGN: - self._handle_assign(payload) + self._handle_assign(active_task_ids, payload) elif event_name == EVENT_MILESTONE_UNASSIGN: - self._handle_unassign(payload) + self._handle_unassign(active_task_ids, payload) - def _handle_assign(self, payload: Mapping[str, Any]) -> None: + def _handle_assign( + self, + active_task_ids: set[str], + payload: Mapping[str, Any], + ) -> None: ... + if task_id not in active_task_ids: + logger.debug( + SPRINT_CEREMONY_SKIPPED, + reason="task_not_in_active_sprint", + task_id=task_id, + milestone=milestone, + strategy="milestone_driven", + ) + return ... - def _handle_unassign(self, payload: Mapping[str, Any]) -> None: + def _handle_unassign( + self, + active_task_ids: set[str], + payload: Mapping[str, Any], + ) -> None: ... + if task_id not in active_task_ids: + returnAs per coding guidelines, "Validate at system boundaries in Python (user input, external APIs, config files)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workflow/strategies/milestone_driven.py` around lines 334 - 352, The on_external_event handlers (_handle_assign and _handle_unassign) currently accept payloads for any task; before mutating self._milestone_tasks validate the payload's "task_id" exists in the current sprint (check membership in sprint.task_ids) and only then call the corresponding handler; apply the same guard for EVENT_MILESTONE_UNASSIGN. Locate on_external_event and the helper methods (_handle_assign, _handle_unassign) and add a short pre-check that extracts task_id from payload, confirms it is a string and in sprint.task_ids, and returns/ignores the event if not in scope so out-of-sprint tasks cannot alter milestone state that later relies on sprint.completed_task_ids.
🤖 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/milestone_driven.py`:
- Around line 394-447: The _handle_assign method is too long and mixes payload
parsing, validation, cap enforcement, mutation and logging; split it into a
small payload normalizer and a mutation helper: implement a helper named
_normalize_assign_payload(self, payload) that returns (task_id, milestone) or
(None, None) after trimming and type checks, and implement a helper named
_add_task_to_milestone(self, task_id, milestone) that encapsulates the existence
check, _MAX_TASKS_PER_MILESTONE cap logic, self._milestone_tasks mutation, and
logging (SPRINT_CEREMONY_SKIPPED warnings and SPRINT_CEREMONY_MILESTONE_ASSIGNED
info); then refactor _handle_assign to just call _normalize_assign_payload and,
if valid, call _add_task_to_milestone and return early—this keeps _handle_assign
under 50 lines and makes adding the active-sprint guard easier.
---
Duplicate comments:
In `@src/synthorg/engine/workflow/strategies/milestone_driven.py`:
- Around line 334-352: The on_external_event handlers (_handle_assign and
_handle_unassign) currently accept payloads for any task; before mutating
self._milestone_tasks validate the payload's "task_id" exists in the current
sprint (check membership in sprint.task_ids) and only then call the
corresponding handler; apply the same guard for EVENT_MILESTONE_UNASSIGN. Locate
on_external_event and the helper methods (_handle_assign, _handle_unassign) and
add a short pre-check that extracts task_id from payload, confirms it is a
string and in sprint.task_ids, and returns/ignores the event if not in scope so
out-of-sprint tasks cannot alter milestone state that later relies on
sprint.completed_task_ids.
🪄 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: 69df0ce1-bb5b-4f19-8130-1c78d72f4f1b
📒 Files selected for processing (6)
src/synthorg/core/company.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/milestone_driven.pytests/unit/engine/workflow/strategies/test_milestone_driven.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/templates/test_renderer.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Files:
src/synthorg/engine/workflow/__init__.pytests/unit/templates/test_renderer.pysrc/synthorg/core/company.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/engine/workflow/strategies/test_milestone_driven.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every Python module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging with kwargs in Python: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/core/company.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
src/synthorg/engine/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
RetryExhaustedError in Python signals that all retries failed; the engine layer catches this to trigger fallback chains
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All Python test files must use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer@pytest.mark.parametrizefor testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given+@settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/templates/test_renderer.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/engine/workflow/strategies/test_milestone_driven.py
🧠 Learnings (27)
📚 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/core/company.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to **/*.py : Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-16T23:05:29.577Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T23:05:29.577Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/core/company.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 frozen Pydantic models for config/identity; use separate mutable-via-copy models with `model_copy(update=...)` for runtime state that evolves
Applied to files:
src/synthorg/core/company.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 **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/core/company.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/core/company.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/core/company.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/core/company.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 Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Functions must be less than 50 lines; files must be less than 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Python functions must be under 50 lines; files must be under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to src/synthorg/**/*.py : All state transitions in Python must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
🔇 Additional comments (1)
tests/unit/templates/test_renderer.py (1)
548-556: LGTM!The test correctly validates that
build_departmentsraisesTemplateRenderErrorwhenceremony_policyis a non-mapping type. The test follows the established pattern of sibling tests in this class, has appropriate assertions matching the implementation's error message, and maintains proper test structure with docstring and type hints.
5d0d8ee to
61ecaf8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/templates/_render_helpers.py`:
- Around line 127-139: The code is double deep-copying ceremony_policy; keep the
type check but remove the internal copy and assign the original mapping to
dept_dict so the deep-copy in the Department model validation handles isolation.
Concretely, after validating ceremony_policy is a dict, replace
dept_dict["ceremony_policy"] = copy.deepcopy(ceremony_policy) with
dept_dict["ceremony_policy"] = ceremony_policy (leave the validation/logging and
the Department model's deepcopy intact).
In `@tests/unit/core/test_company.py`:
- Around line 234-260: Add a mutation-safety unit test for the
Department.ceremony_policy field: construct a source dict, pass it into
Department(...) (e.g., in a new test function like
test_ceremony_policy_is_defensively_copied), then mutate the original dict and
assert that dept.ceremony_policy did not change (and still has the original
values); reference Department and its ceremony_policy attribute to ensure the
model makes a defensive copy rather than retaining a live reference.
🪄 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: bc6fb35c-7bc3-4741-a12e-a55000745a33
📒 Files selected for processing (24)
docs/design/ceremony-scheduling.mdsrc/synthorg/core/company.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/milestone_driven.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/templates/builtins/agency.yamlsrc/synthorg/templates/builtins/consultancy.yamlsrc/synthorg/templates/builtins/data_team.yamlsrc/synthorg/templates/builtins/dev_shop.yamlsrc/synthorg/templates/builtins/full_company.yamlsrc/synthorg/templates/builtins/product_team.yamlsrc/synthorg/templates/builtins/research_lab.yamlsrc/synthorg/templates/builtins/solo_founder.yamlsrc/synthorg/templates/builtins/startup.yamlsrc/synthorg/templates/schema.pytests/unit/core/test_company.pytests/unit/engine/workflow/strategies/test_milestone_driven.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/templates/test_renderer.pytests/unit/templates/test_schema.pytests/unit/templates/test_workflow_config_integration.pyweb/public/mockServiceWorker.js
💤 Files with no reviewable changes (1)
- web/public/mockServiceWorker.js
📜 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). (11)
- GitHub Check: Deploy Preview
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dashboard Test
- GitHub Check: Dashboard Storybook Build
- GitHub Check: Dashboard Build
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import 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 must have type hints and pass mypy strict mode
Docstrings must use Google style and are required on all public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Always useallow_inf_nan=FalseinConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Maximum line length: 88 characters (enforced by ruff)
Functions must be less than 50 lines, files must be less than 800 lines
Files:
src/synthorg/templates/schema.pysrc/synthorg/engine/workflow/strategies/__init__.pytests/unit/core/test_company.pysrc/synthorg/engine/workflow/__init__.pytests/unit/templates/test_schema.pytests/unit/templates/test_renderer.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.pysrc/synthorg/observability/events/workflow.pytests/unit/templates/test_workflow_config_integration.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/engine/workflow/strategies/test_milestone_driven.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr))
Logger variable name must always belogger(not_logger, notlog)
Always use event constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api). Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured kwargs for logging:logger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, entry/exit of key functions
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/small.
In Pydantic models, useallow_inf_nan=Falsein allConfigDictdeclarations
Pure data models, enums, and re-exports do NOT need logging
Files:
src/synthorg/templates/schema.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All test files must use markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests: useasyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Prefer@pytest.mark.parametrizefor testing similar cases
For property-based testing in Python, use Hypothesis (@given+@settings). Profiles:ci(50 examples, default) anddev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n 8 -k properties
Never skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/core/test_company.pytests/unit/templates/test_schema.pytests/unit/templates/test_renderer.pytests/unit/templates/test_workflow_config_integration.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/engine/workflow/strategies/test_milestone_driven.py
🧠 Learnings (40)
📚 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/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 : 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/core/company.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.py : For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/core/company.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/core/company.py
📚 Learning: 2026-03-16T23:05:29.577Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T23:05:29.577Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields
Applied to files:
src/synthorg/core/company.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/core/company.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.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/core/company.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to **/*.py : Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/core/company.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 frozen Pydantic models for config/identity; use separate mutable-via-copy models with `model_copy(update=...)` for runtime state that evolves
Applied to files:
src/synthorg/core/company.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 **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/core/company.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 Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Always use `allow_inf_nan=False` in `ConfigDict` declarations to reject `NaN`/`Inf` in numeric fields at validation time.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-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-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-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-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to src/synthorg/**/*.py : Always use event constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to 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_milestone_driven.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/engine/workflow/strategies/test_milestone_driven.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Functions must be less than 50 lines; files must be less than 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.py : Functions must be less than 50 lines, files must be less than 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
🔇 Additional comments (32)
src/synthorg/templates/builtins/consultancy.yaml (1)
110-112: Correct template default strategy placement.
ceremony_policyis correctly nested underworkflow_config.sprint, and"calendar"matches the intended consultancy default.src/synthorg/templates/builtins/data_team.yaml (1)
105-107: Nesting and default strategy are correct.The new sprint ceremony policy is placed at the correct
workflow_config.sprintlevel, and"task_driven"aligns with the template default mapping.src/synthorg/templates/builtins/dev_shop.yaml (1)
173-174: Good incremental extension of sprint config.Adding
"hybrid"underworkflow_config.sprint.ceremony_policyis consistent and non-breaking with the existing sprint ceremony definitions.src/synthorg/templates/builtins/research_lab.yaml (1)
125-127: Looks correct for research_lab defaults.The added ceremony policy is correctly scoped under sprint config, and
"throughput_adaptive"matches the expected strategy default.src/synthorg/templates/builtins/solo_founder.yaml (1)
74-76: Default ceremony strategy is correctly added.
task_drivenis configured in the correct sprint policy location and matches the expected template behavior.src/synthorg/templates/builtins/full_company.yaml (1)
376-377: Clean additive update to sprint policy.The
hybridceremony strategy is inserted in the right location without impacting existing sprint configuration fields.src/synthorg/templates/schema.py (1)
230-233: Schema extension is consistent and backward-compatible.Adding optional
ceremony_policytoTemplateDepartmentConfigcleanly enables department-level overrides without breaking existing templates.src/synthorg/templates/builtins/agency.yaml (1)
189-191: Agency ceremony policy default is wired correctly.
event_drivenis configured under the proper sprint ceremony policy path and aligns with the new template defaults.src/synthorg/templates/builtins/startup.yaml (1)
117-118: Default sprint ceremony strategy looks correct.Setting
workflow_config.sprint.ceremony_policy.strategyto"task_driven"is consistent with the template-default objective and keeps the change narrowly scoped.src/synthorg/engine/workflow/__init__.py (1)
64-64: Public API export wiring is complete.
MilestoneDrivenStrategyis added both to imports and__all__, so package-level access is correctly exposed.Also applies to: 98-98
tests/unit/templates/test_renderer.py (1)
548-555: Negative type-check coverage is good here.This test closes the renderer validation gap for non-mapping
ceremony_policyinputs.src/synthorg/engine/workflow/strategies/__init__.py (1)
23-25: Strategy package export update looks correct.
MilestoneDrivenStrategyis properly imported and exposed in__all__.Also applies to: 39-39
tests/unit/templates/test_schema.py (1)
277-299: Schema coverage for the new field is solid.The added tests validate both
Nonedefaulting and multi-field dict acceptance forceremony_policy.src/synthorg/templates/builtins/product_team.yaml (1)
180-181: Template default strategy is correctly set.
"hybrid"underworkflow_config.sprint.ceremony_policyaligns with the intended builtin defaults.src/synthorg/core/company.py (1)
414-428: LGTM! Correct defensive copy pattern for frozen Pydantic model.The
ceremony_policyfield and its_deepcopy_ceremony_policyvalidator correctly implement the defensive copy pattern:
- Uses
copy.deepcopy()to prevent external mutation of the stored dict- Uses
object.__setattr__to bypass the frozen model restriction during validation- Only copies when the value is not
NoneThis aligns with the project's guidelines for
dictfields in frozen Pydantic models. Based on learnings: "Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries."tests/unit/templates/test_workflow_config_integration.py (2)
574-602: LGTM! Comprehensive test coverage for builtin ceremony strategies.The test matrix correctly maps all 9 builtin templates to their expected default ceremony strategies, and the parametrized test verifies each template renders with the correct
ceremony_policy.strategyvalue. This aligns with the template defaults documented inceremony-scheduling.md.
655-722: LGTM! Good coverage for department ceremony policy passthrough.The tests cover the key scenarios:
- Policy data flows correctly from template YAML to
RootConfig.departments- Departments without explicit policy default to
None- Invalid (non-dict) ceremony_policy values are rejected at load time
The error matching on
"ceremony_policy"ensures the validation message is informative.docs/design/ceremony-scheduling.md (2)
674-678: LGTM! Documentation matches implementation.The new observability event constants are correctly documented and match those defined in
src/synthorg/observability/events/workflow.py.
727-734: LGTM! Clear documentation of shipped features.The roadmap section accurately describes the milestone-driven strategy behavior, template defaults, and department override functionality. The note about scheduler integration being tracked in follow-up work is appropriately included.
src/synthorg/observability/events/workflow.py (1)
189-205: LGTM! Well-structured observability event constants.The new milestone-driven strategy event constants follow the established naming convention (
workflow.sprint.*) and include appropriate docstrings. Each constant is used in theMilestoneDrivenStrategyimplementation as shown in the relevant context snippets.tests/unit/engine/workflow/strategies/test_milestone_driven.py (4)
58-78: LGTM! Good protocol compliance tests.The protocol tests correctly verify that
MilestoneDrivenStrategy:
- Is an instance of
CeremonySchedulingStrategy- Returns
CeremonyStrategyType.MILESTONE_DRIVENfromstrategy_type- Returns
VelocityCalcType.POINTS_PER_SPRINTfromget_default_velocity_calculator()
128-150: LGTM! Critical edge-triggered behavior is tested.This test correctly verifies that milestone-driven ceremonies are edge-triggered (fire exactly once per sprint). The first
should_fire_ceremonycall returnsTrueand marks the milestone as fired, while subsequent calls returnFalse.
310-338: LGTM! Properly tests sprint status guard.This test correctly initializes the strategy with an active sprint, assigns tasks, then verifies that
should_transition_sprintreturnsNonefor aPLANNINGstatus sprint. This confirms the status guard works correctly to prevent transitions from non-ACTIVE sprints.
370-434: LGTM! Comprehensive lifecycle hook tests.The tests properly verify:
on_sprint_activatedpopulates_milestonesdict- Re-activation resets state (clears
_milestone_tasksand_fired_milestones)on_sprint_deactivatedclears all internal state including_transition_milestonetests/unit/engine/workflow/strategies/test_milestone_driven_validation.py (2)
385-422: LGTM! Correct approach for testing task-per-milestone limit.The test correctly uses
unittest.mock.patchto temporarily set_MAX_TASKS_PER_MILESTONEto a small value (3), avoiding the need to create 1001 tasks. The patch path matches the module where the constant is defined and referenced.
462-481: LGTM! Tests non-sprint task rejection.This test verifies that tasks not in
sprint.task_idsare silently rejected duringmilestone_assign, addressing the past review concern about stray/global assignments blocking milestones.src/synthorg/engine/workflow/strategies/milestone_driven.py (6)
66-91: LGTM! Clean class design with__slots__.The strategy class uses
__slots__to optimize memory and prevent accidental attribute creation. The four instance variables (_milestones,_milestone_tasks,_fired_milestones,_transition_milestone) correctly track the per-sprint state needed for milestone-driven scheduling.
94-128: LGTM! Well-structured ceremony evaluation.The
should_fire_ceremonymethod correctly:
- Converts
completed_task_idsto a set for O(1) membership testing- Iterates configured milestones and delegates to
_evaluate_milestone- Logs at DEBUG when no milestone matches
The helper extraction keeps the method under 50 lines.
130-179: LGTM! Correct edge-triggered milestone evaluation.The
_evaluate_milestonehelper correctly implements edge-triggered semantics:
- Returns
Falseif milestone has no assigned tasks (logsMILESTONE_NOT_READY)- Returns
Falseif milestone already fired (no re-fire)- Fires when all tasks are complete (
tasks <= completed_set)- Logs at INFO for
MILESTONE_COMPLETEDandCEREMONY_TRIGGERED
396-417: LGTM! Sprint task membership validation.The
_handle_assignmethod correctly guards against assigning tasks that are not in the active sprint (line 407). This prevents stray assignments from blocking milestones, addressing the past review concern.
489-495: LGTM! Milestone assignment logs at INFO level.The
SPRINT_CEREMONY_MILESTONE_ASSIGNEDevent is correctly logged at INFO level with structured kwargs (task_id,milestone,task_count,strategy). This addresses the past review comment about making state transitions visible at production log levels.
585-644: LGTM! Well-structured validation helpers.The module-level validation functions (
_validate_single_milestone,_validate_milestone_string) are cleanly extracted and handle:
- Type checking for dict/string
- Non-empty string validation
- Boolean rejection (since
boolis a subclass ofintin Python)- Length constraints with
_MAX_NAME_LEN- Duplicate milestone name detection
Implement MilestoneDrivenStrategy for the pluggable ceremony scheduling system. Ceremonies fire when all tasks tagged with a milestone are complete, managed via on_external_event lifecycle hooks. - New MilestoneDrivenStrategy with edge-triggered firing - milestone_assign/milestone_unassign external events for task-to-milestone mapping - Configurable transition_milestone for sprint auto-transition - Default velocity calculator: PointsPerSprintVelocityCalculator - 5 new observability event constants - 47 unit tests covering protocol compliance, firing logic, lifecycle hooks, config validation Closes #975
Add ceremony_policy section to all 9 builtin template YAMLs with the strategy matching each template's persona. - solo_founder, startup, data_team: task_driven - dev_shop, product_team, full_company: hybrid - agency: event_driven - research_lab: throughput_adaptive - consultancy: calendar - Parametrized test verifying all 9 templates' ceremony strategies Closes #976
Extend TemplateDepartmentConfig and Department models with optional ceremony_policy field for per-department ceremony scheduling overrides. - TemplateDepartmentConfig: dict[str, Any] | None field (template schema) - Department: dict[str, Any] | None field (runtime model, avoids circular import) - Renderer passes ceremony_policy through build_departments - 3-level resolution: project default > department override > per-ceremony override - Tests for schema, renderer passthrough, and Department model Closes #980
- Fix transition_milestone logic bug: accept task assignments for transition-only milestones not in the milestones list - Use cached _transition_milestone instead of re-reading config - Use SPRINT_CEREMONY_MILESTONE_COMPLETED event constant (was unused) - Add _MAX_TASKS_PER_MILESTONE bound (1000) to prevent unbounded growth - Update design spec: move #975/#976/#980 to shipped, add milestone event constants to observability table - Add 8 new tests: transition-only milestone, strategy_config=None, whitespace payloads, unassign invalid payload, malformed entries, renderer type validation error path Pre-reviewed by 6 agents, 11 findings addressed
…nd CodeRabbit Source fixes (milestone_driven.py): - Extract _evaluate_milestone helper (should_fire_ceremony < 50 lines) - Extract _validate_single_milestone helper (_validate_milestones < 50 lines) - Add _MAX_MILESTONES/_MAX_NAME_LEN enforcement in on_sprint_activated - Fix _handle_assign idempotency: check task presence before limit - Promote milestone assign/unassign logging from DEBUG to INFO - Add logging for invalid payloads in _handle_unassign - Use setdefault pattern in _handle_assign - Fix validate_strategy_config docstring to include TypeError - Fix module docstring: clarify definition vs membership - Fix should_transition_sprint docstring: cached -> parsed Infrastructure fixes: - Add MilestoneDrivenStrategy export to engine/workflow/__init__.py - Fix company.py docstring: correct circular import rationale - Add deepcopy model_validator for Department.ceremony_policy Test improvements: - Split test_milestone_driven.py (803 -> 565 + 460 lines) - Add _MAX_TASKS_PER_MILESTONE limit test with idempotency - Add boolean rejection parametrize cases (name/ceremony) - Add ceremony field too-long rejection test - Add unassign whitespace payload test - Add unassign from nonexistent milestone test - Add renderer-level ceremony_policy non-dict rejection test - Fix should_transition_sprint test: initialize strategy first
- Extract _normalize_payload (shared by assign/unassign) and _add_task_to_milestone helpers from _handle_assign - _handle_assign now validates task_id is in sprint.task_ids before mutating milestone state (prevents stray global assignments from silently blocking milestones) - Unassign is NOT guarded: cleanup must always be possible even if a task leaves the sprint - Tests: add sprint guard test + unassign non-guard test
… test - _render_helpers: remove double deepcopy of ceremony_policy since Department model_validator now handles defensive copying - test_company: add test_ceremony_policy_is_defensively_copied to verify source dict mutation does not affect stored policy
61ecaf8 to
9a36a72
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/milestone_driven.py`:
- Around line 475-495: The code logs
SPRINT_CEREMONY_MILESTONE_ASSIGNED/UNASSIGNED even when set.add()/set.discard()
are no-ops; fix by checking membership before mutating
_milestone_tasks.setdefault(...)'s set: for assignment, do "if task_id not in
tasks: tasks.add(task_id); logger.info(...)" (and still enforce the
capacity/limit check); for unassignment, do "if task_id in tasks:
tasks.discard(task_id); logger.info(...)" so the logger calls in the
milestone_driven strategy only occur on real membership changes.
In `@src/synthorg/templates/schema.py`:
- Around line 230-233: Update the TemplateDepartmentConfig class docstring
Attributes section to include the new ceremony_policy field: mention its type
(dict[str, Any] | None), that it is a per-department ceremony policy override,
and note the default is None; locate the docstring in TemplateDepartmentConfig
and add a short Attributes entry for ceremony_policy to keep the documentation
in sync with the Field declaration.
In `@tests/unit/engine/workflow/strategies/test_milestone_driven.py`:
- Around line 233-249: The test test_no_policy_override_returns_false never
exercises the milestone-policy branch because no tasks are assigned to the
"alpha" milestone; update the test so the sprint contains at least one task
assigned to the "alpha" milestone before calling strategy.should_fire_ceremony.
Specifically, after creating sprint via make_sprint(...) and config via
_make_sprint_config(...), seed sprint.tasks (or call make_sprint with a task
that has milestone="alpha") so the MilestoneDrivenStrategy sees milestone tasks,
then call await strategy.on_sprint_activated(sprint, config) and assert that
strategy.should_fire_ceremony(SprintCeremonyConfig(...), sprint, ctx) returns
False when no policy_override is present.
In `@tests/unit/templates/test_workflow_config_integration.py`:
- Around line 575-602: The test defines a separate _EXPECTED_STRATEGIES list but
lacks a coverage assertion tying it to BUILTIN_TEMPLATES, allowing templates to
be missed; update test_builtin_ceremony_policy_strategy to either assert
len(_EXPECTED_STRATEGIES) == len(BUILTIN_TEMPLATES) or derive
_EXPECTED_STRATEGIES from BUILTIN_TEMPLATES so the test fails if any builtin
template is not represented, referencing the existing symbols
_EXPECTED_STRATEGIES, BUILTIN_TEMPLATES and
test_builtin_ceremony_policy_strategy to locate where to add the check or the
shared matrix.
🪄 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: 5a6b7031-18c5-44a0-9b29-9338bae88240
📒 Files selected for processing (24)
docs/design/ceremony-scheduling.mdsrc/synthorg/core/company.pysrc/synthorg/engine/workflow/__init__.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/engine/workflow/strategies/milestone_driven.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/templates/builtins/agency.yamlsrc/synthorg/templates/builtins/consultancy.yamlsrc/synthorg/templates/builtins/data_team.yamlsrc/synthorg/templates/builtins/dev_shop.yamlsrc/synthorg/templates/builtins/full_company.yamlsrc/synthorg/templates/builtins/product_team.yamlsrc/synthorg/templates/builtins/research_lab.yamlsrc/synthorg/templates/builtins/solo_founder.yamlsrc/synthorg/templates/builtins/startup.yamlsrc/synthorg/templates/schema.pytests/unit/core/test_company.pytests/unit/engine/workflow/strategies/test_milestone_driven.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pytests/unit/templates/test_renderer.pytests/unit/templates/test_schema.pytests/unit/templates/test_workflow_config_integration.pyweb/public/mockServiceWorker.js
💤 Files with no reviewable changes (1)
- web/public/mockServiceWorker.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dashboard Test
- 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 (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: 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 must have type hints and pass mypy strict mode
Docstrings must use Google style and are required on all public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Always useallow_inf_nan=FalseinConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Maximum line length: 88 characters (enforced by ruff)
Functions must be less than 50 lines, files must be less than 800 lines
Files:
src/synthorg/engine/workflow/__init__.pytests/unit/templates/test_renderer.pytests/unit/core/test_company.pytests/unit/templates/test_schema.pysrc/synthorg/templates/schema.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/templates/_render_helpers.pytests/unit/templates/test_workflow_config_integration.pysrc/synthorg/core/company.pysrc/synthorg/observability/events/workflow.pytests/unit/engine/workflow/strategies/test_milestone_driven.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr))
Logger variable name must always belogger(not_logger, notlog)
Always use event constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api). Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured kwargs for logging:logger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, entry/exit of key functions
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/small.
In Pydantic models, useallow_inf_nan=Falsein allConfigDictdeclarations
Pure data models, enums, and re-exports do NOT need logging
Files:
src/synthorg/engine/workflow/__init__.pysrc/synthorg/templates/schema.pysrc/synthorg/engine/workflow/strategies/__init__.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.pysrc/synthorg/observability/events/workflow.pysrc/synthorg/engine/workflow/strategies/milestone_driven.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All test files must use markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests: useasyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Prefer@pytest.mark.parametrizefor testing similar cases
For property-based testing in Python, use Hypothesis (@given+@settings). Profiles:ci(50 examples, default) anddev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n 8 -k properties
Never skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/templates/test_renderer.pytests/unit/core/test_company.pytests/unit/templates/test_schema.pytests/unit/templates/test_workflow_config_integration.pytests/unit/engine/workflow/strategies/test_milestone_driven.pytests/unit/engine/workflow/strategies/test_milestone_driven_validation.py
🧠 Learnings (43)
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.py : For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.py
📚 Learning: 2026-03-16T23:05:29.577Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T23:05:29.577Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.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:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for dict/list fields in frozen Pydantic models
Applied to files:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.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:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.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:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.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 frozen Pydantic models for config/identity; use separate mutable-via-copy models with `model_copy(update=...)` for runtime state that evolves
Applied to files:
tests/unit/core/test_company.pysrc/synthorg/core/company.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:
tests/unit/core/test_company.pysrc/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.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/strategies/__init__.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 : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/templates/_render_helpers.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields
Applied to files:
src/synthorg/templates/_render_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 **/*.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/templates/_render_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 : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/templates/_render_helpers.pysrc/synthorg/core/company.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to **/*.py : Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/core/company.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 **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/core/company.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 Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Always use `allow_inf_nan=False` in `ConfigDict` declarations to reject `NaN`/`Inf` in numeric fields at validation time.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/core/company.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/core/company.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-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-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-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-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-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/workflow.py
📚 Learning: 2026-03-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
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/engine/workflow/strategies/test_milestone_driven.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).
Applied to files:
tests/unit/engine/workflow/strategies/test_milestone_driven.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/workflow/strategies/test_milestone_driven.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Keep functions under 50 lines and files under 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Functions must be less than 50 lines; files must be less than 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to **/*.py : Functions must be less than 50 lines, files must be less than 800 lines
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/engine/workflow/strategies/milestone_driven.py
🔇 Additional comments (16)
src/synthorg/templates/builtins/agency.yaml (1)
189-191: Default strategy wiring looks correct.The
sprint.ceremony_policy.strategyaddition is well-placed and consistent with the new template-level default strategy mechanism.src/synthorg/core/company.py (1)
414-428:ceremony_policypassthrough and defensive-copy handling are solid.This cleanly supports department-level raw policy overrides while reducing input aliasing risk for mutable dict payloads.
src/synthorg/templates/builtins/consultancy.yaml (1)
110-112: Consultancy default strategy mapping is correctly applied.The new sprint ceremony policy block is valid and aligns with the intended default strategy assignment.
src/synthorg/templates/builtins/full_company.yaml (1)
376-377: Hybrid strategy default is integrated cleanly.Good placement under existing sprint config without altering current ceremony definitions.
src/synthorg/templates/builtins/dev_shop.yaml (1)
173-174: Dev shop sprint policy default is correctly configured.This is consistent with the intended template default strategy matrix.
tests/unit/templates/test_renderer.py (1)
548-555: Good negative-path coverage forceremony_policytype validation.This test closes the render-time guard path for malformed department ceremony policy inputs.
src/synthorg/templates/builtins/solo_founder.yaml (1)
74-76: Solo founder default strategy is set correctly.The added sprint ceremony policy block is valid and aligned with the expected baseline behavior.
src/synthorg/templates/builtins/research_lab.yaml (1)
125-127: Research lab strategy default is correctly wired.The throughput-adaptive default is integrated in the expected sprint ceremony policy location.
src/synthorg/templates/builtins/startup.yaml (1)
117-118: Default ceremony strategy mapping for startup looks correct.This aligns with the expected template default and keeps sprint policy explicit.
src/synthorg/templates/builtins/data_team.yaml (1)
105-107:data_teamdefault strategy assignment is consistent.Good addition for explicit ceremony policy defaults.
src/synthorg/engine/workflow/__init__.py (1)
64-64: Re-export wiring forMilestoneDrivenStrategyis complete.Import and
__all__are both updated, so downstream API access is consistent.Also applies to: 98-98
tests/unit/templates/test_schema.py (1)
277-299: Good schema test coverage for departmentceremony_policy.The new cases validate defaulting and accepted mapping payloads, including multi-field policy data.
src/synthorg/engine/workflow/strategies/__init__.py (1)
23-25: Strategy package export update looks correct.
MilestoneDrivenStrategyis imported and exposed in__all__as expected.Also applies to: 39-39
src/synthorg/templates/builtins/product_team.yaml (1)
180-181:product_teamdefault strategy assignment is on target.The explicit
hybridpolicy under sprint config matches the intended template default.tests/unit/core/test_company.py (1)
234-279: ExcellentDepartment.ceremony_policytest expansion.This set now validates default behavior, accepted payloads, and the defensive-copy contract against source-dict mutation.
src/synthorg/engine/workflow/strategies/milestone_driven.py (1)
334-355:on_external_event()is unreachable from production code and cannot populate_milestone_tasks.Lines 334–355 define the only entry point for milestone assignments, but there is no production call site. The
CeremonySchedulerhardcodesexternal_events=()in its context (lines 442, 468) and has no method to forward external events to_active_strategy.on_external_event(). Unless upstream workflow code adds this dispatch,_milestone_tasksremains empty at runtime, makingshould_fire_ceremony()andshould_transition_sprint()unable to fire based on milestone completion. Tests manually invoke this method, but the feature is not wired into the active sprint lifecycle.Additionally, lines 475–495 and 508–517 log INFO-level transitions (
SPRINT_CEREMONY_MILESTONE_ASSIGNED,SPRINT_CEREMONY_MILESTONE_UNASSIGNED) unconditionally after idempotent set operations. These logs fire even when task membership did not actually change (e.g., assigning the same task twice). Check if membership changed before logging state transitions.
| async def test_no_policy_override_returns_false(self) -> None: | ||
| strategy = MilestoneDrivenStrategy() | ||
| sprint = make_sprint(task_count=2, completed_count=2) | ||
| config = _make_sprint_config( | ||
| milestones=[{"name": "alpha", "ceremony": "sprint_review"}], | ||
| ) | ||
| await strategy.on_sprint_activated(sprint, config) | ||
|
|
||
| # Ceremony with frequency but no milestone policy_override | ||
| ceremony = SprintCeremonyConfig( | ||
| name="sprint_review", | ||
| protocol=MeetingProtocolType.ROUND_ROBIN, | ||
| frequency=MeetingFrequency.BI_WEEKLY, | ||
| ) | ||
| ctx = make_context() | ||
| # No milestone tasks assigned, so False regardless | ||
| assert strategy.should_fire_ceremony(ceremony, sprint, ctx) is False |
There was a problem hiding this comment.
This test never reaches the branch named in its title.
Lines 241-249 return False because no tasks are assigned to the milestone, so a regression in "no policy_override" handling would still pass. Seed the milestone tasks first and then assert the intended behavior explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/engine/workflow/strategies/test_milestone_driven.py` around lines
233 - 249, The test test_no_policy_override_returns_false never exercises the
milestone-policy branch because no tasks are assigned to the "alpha" milestone;
update the test so the sprint contains at least one task assigned to the "alpha"
milestone before calling strategy.should_fire_ceremony. Specifically, after
creating sprint via make_sprint(...) and config via _make_sprint_config(...),
seed sprint.tasks (or call make_sprint with a task that has milestone="alpha")
so the MilestoneDrivenStrategy sees milestone tasks, then call await
strategy.on_sprint_activated(sprint, config) and assert that
strategy.should_fire_ceremony(SprintCeremonyConfig(...), sprint, ctx) returns
False when no policy_override is present.
- milestone_driven: only log ASSIGNED/UNASSIGNED on actual membership changes (skip no-op re-adds and discard-of-absent) - schema.py: add ceremony_policy to TemplateDepartmentConfig docstring - test_no_policy_override_returns_false: assign tasks so the milestone branch is actually exercised - test_strategy_matrix_covers_all_builtins: assert _EXPECTED_STRATEGIES covers every BUILTIN_TEMPLATES entry
🤖 I have created a release *beep* *boop* --- ## [0.5.8](v0.5.7...v0.5.8) (2026-04-03) ### Features * auto-select embedding model + fine-tuning pipeline wiring ([#999](#999)) ([a4cbc4e](a4cbc4e)), closes [#965](#965) [#966](#966) * ceremony scheduling batch 3 -- milestone strategy, template defaults, department overrides ([#1019](#1019)) ([321d245](321d245)) * five-pillar evaluation framework for HR performance tracking ([#1017](#1017)) ([5e66cbd](5e66cbd)), closes [#699](#699) * populate comparison page with 53 competitor entries ([#1000](#1000)) ([5cb232d](5cb232d)), closes [#993](#993) * throughput-adaptive and external-trigger ceremony scheduling strategies ([#1003](#1003)) ([bb5c9a4](bb5c9a4)), closes [#973](#973) [#974](#974) ### Bug Fixes * eliminate backup service I/O from API test lifecycle ([#1015](#1015)) ([08d9183](08d9183)) * update run_affected_tests.py to use -n 8 ([#1014](#1014)) ([3ee9fa7](3ee9fa7)) ### Performance * reduce pytest parallelism from -n auto to -n 8 ([#1013](#1013)) ([43e0707](43e0707)) ### CI/CD * bump docker/login-action from 4.0.0 to 4.1.0 in the all group ([#1027](#1027)) ([e7e28ec](e7e28ec)) * bump wrangler from 4.79.0 to 4.80.0 in /.github in the all group ([#1023](#1023)) ([1322a0d](1322a0d)) ### Maintenance * bump github.com/mattn/go-runewidth from 0.0.21 to 0.0.22 in /cli in the all group ([#1024](#1024)) ([b311694](b311694)) * bump https://github.com/astral-sh/ruff-pre-commit from v0.15.8 to 0.15.9 in the all group ([#1022](#1022)) ([1650087](1650087)) * bump node from `71be405` to `387eebd` in /docker/sandbox in the all group ([#1021](#1021)) ([40bd2f6](40bd2f6)) * bump node from `cf38e1f` to `ad82eca` in /docker/web in the all group ([#1020](#1020)) ([f05ab9f](f05ab9f)) * bump the all group in /web with 3 updates ([#1025](#1025)) ([21d40d3](21d40d3)) * bump the all group with 2 updates ([#1026](#1026)) ([36778de](36778de)) * enable additional eslint-react rules and fix violations ([#1028](#1028)) ([80423be](80423be)) --- 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
Implements the final three ceremony scheduling issues: milestone-driven strategy (#975), template default strategy assignments (#976), and per-department ceremony policy overrides (#980).
Changes
#975 -- Milestone-driven ceremony scheduling strategy
MilestoneDrivenStrategyimplementingCeremonySchedulingStrategyprotocolon_external_event(milestone_assign/milestone_unassign)transition_milestonefor sprint auto-transition (works independently of ceremony milestones)PointsPerSprintVelocityCalculator#976 -- Template default ceremony strategy assignments
ceremony_policysection to all 9 builtin template YAMLstask_drivenhybridevent_driventhroughput_adaptivecalendar#980 -- Per-department ceremony policy override
TemplateDepartmentConfigwith optionalceremony_policy: dict[str, Any] | NoneDepartmentmodel with matching field (raw dict to avoid circular imports)build_departmentsTest plan
Review coverage
Pre-reviewed by 6 agents (code-reviewer, conventions-enforcer, logging-audit, docs-consistency, test-analyzer, issue-verifier). 11 findings addressed including a critical logic bug (transition_milestone silently broken when not in milestones list).
Closes #975, #976, #980