feat: personality preset support in template YAML schema#959
Conversation
Extend template rendering to resolve custom presets (from DB) in addition to builtin presets. Thread an optional custom_presets mapping through the renderer call chain so user-defined presets created via the REST API are found during template expansion. Key changes: - get_personality_preset() accepts optional custom_presets mapping (custom checked first, then builtins) - Renderer warns and skips (instead of erroring) on unknown presets - Add validate_preset_references() for pre-flight validation - Add fetch_custom_presets_map() async helper to bridge persistence - Setup controllers pre-fetch custom presets before agent expansion Closes #757 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 7 agents, 9 findings addressed: - fetch_custom_presets_map: handle corrupt JSON, lowercase keys - build_agent_config/setup_personality: catch KeyError as ApiValidationError - _resolve_agent_personality: add exc_info for traceability, remove double lookup - validate_preset_references: rename shadowing variable - docs/design/organization.md: document custom preset rendering behavior - Add tests: build_agent_config coverage, corrupt JSON, key lowercasing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughCustom personality presets are loaded from persistence via 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements support for custom personality presets within the template rendering and agent setup pipelines. It introduces a mechanism to fetch user-defined presets from the database and prioritize them over built-in presets during resolution. Additionally, the rendering logic has been updated to log warnings and proceed without a personality when a referenced preset is missing, rather than raising an error. New utility functions for pre-flight validation and comprehensive unit tests for custom preset resolution have also been added. I have no feedback to provide.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
Adds support for resolving user-defined (DB-backed) personality presets during template rendering and setup flows, alongside existing built-in presets, and introduces pre-flight validation for preset references.
Changes:
- Thread
custom_presetsthrough the template renderer pipeline and setup controllers so templates can resolve DB-backed presets. - Change renderer behavior for unknown presets to warn-and-skip (agent proceeds without a personality) and add
validate_preset_references()for advisory checks. - Add
fetch_custom_presets_map()to bridge async preset persistence to the sync rendering/expansion pipeline; update docs and add targeted tests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/synthorg/templates/renderer.py |
Threads custom_presets through render/expand flow; warns and skips unknown presets; adds validate_preset_references(). |
src/synthorg/templates/presets.py |
Extends get_personality_preset() to accept optional custom_presets. |
src/synthorg/templates/preset_service.py |
Adds async fetch_custom_presets_map() helper for DB → mapping conversion. |
src/synthorg/templates/__init__.py |
Exports validate_preset_references() from templates package. |
src/synthorg/observability/events/template.py |
Adds event constant for custom preset resolution debug logging. |
src/synthorg/api/controllers/setup.py |
Prefetches custom presets for agent creation during setup. |
src/synthorg/api/controllers/setup_personality.py |
Resolves preset selection using custom presets and converts unknowns to ApiValidationError. |
src/synthorg/api/controllers/setup_helpers.py |
Prefetches/passes custom presets when auto-creating agents from templates. |
src/synthorg/api/controllers/setup_agents.py |
Extends expand_template_agents()/build_agent_config() to accept custom_presets. |
docs/design/organization.md |
Documents custom preset rendering behavior and validation function. |
tests/unit/templates/test_renderer.py |
Updates unknown-preset expectation (warn+skip) and adds custom preset resolution tests. |
tests/unit/templates/test_renderer_validation.py |
Adds unit tests for validate_preset_references(). |
tests/unit/templates/test_presets.py |
Adds unit tests for get_personality_preset(..., custom_presets=...). |
tests/unit/templates/test_preset_service.py |
Adds unit tests for fetch_custom_presets_map() including corrupt JSON and key normalization. |
tests/unit/api/controllers/test_setup_agents.py |
Adds coverage for custom preset threading through setup agent expansion/config building. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def get_personality_preset( | ||
| name: str, | ||
| custom_presets: Mapping[str, dict[str, Any]] | None = None, | ||
| ) -> dict[str, Any]: | ||
| """Look up a personality preset by name. | ||
|
|
||
| Custom presets are checked first (higher precedence), then builtins. | ||
|
|
||
| Args: | ||
| name: Preset name (case-insensitive, whitespace-stripped). | ||
| custom_presets: Optional mapping of custom preset names to | ||
| personality config dicts. Keys must be lowercased. | ||
|
|
||
| Returns: | ||
| A *copy* of the personality configuration dict. | ||
|
|
||
| Raises: | ||
| KeyError: If the preset name is not found. | ||
| KeyError: If the preset name is not found in either source. | ||
| """ | ||
| key = name.strip().lower() | ||
| if key not in PERSONALITY_PRESETS: | ||
| available = sorted(PERSONALITY_PRESETS) | ||
| msg = f"Unknown personality preset {name!r}. Available: {available}" | ||
| logger.warning( | ||
| TEMPLATE_PERSONALITY_PRESET_UNKNOWN, | ||
| preset_name=name, | ||
| available=available, | ||
| ) | ||
| raise KeyError(msg) | ||
| return dict(PERSONALITY_PRESETS[key]) | ||
| if custom_presets is not None and key in custom_presets: | ||
| return dict(custom_presets[key]) | ||
| if key in PERSONALITY_PRESETS: | ||
| return dict(PERSONALITY_PRESETS[key]) |
There was a problem hiding this comment.
get_personality_preset() currently gives custom presets higher precedence than built-ins, which allows custom entries to shadow built-in names. However, PersonalityPresetService enforces “cannot create/update/delete builtin preset” (_check_not_builtin) and the design docs state custom presets cannot shadow built-ins. Consider guarding against collisions here (e.g., ignore/skip custom values when key in PERSONALITY_PRESETS, or treat it as an error) so behavior is consistent even if the DB contains corrupted/legacy rows.
src/synthorg/templates/renderer.py
Outdated
| TEMPLATE_PERSONALITY_PRESET_UNKNOWN, | ||
| agent=name, | ||
| preset=preset_name, | ||
| exc_info=True, |
There was a problem hiding this comment.
_resolve_agent_personality() logs TEMPLATE_PERSONALITY_PRESET_UNKNOWN with exc_info=True when get_personality_preset() raises KeyError, but get_personality_preset() already logs an unknown-preset warning. This can lead to duplicate warnings and stack traces for an expected missing-preset case. Consider logging only once (either remove this warning, or downgrade it / set exc_info=False) while keeping the agent/preset context.
| exc_info=True, |
tests/unit/templates/test_presets.py
Outdated
| def test_custom_preset_overrides_builtin(self) -> None: | ||
| """Custom preset with same name as builtin takes precedence.""" | ||
| custom = { | ||
| "visionary_leader": { | ||
| "traits": ("overridden",), | ||
| "communication_style": "overridden", | ||
| "description": "Overridden builtin", | ||
| "openness": 0.1, | ||
| "conscientiousness": 0.1, | ||
| "extraversion": 0.1, | ||
| "agreeableness": 0.1, | ||
| "stress_response": 0.1, | ||
| }, | ||
| } | ||
| result = get_personality_preset( | ||
| "visionary_leader", | ||
| custom_presets=custom, | ||
| ) | ||
| assert result["communication_style"] == "overridden" | ||
|
|
There was a problem hiding this comment.
This test codifies behavior where a custom preset can override a built-in preset name (visionary_leader). That conflicts with the project invariant that custom presets cannot shadow built-ins (enforced by the preset service via _check_not_builtin, and documented in docs/design/organization.md). Consider removing/rewriting this test to reflect supported behavior (e.g., collisions are rejected or ignored).
| try: | ||
| result[key] = json.loads(row.config_json) | ||
| except json.JSONDecodeError: | ||
| logger.exception( | ||
| PRESET_VALIDATION_FAILED, | ||
| preset_name=row.name, | ||
| reason="corrupt_json_in_fetch_map", | ||
| ) |
There was a problem hiding this comment.
fetch_custom_presets_map() blindly inserts json.loads(row.config_json) into the result. If the JSON decodes to a non-object (e.g., list/string/number) or if a row name collides with a built-in preset, downstream callers (and the “no shadowing built-ins” invariant) can break. Consider (1) verifying the decoded value is a dict before adding it, and (2) skipping/logging any rows whose normalized key is in PERSONALITY_PRESETS to avoid shadowing built-ins even if the DB contains legacy/corrupt rows.
| try: | |
| result[key] = json.loads(row.config_json) | |
| except json.JSONDecodeError: | |
| logger.exception( | |
| PRESET_VALIDATION_FAILED, | |
| preset_name=row.name, | |
| reason="corrupt_json_in_fetch_map", | |
| ) | |
| # Do not allow custom presets to shadow built-in presets, even if | |
| # such rows exist in persistence (e.g. legacy/corrupt data). | |
| if key in PERSONALITY_PRESETS: | |
| logger.warning( | |
| PRESET_CONFLICT, | |
| preset_name=row.name, | |
| reason="custom_preset_shadows_builtin_in_fetch_map", | |
| ) | |
| continue | |
| try: | |
| decoded = json.loads(row.config_json) | |
| except json.JSONDecodeError: | |
| logger.exception( | |
| PRESET_VALIDATION_FAILED, | |
| preset_name=row.name, | |
| reason="corrupt_json_in_fetch_map", | |
| ) | |
| continue | |
| # Ensure we only expose object-like configs; non-dict JSON values | |
| # are considered invalid and are skipped. | |
| if not isinstance(decoded, dict): | |
| logger.error( | |
| PRESET_VALIDATION_FAILED, | |
| preset_name=row.name, | |
| reason="non_object_config_in_fetch_map", | |
| ) | |
| continue | |
| result[key] = decoded |
docs/design/organization.md
Outdated
| Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow built-in names. All custom presets are validated against `PersonalityConfig` before persistence. The API distinguishes origin via a `source: "builtin" | "custom"` field in responses. | ||
|
|
||
| During template rendering and setup agent expansion, custom presets are fetched from the database and passed into the rendering pipeline alongside builtins. If an agent references a preset name that exists in neither custom nor built-in collections, the renderer logs a warning and omits the personality (the agent proceeds with no personality assigned) rather than raising an error. The `validate_preset_references()` function provides advisory pre-flight validation for template import/export scenarios, returning warning strings for unknown presets without raising. |
There was a problem hiding this comment.
The docs state that unknown presets during both template rendering and setup agent expansion “omit the personality (agent proceeds with no personality assigned)”. However, expand_template_agents() currently falls back to the pragmatic_builder preset on unknown names and always sets a personality dict. Also, this section reiterates “cannot shadow built-in names” while the renderer/preset lookup now gives custom presets higher precedence. Please align the documentation with actual behavior/invariants (or adjust the code to match the documented behavior).
| Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow built-in names. All custom presets are validated against `PersonalityConfig` before persistence. The API distinguishes origin via a `source: "builtin" | "custom"` field in responses. | |
| During template rendering and setup agent expansion, custom presets are fetched from the database and passed into the rendering pipeline alongside builtins. If an agent references a preset name that exists in neither custom nor built-in collections, the renderer logs a warning and omits the personality (the agent proceeds with no personality assigned) rather than raising an error. The `validate_preset_references()` function provides advisory pre-flight validation for template import/export scenarios, returning warning strings for unknown presets without raising. | |
| Custom preset names must match `^[a-z][a-z0-9_]*$`. All custom presets are validated against `PersonalityConfig` before persistence. When a custom preset shares a name with a built-in preset, the custom definition takes precedence at lookup time. The API distinguishes origin via a `source: "builtin" | "custom"` field in responses. | |
| During template rendering and setup agent expansion, custom presets are fetched from the database and passed into the rendering pipeline alongside builtins. Preset lookup always prefers custom presets over built-ins, falling back from custom → builtin → fallback behavior. If an agent references a preset name that exists in neither custom nor built-in collections, template rendering logs a warning and omits the personality (the agent proceeds with no personality assigned) rather than raising an error. During setup agent expansion, `expand_template_agents()` instead logs a warning and falls back to the `pragmatic_builder` preset, so expanded agents always receive a personality dict. The `validate_preset_references()` function provides advisory pre-flight validation for template import/export scenarios, returning warning strings for unknown presets without raising. |
Move _resolve_agent_personality and _validate_inline_personality into new _preset_resolution.py module (same pattern as _render_helpers.py). Move validate_preset_references to presets.py where it belongs. Move preset-related test classes to test_renderer_validation.py. renderer.py: 874 -> 742 lines test_renderer.py: 845 -> 701 lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #959 +/- ##
=======================================
Coverage 92.00% 92.00%
=======================================
Files 612 613 +1
Lines 33021 33071 +50
Branches 3170 3175 +5
=======================================
+ Hits 30380 30426 +46
- Misses 2097 2100 +3
- Partials 544 545 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/synthorg/templates/__init__.py (1)
1-25: 🧹 Nitpick | 🔵 TrivialUpdate module docstring to include
validate_preset_references.The module docstring lists the public API but omits the newly exported
validate_preset_referencesfunction. Add it to maintain consistency between the docstring and__all__.📝 Suggested docstring update
"""Company templates: built-in presets and custom template loading. Public API ---------- .. autosummary:: load_template load_template_file list_templates list_builtin_templates render_template + validate_preset_references CompanyTemplate ... """Also applies to: 48-48, 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/templates/__init__.py` around lines 1 - 25, The module docstring's public API autosummary list is missing the newly exported function validate_preset_references; update the top-level docstring to include validate_preset_references alongside the other exported names (so the autosummary list matches __all__ and the actual exports such as load_template, load_template_file, list_templates, etc.), and repeat the same addition at the other two docstring locations noted (around lines referenced in the review) to keep all docstring occurrences consistent.src/synthorg/api/controllers/setup_agents.py (1)
86-123:⚠️ Potential issue | 🟠 MajorPreserve unresolved preset names during setup expansion.
render_template()now warns and leaves the agent without a personality when a preset is missing. This setup path still rewrites the reference topragmatic_builder, which loses the original preset name during setup/import and prevents a later-created custom preset from ever being picked up automatically.Possible fix
try: personality = get_personality_preset( preset_name, custom_presets=custom_presets, ) except KeyError: logger.warning( SETUP_TEMPLATE_INVALID, preset=preset_name, agent_index=idx, reason="unknown_personality_preset", ) - preset_name = "pragmatic_builder" - personality = get_personality_preset(preset_name) + personality = None @@ agent_dict: dict[str, Any] = { "name": name, "role": agent_cfg.role, "department": agent_cfg.department or "engineering", "level": agent_cfg.level.value, - "personality": personality, "personality_preset": preset_name, "tier": tier, "model": {"provider": "", "model_id": ""}, } + if personality is not None: + agent_dict["personality"] = personality🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup_agents.py` around lines 86 - 123, The catch block currently replaces the missing preset_name with "pragmatic_builder" and assigns a personality, losing the original preset reference; instead, keep preset_name unchanged, log the warning via SETUP_TEMPLATE_INVALID with reason "unknown_personality_preset", and set personality to None (or omit it) so agent_dict["personality_preset"] retains the unresolved preset and later custom presets can be applied; update the KeyError handler around get_personality_preset, and ensure agent_dict uses the preserved preset_name for "personality_preset" while "personality" may be None.
🤖 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/api/controllers/setup_personality.py`:
- Around line 93-104: Add a WARNING log with context before converting the
KeyError to ApiValidationError in the block that calls get_personality_preset
(the except KeyError handler): log the unknown preset value and any relevant
request or user context using the module/logger used elsewhere (and emit a
specific event constant from synthorg.observability.events.setup, create one if
missing) and then raise ApiValidationError as before; ensure the log call
happens immediately before raising and references data.personality_preset and
the same message text used in the ApiValidationError.
In `@src/synthorg/templates/renderer.py`:
- Around line 795-805: The code is double-logging missing personality presets:
get_personality_preset(preset_name, custom_presets=custom_presets) already logs
TEMPLATE_PERSONALITY_PRESET_UNKNOWN before raising, so remove the redundant
logger.warning(...) in the except KeyError block (including the exc_info=True)
and instead silently handle the KeyError (e.g., no-op or set result to None) so
rendering proceeds without producing a second warning/traceback; keep references
to preset_name, custom_presets, get_personality_preset,
TEMPLATE_PERSONALITY_PRESET_UNKNOWN, and logger.warning to locate the change.
In `@tests/unit/templates/test_renderer.py`:
- Around line 675-700: Update the
test_unknown_custom_preset_warns_in_full_render test to assert the full
warn-and-skip behavior: after calling load_template_file and render_template
(functions referenced), verify the rendered RootConfig's first agent has no
personality (e.g., agent.personality is None) and capture the
TEMPLATE_PERSONALITY_PRESET_UNKNOWN warning was emitted (use the pytest caplog
fixture or similar to assert TEMPLATE_PERSONALITY_PRESET_UNKNOWN appears in the
logged messages). Ensure you reference the test function name and the constants
TEMPLATE_PERSONALITY_PRESET_UNKNOWN, load_template_file, render_template, and
RootConfig when adding these assertions.
---
Outside diff comments:
In `@src/synthorg/api/controllers/setup_agents.py`:
- Around line 86-123: The catch block currently replaces the missing preset_name
with "pragmatic_builder" and assigns a personality, losing the original preset
reference; instead, keep preset_name unchanged, log the warning via
SETUP_TEMPLATE_INVALID with reason "unknown_personality_preset", and set
personality to None (or omit it) so agent_dict["personality_preset"] retains the
unresolved preset and later custom presets can be applied; update the KeyError
handler around get_personality_preset, and ensure agent_dict uses the preserved
preset_name for "personality_preset" while "personality" may be None.
In `@src/synthorg/templates/__init__.py`:
- Around line 1-25: The module docstring's public API autosummary list is
missing the newly exported function validate_preset_references; update the
top-level docstring to include validate_preset_references alongside the other
exported names (so the autosummary list matches __all__ and the actual exports
such as load_template, load_template_file, list_templates, etc.), and repeat the
same addition at the other two docstring locations noted (around lines
referenced in the review) to keep all docstring occurrences consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b943db1-58ff-475f-8fe1-1ded094adc74
📒 Files selected for processing (15)
docs/design/organization.mdsrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/observability/events/template.pysrc/synthorg/templates/__init__.pysrc/synthorg/templates/preset_service.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_preset_service.pytests/unit/templates/test_presets.pytests/unit/templates/test_renderer.pytests/unit/templates/test_renderer_validation.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Agent
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for multiple exception types in except clauses — PEP 758 style, enforced by ruff on Python 3.14
Use line length of 88 characters, enforced by ruff
Files:
src/synthorg/observability/events/template.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/templates/__init__.pysrc/synthorg/api/controllers/setup_personality.pytests/unit/templates/test_preset_service.pysrc/synthorg/templates/preset_service.pytests/unit/templates/test_presets.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_renderer.pysrc/synthorg/templates/presets.pytests/unit/templates/test_renderer_validation.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/renderer.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: All public functions and classes in Python must have type hints; mypy strict mode is enforced
Use Google style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones for immutability. 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 with conventions:allow_inf_nan=Falsein allConfigDictdeclarations,@computed_fieldfor derived values instead of storing redundant fields,NotBlankStrfromcore.typesfor all identifier/name fields including optional and tuple variants
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Maintain 80% minimum test coverage; enforced in CI
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, orlarge/medium/smallaliases
Apply mypy type-checking in pre-push hooks for Python code
Files:
src/synthorg/observability/events/template.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/templates/__init__.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/templates/preset_service.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/renderer.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. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code
Always useloggeras the variable name for loggers (not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g., API_REQUEST_STARTED from events.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
DEBUG logging is appropriate for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not need logging
Files:
src/synthorg/observability/events/template.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/templates/__init__.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/templates/preset_service.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/renderer.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Apply ruff check+format in pre-commit hooks to ensure consistent Python code style
Files:
src/synthorg/observability/events/template.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/templates/__init__.pysrc/synthorg/api/controllers/setup_personality.pytests/unit/templates/test_preset_service.pysrc/synthorg/templates/preset_service.pytests/unit/templates/test_presets.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_renderer.pysrc/synthorg/templates/presets.pytests/unit/templates/test_renderer_validation.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/renderer.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect the approved deviation from the design spec
Files:
docs/design/organization.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test categorization
Global test timeout is 30 seconds per test (in pyproject.toml) — do not add per-file markers unless overriding with longer timeouts liketimeout(60)
Prefer@pytest.mark.parametrizefor testing similar cases
Usetest-provider,test-small-001, etc. in tests instead of real vendor names
Use Hypothesis for property-based testing with@given+@settings. UseHYPOTHESIS_PROFILEenv var to select profiles:ci(50 examples, default) ordev(1000 examples)
Never skip, dismiss, or ignore flaky tests. Always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic. For indefinite blocking tasks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/templates/test_preset_service.pytests/unit/templates/test_presets.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_renderer.pytests/unit/templates/test_renderer_validation.py
🧠 Learnings (10)
📚 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 src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
src/synthorg/observability/events/template.pydocs/design/organization.mdsrc/synthorg/templates/__init__.pysrc/synthorg/templates/presets.pytests/unit/templates/test_renderer_validation.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/renderer.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {docs/design/operations.md,src/synthorg/providers/presets.py,.claude/**/*.{md,yml,yaml}} : Vendor names may appear only in: (1) Operations design page, (2) `.claude/` skill/agent files, (3) third-party import paths, (4) provider presets (`src/synthorg/providers/presets.py`), (5) tests using `test-provider`, `test-small-001`, etc.
Applied to files:
docs/design/organization.md
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : 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/api/controllers/setup.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 src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/api/controllers/setup_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 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/api/controllers/test_setup_agents.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
tests/unit/api/controllers/test_setup_agents.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/presets.pysrc/synthorg/templates/renderer.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement.
Applied to files:
src/synthorg/templates/presets.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Applied to files:
src/synthorg/templates/presets.pysrc/synthorg/templates/renderer.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/settings/**/*.py : Settings package (settings/): runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge (JSON serialization for Pydantic/collections), ConfigResolver (typed accessors), validation, registry, change notifications via message bus, SettingsSubscriber protocol, SettingsChangeDispatcher (polls `#settings` channel, routes to subscribers, restart_required filtering)
Applied to files:
src/synthorg/api/controllers/setup_agents.py
🔇 Additional comments (6)
src/synthorg/observability/events/template.py (1)
18-20: LGTM!The new event constant follows the established naming conventions and is properly typed with
Final[str].src/synthorg/api/controllers/setup_helpers.py (1)
395-424: LGTM!The integration of custom presets into
auto_create_template_agentsis clean. The deferred import avoids circular dependencies, and the custom presets are correctly fetched and passed through toexpand_template_agents.docs/design/organization.md (1)
299-300: LGTM!The documentation accurately describes the custom preset resolution behavior, the warn-and-skip approach for unknown presets, and the advisory
validate_preset_references()function. This aligns well with the code changes.src/synthorg/api/controllers/setup.py (1)
333-345: LGTM!The custom preset integration in
create_agentfollows a sensible order: validate provider/model first, then fetch and pass custom presets tobuild_agent_config. The deferred import pattern is consistent with other controllers.tests/unit/templates/test_preset_service.py (1)
257-348: LGTM!Comprehensive test coverage for
fetch_custom_presets_mapincluding edge cases: empty repository, multiple presets, corrupt JSON handling (skip and continue), and key lowercasing. The tests align well with the documented behavior.tests/unit/templates/test_presets.py (1)
101-194: LGTM!Excellent test coverage for the
custom_presetsparameter: precedence over builtins, copy semantics, case-insensitive lookup, fallback behavior, and error semantics. The tests thoroughly validate the contract documented in the PR objectives.
| def test_unknown_custom_preset_warns_in_full_render( | ||
| self, | ||
| tmp_template_file: TemplateFileFactory, | ||
| ) -> None: | ||
| """Template with unknown preset renders without error.""" | ||
| yaml_content = """\ | ||
| template: | ||
| name: "Unknown Preset Test" | ||
| description: "References nonexistent preset" | ||
| version: "1.0.0" | ||
|
|
||
| company: | ||
| type: "custom" | ||
|
|
||
| agents: | ||
| - role: "Backend Developer" | ||
| name: "Test Dev" | ||
| level: "mid" | ||
| model: "medium" | ||
| department: "engineering" | ||
| personality_preset: "totally_unknown" | ||
| """ | ||
| path = tmp_template_file(yaml_content) | ||
| loaded = load_template_file(path) | ||
| config = render_template(loaded) | ||
| assert isinstance(config, RootConfig) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Assert the full warn-and-skip contract here.
This only proves that render_template() does not raise. It still passes if the full-render path silently falls back to another personality or stops emitting the unknown-preset warning. Please assert that the rendered agent ends up without a personality and capture TEMPLATE_PERSONALITY_PRESET_UNKNOWN.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/templates/test_renderer.py` around lines 675 - 700, Update the
test_unknown_custom_preset_warns_in_full_render test to assert the full
warn-and-skip behavior: after calling load_template_file and render_template
(functions referenced), verify the rendered RootConfig's first agent has no
personality (e.g., agent.personality is None) and capture the
TEMPLATE_PERSONALITY_PRESET_UNKNOWN warning was emitted (use the pytest caplog
fixture or similar to assert TEMPLATE_PERSONALITY_PRESET_UNKNOWN appears in the
logged messages). Ensure you reference the test function name and the constants
TEMPLATE_PERSONALITY_PRESET_UNKNOWN, load_template_file, render_template, and
RootConfig when adding these assertions.
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/_preset_resolution.py`:
- Around line 78-85: The except KeyError handler in resolve_personality_preset
(the block that logs TEMPLATE_PERSONALITY_PRESET_UNKNOWN with agent=name,
preset=preset_name and exc_info=True) is duplicating the warning already emitted
by get_personality_preset(); remove that logger.warning call (leave the return
None) so the unknown-preset case is only logged once by get_personality_preset.
In `@src/synthorg/templates/presets.py`:
- Around line 465-495: The validate_preset_references function is implemented
but never invoked; call it from the setup controller paths that perform template
import/export (where templates are parsed/loaded) so preset warnings are
surfaced during setup. Specifically, after you parse or load a CompanyTemplate
in the import/export controller method, invoke
validate_preset_references(template, custom_presets=...) and handle the returned
tuple: if non-empty, attach the warnings to the controller response (or convert
to flash/log entries or include in the API payload) so callers see the advisory
messages; use the existing PERSONALITY_PRESETS and CompanyTemplate types to
locate the relevant code paths and propagate the warnings consistently with
other setup validation behavior.
🪄 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: a3101551-2618-47b9-97df-b2c9e90dc7c4
📒 Files selected for processing (6)
src/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.pytests/unit/templates/test_renderer.pytests/unit/templates/test_renderer_validation.py
💤 Files with no reviewable changes (1)
- tests/unit/templates/test_renderer.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for multiple exception types in except clauses — PEP 758 style, enforced by ruff on Python 3.14
Use line length of 88 characters, enforced by ruff
Files:
src/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pytests/unit/templates/test_renderer_validation.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: All public functions and classes in Python must have type hints; mypy strict mode is enforced
Use Google style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones for immutability. 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 with conventions:allow_inf_nan=Falsein allConfigDictdeclarations,@computed_fieldfor derived values instead of storing redundant fields,NotBlankStrfromcore.typesfor all identifier/name fields including optional and tuple variants
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Maintain 80% minimum test coverage; enforced in CI
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, orlarge/medium/smallaliases
Apply mypy type-checking in pre-push hooks for Python code
Files:
src/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.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. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code
Always useloggeras the variable name for loggers (not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g., API_REQUEST_STARTED from events.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
DEBUG logging is appropriate for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not need logging
Files:
src/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Apply ruff check+format in pre-commit hooks to ensure consistent Python code style
Files:
src/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pytests/unit/templates/test_renderer_validation.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test categorization
Global test timeout is 30 seconds per test (in pyproject.toml) — do not add per-file markers unless overriding with longer timeouts liketimeout(60)
Prefer@pytest.mark.parametrizefor testing similar cases
Usetest-provider,test-small-001, etc. in tests instead of real vendor names
Use Hypothesis for property-based testing with@given+@settings. UseHYPOTHESIS_PROFILEenv var to select profiles:ci(50 examples, default) ordev(1000 examples)
Never skip, dismiss, or ignore flaky tests. Always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic. For indefinite blocking tasks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/templates/test_renderer_validation.py
🧠 Learnings (6)
📚 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 src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
src/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pytests/unit/templates/test_renderer_validation.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.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/templates/__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 : 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/templates/renderer.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/templates/renderer.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Applied to files:
src/synthorg/templates/renderer.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/renderer.py
🔇 Additional comments (17)
src/synthorg/templates/__init__.py (1)
48-48: LGTM!The new
validate_preset_referencesexport is correctly imported from the presets module and added to__all__in alphabetical order, maintaining consistency with the existing public API surface.Also applies to: 82-82
src/synthorg/templates/renderer.py (7)
73-109: LGTM!The
custom_presetsparameter is properly added to the publicrender_templatefunction with correct type annotation and documentation. The parameter is correctly threaded to_render_to_dict.
121-176: LGTM!The
custom_presetsparameter is correctly threaded through_render_to_dictto_build_config_dictand_resolve_inheritance, maintaining consistent type annotations.
179-233: LGTM!The
_resolve_inheritancefunction correctly passescustom_presetsto_render_and_merge_parent, ensuring inherited templates also receive the custom preset mapping.
269-293: LGTM!The
_render_and_merge_parentfunction correctly threadscustom_presetsto the recursive_render_to_dictcall, ensuring parent templates can resolve custom presets.
448-509: LGTM!The
_build_config_dictfunction correctly passescustom_presetsto_expand_agents.
597-628: LGTM!The
_expand_agentsfunction correctly threadscustom_presetsto each_expand_single_agentcall.
631-676: LGTM!The
_expand_single_agentfunction cleanly delegates personality resolution toresolve_agent_personality, correctly passingcustom_presets. ThePLR0913suppression is appropriate given the number of parameters required for the rendering context.src/synthorg/templates/presets.py (2)
10-15: LGTM!The
TYPE_CHECKINGimports are correctly structured for type-only references toMappingandCompanyTemplate.
412-443: LGTM!The
get_personality_presetfunction correctly:
- Normalizes the key consistently with
fetch_custom_presets_map- Prioritizes custom presets over builtins
- Returns copies via
dict()to prevent mutation- Logs a warning before raising
KeyErrorfor unknown presetstests/unit/templates/test_renderer_validation.py (5)
1-32: LGTM!The test file imports and helper function are well-structured. The
_make_templatehelper provides a clean way to construct test templates with minimal boilerplate.
38-104: LGTM!Comprehensive test coverage for
validate_preset_references:
- Validates no warnings for builtin/custom presets
- Validates warnings generated for unknown presets
- Verifies case-insensitive matching
- Confirms immutable tuple return type
110-133: LGTM!Tests correctly verify the new warn-and-skip behavior for unknown presets: the
personalityfield is omitted without raisingTemplateRenderError, and the agent still has valid role data.
139-209: LGTM!The custom preset resolution tests provide good coverage:
- Verifies custom preset is resolved during agent expansion
- End-to-end test confirms
render_templatehonorscustom_presetsparameter- Custom preset test data includes essential personality fields
211-254: LGTM!Good coverage for edge cases:
- Unknown preset in full render proceeds without error
- Builtin presets continue to work when
custom_presetsdict is provided (ensures the fallback chain works)src/synthorg/templates/_preset_resolution.py (2)
1-25: LGTM!The module setup follows coding guidelines correctly: internal module documentation, appropriate imports, and proper logger initialization with
get_logger(__name__).
96-118: LGTM!The
_validate_inline_personalityhelper correctly validates againstPersonalityConfig, catches bothValidationErrorandTypeError, and logs at WARNING level before raisingTemplateRenderErroras required by coding guidelines.
| def validate_preset_references( | ||
| template: CompanyTemplate, | ||
| custom_presets: Mapping[str, dict[str, Any]] | None = None, | ||
| ) -> tuple[str, ...]: | ||
| """Check all agent personality_preset references against known presets. | ||
|
|
||
| Returns a tuple of warning messages for unknown presets. Does not | ||
| raise -- purely advisory for pre-flight validation and template | ||
| import/export scenarios. | ||
|
|
||
| Args: | ||
| template: Parsed template to validate. | ||
| custom_presets: Optional custom preset mapping. | ||
|
|
||
| Returns: | ||
| Tuple of warning strings (empty when all presets are known). | ||
| """ | ||
| issues: list[str] = [] | ||
| for agent_cfg in template.agents: | ||
| preset = agent_cfg.personality_preset | ||
| if preset is None: | ||
| continue | ||
| key = preset.strip().lower() | ||
| if custom_presets is not None and key in custom_presets: | ||
| continue | ||
| if key in PERSONALITY_PRESETS: | ||
| continue | ||
| issues.append( | ||
| f"Agent {agent_cfg.role!r} references unknown personality preset {preset!r}" | ||
| ) | ||
| return tuple(issues) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for callers of validate_preset_references in the codebase
# Find all usages of validate_preset_references
rg -n --type=py 'validate_preset_references' -g '!*test*'Repository: Aureliolo/synthorg
Length of output: 307
🏁 Script executed:
# Search for all callers of validate_preset_references, including test files
rg -n --type=py 'validate_preset_references'
# Also search for setup controllers that might call this function
fd -e py -path '*/setup*' -o -path '*/controller*' | head -20Repository: Aureliolo/synthorg
Length of output: 1484
Consider integrating validate_preset_references into setup controller import/export paths.
The function is properly exported and well-tested, but has no callers within the application code itself. If template import/export operations should surface these preset warnings during setup, wire this validation into the appropriate controller methods that handle template loading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/templates/presets.py` around lines 465 - 495, The
validate_preset_references function is implemented but never invoked; call it
from the setup controller paths that perform template import/export (where
templates are parsed/loaded) so preset warnings are surfaced during setup.
Specifically, after you parse or load a CompanyTemplate in the import/export
controller method, invoke validate_preset_references(template,
custom_presets=...) and handle the returned tuple: if non-empty, attach the
warnings to the controller response (or convert to flash/log entries or include
in the API payload) so callers see the advisory messages; use the existing
PERSONALITY_PRESETS and CompanyTemplate types to locate the relevant code paths
and propagate the warnings consistently with other setup validation behavior.
…abbit - Move fetch_custom_presets_map outside AGENT_LOCK in setup_personality to reduce lock contention (matches pattern in setup.py) - Use copy.deepcopy() for custom preset dicts at system boundary - Remove exc_info=True from warn-and-skip path (avoids double-logging) - Add warning log before raise ApiValidationError in build_agent_config and update_agent_personality (CLAUDE.md: log before raising) - Add SETUP_PRESET_NOT_FOUND event constant - Validate json.loads result is a dict in fetch_custom_presets_map - Use asyncio.TaskGroup for 3 independent awaits in auto_create_template_agents - Include custom presets in KeyError available-presets message - Add validate_preset_references to autosummary directive - Distinguish renderer vs setup-agent fallback behavior in docs - Add Raises sections to build_agent_config and expand_template_agents - Add custom_presets arg and merge-directive step to _expand_single_agent docstring - Add "Keys must be lowercased" to validate_preset_references docstring - Clarify inline-personality precedence in resolve_agent_personality - Rename test to clarify defense-in-depth for builtin shadowing - Rename test_unknown_preset_falls_back_to_default for clarity - Assert personality is absent in full-render unknown-preset test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/synthorg/templates/preset_service.py (1)
433-451:⚠️ Potential issue | 🟠 MajorValidate schema-invalid preset rows before caching them.
This bridge only filters bad JSON and non-object payloads. A row that is still a
dictbut not a validPersonalityConfigwill be added tocustom_presets, and the failure is just deferred until some setup/render path references that preset. Reuse_validate_personality_config()here and skip invalid rows too.🛡️ Suggested fix
if not isinstance(decoded, dict): logger.error( PRESET_VALIDATION_FAILED, preset_name=row.name, reason="non_object_config_in_fetch_map", ) continue - result[key] = decoded + try: + validated = _validate_personality_config(decoded) + except ApiValidationError: + continue + result[key] = validated.model_dump(mode="json")As per coding guidelines, "Validate at system boundaries (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/templates/preset_service.py` around lines 433 - 451, The loop currently only checks JSON and dict-ness and then adds decoded rows to result; instead call the existing _validate_personality_config(decoded) after the isinstance check and skip rows that fail schema validation, logging PRESET_VALIDATION_FAILED with preset_name=row.name and reason="invalid_schema_in_fetch_map" (or include validation details if _validate_personality_config returns them) so only validated PersonalityConfig dicts are inserted into result[key]; reference the variables rows, row, decoded, result and the helper _validate_personality_config to locate where to add this check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/templates/test_presets.py`:
- Around line 118-134: The test test_custom_presets_returns_copy currently only
verifies the outer dict is copied; modify the custom preset passed to
get_personality_preset to include a JSON-like mutable nested field (e.g., a list
or dict inside the "my_custom" preset) and then mutate that nested field on one
result (returned from get_personality_preset) and assert the other result's
nested field is unchanged; keep the existing assertions (a == b and a is not b)
and add an assertion that the nested mutable field is not shared to ensure a
deep/shallow-copy bug would be caught.
---
Duplicate comments:
In `@src/synthorg/templates/preset_service.py`:
- Around line 433-451: The loop currently only checks JSON and dict-ness and
then adds decoded rows to result; instead call the existing
_validate_personality_config(decoded) after the isinstance check and skip rows
that fail schema validation, logging PRESET_VALIDATION_FAILED with
preset_name=row.name and reason="invalid_schema_in_fetch_map" (or include
validation details if _validate_personality_config returns them) so only
validated PersonalityConfig dicts are inserted into result[key]; reference the
variables rows, row, decoded, result and the helper _validate_personality_config
to locate where to add this check.
🪄 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: f420fb0a-f7ee-47d4-87fc-17aa574f24cd
📒 Files selected for processing (13)
docs/design/organization.mdsrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/api/controllers/setup_helpers.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/observability/events/setup.pysrc/synthorg/templates/__init__.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/preset_service.pysrc/synthorg/templates/presets.pysrc/synthorg/templates/renderer.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_presets.pytests/unit/templates/test_renderer_validation.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for multiple exception types in except clauses — PEP 758 style, enforced by ruff on Python 3.14
Use line length of 88 characters, enforced by ruff
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/templates/__init__.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/controllers/setup_personality.pytests/unit/templates/test_presets.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.pysrc/synthorg/api/controllers/setup_helpers.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_renderer_validation.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: All public functions and classes in Python must have type hints; mypy strict mode is enforced
Use Google style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones for immutability. 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 with conventions:allow_inf_nan=Falsein allConfigDictdeclarations,@computed_fieldfor derived values instead of storing redundant fields,NotBlankStrfromcore.typesfor all identifier/name fields including optional and tuple variants
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Maintain 80% minimum test coverage; enforced in CI
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, orlarge/medium/smallaliases
Apply mypy type-checking in pre-push hooks for Python code
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/templates/__init__.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.pysrc/synthorg/api/controllers/setup_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code
Always useloggeras the variable name for loggers (not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g., API_REQUEST_STARTED from events.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
DEBUG logging is appropriate for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not need logging
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/templates/__init__.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/controllers/setup_personality.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.pysrc/synthorg/api/controllers/setup_helpers.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Apply ruff check+format in pre-commit hooks to ensure consistent Python code style
Files:
src/synthorg/observability/events/setup.pysrc/synthorg/templates/__init__.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/controllers/setup_personality.pytests/unit/templates/test_presets.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.pysrc/synthorg/api/controllers/setup_helpers.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_renderer_validation.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect the approved deviation from the design spec
Files:
docs/design/organization.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test categorization
Global test timeout is 30 seconds per test (in pyproject.toml) — do not add per-file markers unless overriding with longer timeouts liketimeout(60)
Prefer@pytest.mark.parametrizefor testing similar cases
Usetest-provider,test-small-001, etc. in tests instead of real vendor names
Use Hypothesis for property-based testing with@given+@settings. UseHYPOTHESIS_PROFILEenv var to select profiles:ci(50 examples, default) ordev(1000 examples)
Never skip, dismiss, or ignore flaky tests. Always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic. For indefinite blocking tasks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/templates/test_presets.pytests/unit/api/controllers/test_setup_agents.pytests/unit/templates/test_renderer_validation.py
🧠 Learnings (21)
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {docs/design/operations.md,src/synthorg/providers/presets.py,.claude/**/*.{md,yml,yaml}} : Vendor names may appear only in: (1) Operations design page, (2) `.claude/` skill/agent files, (3) third-party import paths, (4) provider presets (`src/synthorg/providers/presets.py`), (5) tests using `test-provider`, `test-small-001`, etc.
Applied to files:
docs/design/organization.md
📚 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 src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
docs/design/organization.mdsrc/synthorg/templates/__init__.pysrc/synthorg/templates/presets.pysrc/synthorg/api/controllers/setup_agents.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.pytests/unit/templates/test_renderer_validation.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/templates/__init__.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/templates/__init__.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/templates/preset_service.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/api/controllers/setup_personality.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/setup_personality.pysrc/synthorg/templates/_preset_resolution.pysrc/synthorg/templates/renderer.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/setup_personality.pysrc/synthorg/templates/_preset_resolution.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/api/controllers/setup_personality.pysrc/synthorg/templates/_preset_resolution.py
📚 Learning: 2026-03-31T22:07:03.782Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T22:07:03.782Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/setup_personality.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/api/controllers/setup_personality.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/api/controllers/setup_personality.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Applied to files:
src/synthorg/templates/presets.pysrc/synthorg/templates/renderer.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/presets.pysrc/synthorg/templates/renderer.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-31T22:07:03.782Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T22:07:03.782Z
Learning: Applies to src/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/api/controllers/setup_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 : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/engine/coordination/**/*.py : Task coordination uses multi-agent pipeline with 4 dispatchers (SAS/centralized/decentralized/context-dependent), wave execution, and workspace lifecycle integration.
Applied to files:
src/synthorg/api/controllers/setup_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/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/api/controllers/setup_helpers.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/api/controllers/test_setup_agents.py
🔇 Additional comments (20)
src/synthorg/templates/presets.py (4)
468-499: Advisory function not wired into application code.The
validate_preset_referencesfunction is well-implemented and tested, but per the past review comment, it has no callers in the application code. If template import/export operations should surface preset warnings during setup, consider wiring this validation into appropriate controller methods.
8-16: LGTM!The imports are well-organized:
copyadded fordeepcopyusageTYPE_CHECKINGguard properly used forMappingandCompanyTemplateto avoid runtime import overhead
413-446: LGTM!The
get_personality_presetfunction changes are well-implemented:
- Custom presets take precedence over builtins (line 433-434)
copy.deepcopyused for custom presets to prevent mutation at system boundariesdict()for builtin presets is sufficient sinceMappingProxyTypevalues are immutable- The "available" list includes custom preset names when provided (line 438-439)
- Warning logged before raising
KeyErrorwith context
543-547: LGTM!The
except MemoryError, RecursionError:syntax correctly uses PEP 758 style (no parentheses) as required by the coding guidelines for Python 3.14.tests/unit/api/controllers/test_setup_agents.py (3)
1-11: LGTM!Imports and module setup are correct. The test file properly imports the functions under test and the expected error type.
110-149: LGTM!The
TestExpandTemplateAgentsCustomPresetsclass provides good coverage for custom preset resolution behavior:
test_custom_preset_resolved: verifies custom presets are used when passedtest_unknown_preset_falls_back_to_pragmatic_builder: confirms fallback behavior for unknown presetstest_builtin_preset_works_with_custom_presets: ensures builtins still resolve when custom presets are presentThe test at line 143 uses an incomplete custom preset dict (
{"other": {"traits": ("a",)}}), but this is acceptable since the test only verifies that the builtin preset"pragmatic_builder"resolves correctly despitecustom_presetsbeing present.
152-195: LGTM!The
TestBuildAgentConfigCustomPresetsclass properly tests:
- Builtin preset resolution via
build_agent_config- Custom preset resolution when
custom_presetsis passedApiValidationErrorraised for unknown presets with appropriate message matchingThe
_make_requesthelper uses compliant test names (test-provider,test-small-001) per coding guidelines.tests/unit/templates/test_renderer_validation.py (4)
1-19: LGTM!Module imports and type checking setup are correct. The
TYPE_CHECKINGguard forTemplateFileFactoryis appropriate for avoiding circular imports.
38-105: LGTM!The
TestValidatePresetReferencesclass provides thorough coverage:
- Valid builtin presets produce no warnings
- Missing presets produce no warnings
- Unknown presets produce appropriate warnings with preset name and agent role
- Custom presets suppress warnings when matched
- Multiple unknown presets are all reported
- Case-insensitive validation works
- Return type is a tuple
Test assertions are precise and match the expected behavior of
validate_preset_references.
110-134: LGTM!The
TestUnknownPresetWarningclass correctly verifies the new warn-not-error behavior:
- Unknown presets result in omitted
personalitykey (not an error)- The agent dict is still returned with the correct role
Testing internal
_expand_single_agentdirectly is acceptable here since it's the unit responsible for the warn-and-skip behavior.
139-256: LGTM!The
TestCustomPresetResolutionclass provides comprehensive end-to-end coverage:
- Custom preset resolution during single-agent expansion
- Custom preset threading through
render_templatewith YAML templates- Unknown preset behavior in full render (personality is None or empty)
- Builtin presets still work when custom_presets dict is passed
The YAML template content in the fixture tests is well-formed and exercises the custom preset path through the entire rendering pipeline.
src/synthorg/api/controllers/setup_agents.py (5)
17-25: LGTM!The new imports are correctly added:
SETUP_PRESET_NOT_FOUNDevent constant for loggingMappinginTYPE_CHECKINGblock for type annotation
41-68: LGTM!The
expand_template_agentsfunction signature and docstring are updated correctly:
custom_presetsis keyword-only (after*)- Docstring documents the new parameter and its precedence behavior
- Raises section updated appropriately
90-106: LGTM!The preset resolution logic correctly:
- Threads
custom_presetstoget_personality_preset- Falls back to
"pragmatic_builder"onKeyError(setup path behavior)- Logs
SETUP_TEMPLATE_INVALIDbefore fallback
244-275: LGTM!The
build_agent_configfunction changes are well-implemented:
custom_presetsparameter is keyword-onlyKeyErrorcaught and converted toApiValidationErrorwith clear messagefrom Nonesuppresses exception chaining for cleaner API errors- Warning logged before raising per coding guidelines
312-318: LGTM!The
except MemoryError, RecursionError:syntax correctly uses PEP 758 style as required for Python 3.14.src/synthorg/templates/_preset_resolution.py (4)
1-25: LGTM!Module setup is correct:
- Docstring clearly marks this as an internal module
- Imports are appropriate for the functionality
- Logger properly initialized with
get_logger(__name__)
28-70: LGTM!The
resolve_agent_personalityfunction correctly handles inline personality:
- Validates that inline personality is a dict (type check at line 57)
- Calls
_validate_inline_personalityfor schema validation- Returns a copy of the inline personality via
dict(inline_personality)- Logs appropriate error events before raising
TemplateRenderError
71-94: Double warning on unknown preset is now intentional for context enrichment.The warning at lines 81-85 adds agent-specific context (
agent=name) thatget_personality_presetdoesn't have. Whileget_personality_presetlogsTEMPLATE_PERSONALITY_PRESET_UNKNOWNwithpreset_nameandavailablepresets, this warning adds the agent name for easier debugging. Theexc_info=Truewas removed per the past review, making this acceptable.The
is_customcheck at line 74 before thetryblock is a minor optimization that avoids redundant lookups when logging the custom preset resolution at lines 87-92.
97-119: LGTM!The
_validate_inline_personalityhelper is well-structured:
- Validates against
PersonalityConfigschema- Catches both
ValidationErrorandTypeError- Logs warning with context before raising
- Chains the original exception for debugging
- Remove duplicate logger.warning in resolve_agent_personality (the warning is already emitted by get_personality_preset before raising) - Remove unused TEMPLATE_PERSONALITY_PRESET_UNKNOWN import - Strengthen test_custom_presets_returns_copy to verify nested mutable fields are not shared (uses list instead of tuple for traits) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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/_preset_resolution.py`:
- Line 69: The code returns a shallow copy via dict(inline_personality) which
leaves nested mutables shared; change the return to a deep copy by using
copy.deepcopy(inline_personality) and add an import for the copy module at top
of the file; update the function that currently returns dict(inline_personality)
(in src/synthorg/templates/_preset_resolution.py) to return
copy.deepcopy(inline_personality) so callers get an independent, immutable copy.
- Around line 70-73: The code calls .strip() on preset_name without verifying
it's a string; update the block around preset_name (used to compute key and
is_custom) to first validate isinstance(preset_name, str) (allowing None to
skip), and if it's not a string log a warning with context (include the
offending value and template/context identifiers) and raise TemplateRenderError
so the error path is logged before raising; keep the existing logic for valid
strings (key = preset_name.strip().lower(), is_custom = custom_presets is not
None and key in custom_presets).
🪄 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: 9bc6cf7f-b5e5-4c27-932c-2674e20f1fee
📒 Files selected for processing (2)
src/synthorg/templates/_preset_resolution.pytests/unit/templates/test_presets.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 Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Test (Python 3.14)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for multiple exception types in except clauses — PEP 758 style, enforced by ruff on Python 3.14
Use line length of 88 characters, enforced by ruff
Files:
tests/unit/templates/test_presets.pysrc/synthorg/templates/_preset_resolution.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test categorization
Global test timeout is 30 seconds per test (in pyproject.toml) — do not add per-file markers unless overriding with longer timeouts liketimeout(60)
Prefer@pytest.mark.parametrizefor testing similar cases
Usetest-provider,test-small-001, etc. in tests instead of real vendor names
Use Hypothesis for property-based testing with@given+@settings. UseHYPOTHESIS_PROFILEenv var to select profiles:ci(50 examples, default) ordev(1000 examples)
Never skip, dismiss, or ignore flaky tests. Always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic. For indefinite blocking tasks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/templates/test_presets.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Apply ruff check+format in pre-commit hooks to ensure consistent Python code style
Files:
tests/unit/templates/test_presets.pysrc/synthorg/templates/_preset_resolution.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: All public functions and classes in Python must have type hints; mypy strict mode is enforced
Use Google style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones for immutability. 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 with conventions:allow_inf_nan=Falsein allConfigDictdeclarations,@computed_fieldfor derived values instead of storing redundant fields,NotBlankStrfromcore.typesfor all identifier/name fields including optional and tuple variants
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Maintain 80% minimum test coverage; enforced in CI
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, orlarge/medium/smallaliases
Apply mypy type-checking in pre-push hooks for Python code
Files:
src/synthorg/templates/_preset_resolution.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. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code
Always useloggeras the variable name for loggers (not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g., API_REQUEST_STARTED from events.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
DEBUG logging is appropriate for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not need logging
Files:
src/synthorg/templates/_preset_resolution.py
🧠 Learnings (12)
📚 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/templates/test_presets.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:
tests/unit/templates/test_presets.py
📚 Learning: 2026-03-31T22:07:03.782Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T22:07:03.782Z
Learning: Applies to src/**/*.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/templates/test_presets.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:
tests/unit/templates/test_presets.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/templates/test_presets.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/templates/test_presets.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Applied to files:
tests/unit/templates/test_presets.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 src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
src/synthorg/templates/_preset_resolution.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/templates/_preset_resolution.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/templates/_preset_resolution.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/templates/_preset_resolution.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/templates/_preset_resolution.py
🔇 Additional comments (2)
tests/unit/templates/test_presets.py (1)
101-207: Excellent test coverage for custom preset functionality.The seven new tests comprehensively cover the custom_presets parameter behavior:
- test_custom_presets_returns_deep_copy (lines 118-142) properly verifies deep copy isolation by mutating a nested mutable field and confirming both the source dict and subsequent lookups are unaffected. This would catch shallow-copy bugs.
- Resolution precedence, case-insensitivity, builtin fallback, error handling, and None-parameter behavior are all properly tested.
- Test naming is clear and descriptive.
- All edge cases (override builtin, unknown preset with custom presets) are covered.
src/synthorg/templates/_preset_resolution.py (1)
107-107: The code is correct. In Python 3.14 (PEP 758), when catching multiple exception types with anasbinding, parentheses are required:except (ValidationError, TypeError) as exc:is the mandated syntax. Omitting parentheses likeexcept ValidationError, TypeError as exc:is invalid.The repo guideline is incomplete—it should clarify that parentheses are required with
asbinding; the unparenthesized form only applies when no exception binding is used (e.g.,except ValidationError, TypeError:).
| if preset_name: | ||
| # Normalize once for both the lookup and the custom-source check. | ||
| key = preset_name.strip().lower() | ||
| is_custom = custom_presets is not None and key in custom_presets |
There was a problem hiding this comment.
Validate personality_preset type before calling .strip().
At Line 72, non-string personality_preset values raise AttributeError and bypass template error mapping/logging. Add explicit type validation and raise TemplateRenderError with warning context.
Suggested fix
- if preset_name:
+ if preset_name is not None and not isinstance(preset_name, str):
+ msg = (
+ f"personality_preset for agent {name!r} must be a string, "
+ f"got {type(preset_name).__name__}"
+ )
+ logger.warning(
+ TEMPLATE_RENDER_TYPE_ERROR,
+ agent=name,
+ field="personality_preset",
+ got=type(preset_name).__name__,
+ )
+ raise TemplateRenderError(msg)
+ if preset_name:
# Normalize once for both the lookup and the custom-source check.
key = preset_name.strip().lower()As per coding guidelines: "Validate at system boundaries (user input, external APIs, config files)" and "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/templates/_preset_resolution.py` around lines 70 - 73, The code
calls .strip() on preset_name without verifying it's a string; update the block
around preset_name (used to compute key and is_custom) to first validate
isinstance(preset_name, str) (allowing None to skip), and if it's not a string
log a warning with context (include the offending value and template/context
identifiers) and raise TemplateRenderError so the error path is logged before
raising; keep the existing logic for valid strings (key =
preset_name.strip().lower(), is_custom = custom_presets is not None and key in
custom_presets).
…ality Inline personality dicts from YAML can contain mutable nested values (e.g. lists for traits). Use copy.deepcopy() instead of dict() to prevent callers from mutating the source dict. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/synthorg/templates/_preset_resolution.py (1)
71-74:⚠️ Potential issue | 🟡 MinorValidate
personality_presettype before calling.strip().If
preset_nameis truthy but not a string (e.g., an integer or list from malformed YAML), calling.strip()raisesAttributeErrorthat bypasses proper error handling and logging.🛡️ Proposed fix to add type validation
+ if preset_name is not None and not isinstance(preset_name, str): + msg = ( + f"personality_preset for agent {name!r} must be a string, " + f"got {type(preset_name).__name__}" + ) + logger.warning( + TEMPLATE_RENDER_TYPE_ERROR, + agent=name, + field="personality_preset", + got=type(preset_name).__name__, + ) + raise TemplateRenderError(msg) if preset_name: # Normalize once for both the lookup and the custom-source check. key = preset_name.strip().lower()As per coding guidelines: "Validate at system boundaries (user input, external APIs, config files)" and "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/templates/_preset_resolution.py` around lines 71 - 74, preset_name may be truthy but not a str, so validate its type before calling .strip(): replace the current block that uses preset_name.strip() with a guard that checks isinstance(preset_name, str) (or allows None), and if the value is not a string log a WARNING/ERROR including the offending value and its type (and context like "personality_preset") before raising a TypeError; if it is a string then compute key = preset_name.strip().lower() and is_custom = custom_presets is not None and key in custom_presets as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/synthorg/templates/_preset_resolution.py`:
- Around line 71-74: preset_name may be truthy but not a str, so validate its
type before calling .strip(): replace the current block that uses
preset_name.strip() with a guard that checks isinstance(preset_name, str) (or
allows None), and if the value is not a string log a WARNING/ERROR including the
offending value and its type (and context like "personality_preset") before
raising a TypeError; if it is a string then compute key =
preset_name.strip().lower() and is_custom = custom_presets is not None and key
in custom_presets as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f8d46ad-fdd4-4375-abed-bb9ab794eca2
📒 Files selected for processing (1)
src/synthorg/templates/_preset_resolution.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Socket Security: Pull Request Alerts
- 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 annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for multiple exception types in except clauses — PEP 758 style, enforced by ruff on Python 3.14
Use line length of 88 characters, enforced by ruff
Files:
src/synthorg/templates/_preset_resolution.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: All public functions and classes in Python must have type hints; mypy strict mode is enforced
Use Google style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones for immutability. 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 with conventions:allow_inf_nan=Falsein allConfigDictdeclarations,@computed_fieldfor derived values instead of storing redundant fields,NotBlankStrfromcore.typesfor all identifier/name fields including optional and tuple variants
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Maintain 80% minimum test coverage; enforced in CI
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, orlarge/medium/smallaliases
Apply mypy type-checking in pre-push hooks for Python code
Files:
src/synthorg/templates/_preset_resolution.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. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code
Always useloggeras the variable name for loggers (not_logger, notlog)
Always use event name constants from domain-specific modules undersynthorg.observability.events(e.g., API_REQUEST_STARTED from events.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
DEBUG logging is appropriate for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do not need logging
Files:
src/synthorg/templates/_preset_resolution.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Apply ruff check+format in pre-commit hooks to ensure consistent Python code style
Files:
src/synthorg/templates/_preset_resolution.py
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
📚 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 src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
src/synthorg/templates/_preset_resolution.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/templates/_preset_resolution.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/templates/_preset_resolution.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/templates/_preset_resolution.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/templates/_preset_resolution.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/_preset_resolution.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/templates/_preset_resolution.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction and wrap with `MappingProxyType` for read-only enforcement
Applied to files:
src/synthorg/templates/_preset_resolution.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/templates/_preset_resolution.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/templates/_preset_resolution.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/templates/_preset_resolution.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/_preset_resolution.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/_preset_resolution.py
📚 Learning: 2026-03-31T22:07:03.782Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T22:07:03.782Z
Learning: Applies to src/**/*.py : Create new objects instead of mutating existing ones for immutability. For non-Pydantic internal collections (registries, BaseTool), use `copy.deepcopy()` at construction + `MappingProxyType` wrapping for read-only enforcement
Applied to files:
src/synthorg/templates/_preset_resolution.py
📚 Learning: 2026-03-31T22:07:03.782Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T22:07:03.782Z
Learning: Applies to src/**/*.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/templates/_preset_resolution.py
🔇 Additional comments (3)
src/synthorg/templates/_preset_resolution.py (3)
1-25: Module setup and imports look good.Logger is correctly initialized with
get_logger(__name__), imports are well-organized withTYPE_CHECKINGguard forMapping, and the module docstring clearly indicates internal use.
75-90: Preset lookup and KeyError handling are correct.The
except KeyErrorblock correctly returnsNonewithout duplicate logging sinceget_personality_preset()already logsTEMPLATE_PERSONALITY_PRESET_UNKNOWNbefore raising. Debug logging for custom preset resolution provides useful observability.
106-115: No change needed — current syntax is correct per PEP 758.The code at line 108 correctly uses parentheses:
except (ValidationError, TypeError) as exc:. PEP 758 requires parentheses around multiple exception types when using theasbinding clause. The unparenthesized syntaxexcept A, B:is valid only withoutas; binding withasrequiresexcept (A, B) as exc:.
🤖 I have created a release *beep* *boop* --- ## [0.5.5](v0.5.4...v0.5.5) (2026-04-01) ### Features * add workflow configs to builtin templates ([#963](#963)) ([b7fe6e3](b7fe6e3)) * implement Kanban board and Agile sprints workflow types ([#960](#960)) ([f511e1d](f511e1d)) * personality preset support in template YAML schema ([#959](#959)) ([97ca81e](97ca81e)) ### Documentation * LMEB embedding evaluation + CSP accepted risk ([#695](#695), [#925](#925)) ([#962](#962)) ([43dfab3](43dfab3)) ### CI/CD * bump wrangler from 4.78.0 to 4.79.0 in /.github in the all group across 1 directory ([#955](#955)) ([18b4cb1](18b4cb1)) ### Maintenance * bump mypy from 1.19.1 to 1.20.0 in the all group across 1 directory ([#956](#956)) ([29cc419](29cc419)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
custom_presets: Mapping[str, dict[str, Any]] | Nonethrough the entire renderer call chainvalidate_preset_references()public function for pre-flight validation of preset references in templatesfetch_custom_presets_map()async helper to bridge the async persistence layer and the sync rendering pipelinesetup_agents,setup_personality,setup_helpers,setup) to pre-fetch and pass custom presetsKeyErrorasApiValidationErrorin API endpoints for unknown presetsfetch_custom_presets_map(skip bad rows, log error)docs/design/organization.mdto document custom preset rendering behaviorTest plan
validate_preset_references,fetch_custom_presets_map(including corrupt JSON and key lowercasing),build_agent_config,expand_template_agentswith custom presetsuv run python -m pytest tests/ -n auto-- 11508 passeduv run ruff check src/ tests/-- cleanuv run mypy src/ tests/-- cleanCloses #757
🤖 Generated with Claude Code