feat: implement auxiliary tool categories -- design, communication, analytics#1152
Merged
feat: implement auxiliary tool categories -- design, communication, analytics#1152
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds three auxiliary tool categories: design, communication, and analytics. Introduces package entry points and abstract base classes for each category, immutable Pydantic config models (DesignToolsConfig, CommunicationToolsConfig, AnalyticsToolsConfig), and new observability event modules (design, communication, analytics). Implements concrete tools and protocols across categories (design: ImageGeneratorTool, DiagramGeneratorTool, AssetManagerTool; communication: EmailSenderTool, NotificationSenderTool, TemplateFormatterTool; analytics: DataAggregatorTool, ReportGeneratorTool, MetricCollectorTool). Extends RootConfig and default config, updates the tool factory to conditionally register these tools with optional injected backends, and adds comprehensive unit tests and docs updates. Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
Contributor
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces three opt-in auxiliary tool categories (design, communication, analytics) to the SynthOrg tool system, wiring them into the default tool factory and extending configuration + observability to support these new workflows.
Changes:
- Add new tool implementations for Design (image/diagram/asset registry), Communication (SMTP email, notification dispatch, template formatting), and Analytics (aggregation, reports, metric recording) with injectable provider/sink protocols.
- Extend tool factory and RootConfig/default config to conditionally construct these categories only when corresponding config is present.
- Add new/expanded observability event constants and comprehensive unit tests for tool behavior and config validation.
Reviewed changes
Copilot reviewed 43 out of 46 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/tools/test_factory.py | Adds factory-level tests ensuring new categories are skipped/included based on config presence. |
| tests/unit/tools/design/test_image_generator.py | Unit tests for ImageGeneratorTool behavior and provider injection/error paths. |
| tests/unit/tools/design/test_diagram_generator.py | Unit tests for Mermaid/Graphviz output generation and validation. |
| tests/unit/tools/design/test_config.py | Validation tests for DesignToolsConfig defaults/bounds/frozen behavior. |
| tests/unit/tools/design/test_asset_manager.py | Unit tests for AssetManagerTool CRUD/search semantics. |
| tests/unit/tools/design/conftest.py | Shared fixtures (MockImageProvider, configs) for design tool tests. |
| tests/unit/tools/design/init.py | Test package marker for design tool tests. |
| tests/unit/tools/communication/test_template_formatter.py | Unit tests for safe Jinja2 template rendering and format handling. |
| tests/unit/tools/communication/test_notification_sender.py | Unit tests for NotificationSenderTool validation/dispatch behavior. |
| tests/unit/tools/communication/test_email_sender.py | Unit tests for SMTP email tool validation and send paths (mocked). |
| tests/unit/tools/communication/test_config.py | Validation tests for EmailConfig/CommunicationToolsConfig (incl. credential pairing). |
| tests/unit/tools/communication/conftest.py | Shared fixtures (mock dispatcher, configs) for communication tool tests. |
| tests/unit/tools/communication/init.py | Test package marker for communication tool tests. |
| tests/unit/tools/analytics/test_report_generator.py | Unit tests for report generation formatting and provider error handling. |
| tests/unit/tools/analytics/test_metric_collector.py | Unit tests for metric recording, whitelist enforcement, and sink errors. |
| tests/unit/tools/analytics/test_data_aggregator.py | Unit tests for aggregation queries, param validation, and whitelist enforcement. |
| tests/unit/tools/analytics/test_config.py | Validation tests for AnalyticsToolsConfig defaults/bounds/frozen behavior. |
| tests/unit/tools/analytics/conftest.py | Shared fixtures (MockAnalyticsProvider/MockMetricSink) for analytics tests. |
| tests/unit/tools/analytics/init.py | Test package marker for analytics tool tests. |
| tests/unit/observability/test_events.py | Registers the new design events domain for discovery tests. |
| src/synthorg/tools/factory.py | Adds _build_design_tools/_build_communication_tools/_build_analytics_tools and wires them into default factories. |
| src/synthorg/tools/design/image_generator.py | Implements ImageGeneratorTool + ImageProvider protocol + ImageResult model. |
| src/synthorg/tools/design/diagram_generator.py | Implements DiagramGeneratorTool for Mermaid/Graphviz DSL output. |
| src/synthorg/tools/design/config.py | Adds DesignToolsConfig model (timeouts/size limits/storage path). |
| src/synthorg/tools/design/base_design_tool.py | Adds BaseDesignTool with shared config and ToolCategory.DESIGN. |
| src/synthorg/tools/design/asset_manager.py | Implements in-memory AssetManagerTool with list/get/delete/search. |
| src/synthorg/tools/design/init.py | Exposes design tool public API exports. |
| src/synthorg/tools/communication/template_formatter.py | Implements TemplateFormatterTool using Jinja2 SandboxedEnvironment. |
| src/synthorg/tools/communication/notification_sender.py | Implements NotificationSenderTool delegating to NotificationDispatcherProtocol. |
| src/synthorg/tools/communication/email_sender.py | Implements EmailSenderTool using smtplib + asyncio.to_thread with basic injection defenses. |
| src/synthorg/tools/communication/config.py | Adds EmailConfig and CommunicationToolsConfig models. |
| src/synthorg/tools/communication/base_communication_tool.py | Adds BaseCommunicationTool with shared config and ToolCategory.COMMUNICATION. |
| src/synthorg/tools/communication/init.py | Exposes communication tool public API exports. |
| src/synthorg/tools/analytics/report_generator.py | Implements ReportGeneratorTool to format provider data into text/markdown/json. |
| src/synthorg/tools/analytics/metric_collector.py | Implements MetricCollectorTool + MetricSink protocol with whitelist enforcement. |
| src/synthorg/tools/analytics/data_aggregator.py | Implements DataAggregatorTool + AnalyticsProvider protocol with period/date validation and whitelist enforcement. |
| src/synthorg/tools/analytics/config.py | Adds AnalyticsToolsConfig model (query timeout/max rows/metric whitelist). |
| src/synthorg/tools/analytics/base_analytics_tool.py | Adds BaseAnalyticsTool with shared config and metric whitelist helper. |
| src/synthorg/tools/analytics/init.py | Exposes analytics tool public API exports. |
| src/synthorg/observability/events/design.py | Adds new design tool event constants. |
| src/synthorg/observability/events/communication.py | Adds tool-specific email/notification/template event constants. |
| src/synthorg/observability/events/analytics.py | Adds tool-specific analytics query/report/metric event constants. |
| src/synthorg/config/schema.py | Extends RootConfig with design_tools, communication_tools, analytics_tools. |
| src/synthorg/config/defaults.py | Adds new RootConfig fields to default config dict. |
| docs/reference/claude-reference.md | Updates repository reference to include new tool directories. |
| CLAUDE.md | Updates logging/event-constants guidance to include new event modules/constants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+199
to
+203
| try: | ||
| result = await self._provider.generate( | ||
| prompt=prompt, | ||
| width=width, | ||
| height=height, |
There was a problem hiding this comment.
Image generation currently ignores DesignToolsConfig.image_timeout/max_image_size_bytes: provider.generate is awaited without a timeout and the returned base64 payload isn't size-checked. Consider wrapping the provider call with asyncio.wait_for(self._config.image_timeout) (and logging DESIGN_IMAGE_GENERATION_TIMEOUT) and validating/limiting the decoded byte size against max_image_size_bytes before returning.
Comment on lines
+95
to
+99
| name="asset_manager", | ||
| description=("List, retrieve, delete, and search generated design assets."), | ||
| parameters_schema=copy.deepcopy(_PARAMETERS_SCHEMA), | ||
| action_type=ActionType.CODE_READ, | ||
| config=config, |
There was a problem hiding this comment.
AssetManagerTool sets action_type=ActionType.CODE_READ, but the tool supports destructive operations (delete) and mutates internal state. This under-classifies the tool for security/autonomy gating; consider using a write/destructive action type (or at least the DESIGN default ActionType.DOCS_WRITE) so deletes aren't treated as read-only.
Comment on lines
+259
to
+263
| try: | ||
| data = await self._provider.query( | ||
| metrics=metrics, | ||
| period=period, | ||
| group_by=group_by, |
There was a problem hiding this comment.
AnalyticsToolsConfig.query_timeout/max_rows are currently unused here: provider.query is awaited directly, so a slow provider can hang the tool and large result sets aren't constrained. Consider enforcing query_timeout (e.g., asyncio.wait_for) and either passing max_rows to the provider or validating/truncating results based on config.max_rows.
Comment on lines
+183
to
+187
| try: | ||
| data = await self._provider.query( | ||
| metrics=metrics, | ||
| period=period, | ||
| ) |
There was a problem hiding this comment.
AnalyticsToolsConfig.query_timeout/max_rows are also unused in report generation: provider.query is awaited without a timeout and formatting assumes bounded data. Consider applying the same timeout/row-limit enforcement here (or delegating to a shared helper) to avoid long-running queries and oversized reports.
Comment on lines
+261
to
+265
| msg = EmailMessage() | ||
| msg["Subject"] = safe_subject | ||
| msg["From"] = email_config.from_address | ||
| msg["To"] = ", ".join(to_addrs) | ||
| if cc_addrs: |
There was a problem hiding this comment.
Recipient addresses are checked for CR/LF, but email_config.from_address is written directly into the From header without similar validation/sanitization. Consider rejecting CR/LF (and ideally other control chars) in from_address before assigning msg['From'] to prevent header injection via configuration.
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces three new tool categories—analytics, communication, and design—along with their respective configurations, event constants, and comprehensive unit tests. Key additions include tools for data aggregation, report generation, email and notification dispatch, template formatting, and design asset management. A critical syntax error was identified across multiple files where multiple exceptions are caught using Python 2 syntax, which will cause runtime failures in Python 3. Additionally, feedback was provided regarding the integration between the image generator and asset manager, improvements to email header handling, and the need for implicit TLS support in the email sender.
| start_date=start_date, | ||
| end_date=end_date, | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| tags=tags, | ||
| unit=unit, | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| metrics=metrics, | ||
| period=period, | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| body=body, | ||
| body_is_html=body_is_html, | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
|
|
||
| try: | ||
| await self._dispatcher.dispatch(notification) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| markup = self._generate_mermaid(diagram_type, description, title) | ||
| else: | ||
| markup = self._generate_graphviz(diagram_type, description, title) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| style=style, | ||
| quality=quality, | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
The except clause uses invalid syntax for catching multiple exceptions in Python 3. Multiple exceptions must be provided as a tuple: except (MemoryError, RecursionError):. The current syntax will cause a SyntaxError at runtime.
Suggested change
| except MemoryError, RecursionError: | |
| except (MemoryError, RecursionError): |
| } | ||
|
|
||
|
|
||
| class ImageGeneratorTool(BaseDesignTool): |
Contributor
There was a problem hiding this comment.
The ImageGeneratorTool does not register generated images with the AssetManagerTool, despite the documentation in AssetManagerTool stating that it is called programmatically by other tools. This makes the AssetManagerTool registry ineffective for tracking generated images automatically.
Comment on lines
+265
to
+267
| if cc_addrs: | ||
| msg["Cc"] = ", ".join(cc_addrs) | ||
|
|
Contributor
There was a problem hiding this comment.
When setting headers like To and Cc in EmailMessage, it is better to pass the list of addresses directly rather than joining them into a string. The library handles the necessary formatting and encoding of multiple addresses automatically.
msg["To"] = to_addrs
if cc_addrs:
msg["Cc"] = cc_addrs| else: | ||
| msg.set_content(body) | ||
|
|
||
| with smtplib.SMTP(email_config.host, email_config.port, timeout=10) as smtp: |
Contributor
There was a problem hiding this comment.
The current implementation uses smtplib.SMTP which is intended for plain SMTP or STARTTLS (usually on port 587). If the SMTP server requires implicit TLS (usually on port 465), this will fail. Consider using smtplib.SMTP_SSL when the port is 465 or adding a configuration option for implicit TLS.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1152 +/- ##
==========================================
- Coverage 88.91% 88.87% -0.05%
==========================================
Files 860 879 +19
Lines 50695 51478 +783
Branches 5087 5171 +84
==========================================
+ Hits 45074 45749 +675
- Misses 4654 4744 +90
- Partials 967 985 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/synthorg/config/schema.py (1)
547-587:⚠️ Potential issue | 🟡 MinorUpdate docstring Attributes section.
The
RootConfigdocstring's Attributes section (ending at line 587) doesn't document the newdesign_tools,communication_tools, andanalytics_toolsfields. As per coding guidelines, public classes require Google-style docstrings with complete attribute documentation.📝 Suggested docstring additions (before line 587)
terminal: Terminal tool configuration (``None`` = default terminal config). + design_tools: Design tool configuration (``None`` = disabled). + communication_tools: Communication tool configuration + (``None`` = disabled). + analytics_tools: Analytics tool configuration (``None`` = disabled). """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/config/schema.py` around lines 547 - 587, The RootConfig class docstring's Attributes section is missing documentation for the new public fields design_tools, communication_tools, and analytics_tools; update the Google-style docstring for RootConfig to add concise entries for these three attributes (describe type and purpose similarly to existing entries), placing them alongside other top-level attributes in the Attributes list and using the same formatting as fields like providers and workflow so the docstring remains complete and consistent.
🤖 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/tools/analytics/metric_collector.py`:
- Around line 137-144: The code path that returns a ToolExecutionResult when
self._sink is None must log a warning beforehand; in the MetricCollector (or the
method where this check lives) add a warning log statement (e.g.,
self._logger.warning or the module logger) with context like "Metric recording
requires a configured sink" and include identifying details (class/method name
and any request/metric identifiers available) immediately before returning the
ToolExecutionResult so the missing-sink error path mirrors the
metric-not-allowed case logging.
In `@src/synthorg/tools/analytics/report_generator.py`:
- Around line 152-154: After extracting period from arguments (period: str =
arguments["period"]), add the same runtime validation used for report_type and
output_format: check that period is one of the allowed values in _VALID_PERIODS
and if not raise a ValueError (or the same exception type used for other invalid
params) with a clear message that includes the invalid value and the allowed
values; place this check immediately after the period assignment to mirror the
existing checks for report_type and output_format.
In `@src/synthorg/tools/communication/email_sender.py`:
- Around line 273-278: The SMTP client is using a hardcoded timeout (10s) and
there's a potential secret exposure risk for email_config.password; add a
timeout attribute (e.g., timeout_seconds or smtp_timeout) to the EmailConfig
dataclass/class with a sensible default and replace the literal 10 in the
smtplib.SMTP(...) call in email_sender.py with email_config.smtp_timeout, and
ensure any logging paths in the module (or code that handles errors around
smtplib.SMTP, smtp.starttls, or smtp.login) never include email_config.password
(mask or omit the password when logging and avoid printing the entire
email_config object).
In `@src/synthorg/tools/communication/notification_sender.py`:
- Around line 141-148: The code path in NotificationSender where
self._dispatcher is None returns a ToolExecutionResult error without logging;
add a WARNING-level log immediately before the return to match observability
patterns. Specifically, in the method inside NotificationSender that checks
self._dispatcher, call the module/class logger (e.g., self._logger or the
existing logger instance) to log a warning message indicating the
NotificationDispatcher is not configured and include the same contextual text as
the returned ToolExecutionResult (or reuse the
ANALYTICS_TOOL_PROVIDER_NOT_CONFIGURED-style constant/message for consistency),
then return the ToolExecutionResult as before.
In `@src/synthorg/tools/design/asset_manager.py`:
- Around line 275-280: Replace the reused DESIGN_ASSET_LISTED event with a
dedicated event constant for searches: define a new constant named
DESIGN_ASSET_SEARCHED (matching the naming style of existing event constants)
and use it in the logger.info call inside the search handler (the logger.info
invocation that currently passes search_query rather than filter_tags in
asset_manager.py), so the call becomes logger.info(DESIGN_ASSET_SEARCHED,
total=..., matched=..., search_query=...); leave other uses of
DESIGN_ASSET_LISTED unchanged.
In `@src/synthorg/tools/design/diagram_generator.py`:
- Around line 219-225: The title is user-provided and must be sanitized before
being interpolated into the Mermaid/DOT output; update the code that builds the
diagram string (the block that checks `if title:` and appends the frontmatter
and uses `title`) to (1) strip/control-character-clean the title by removing or
replacing `\r` and `\n` with a single space, (2) neutralize leading YAML
frontmatter markers like `---` (e.g., replace any occurrence of `---` with a
safe alternative or insert a zero-width space), and (3) escape or quote
characters that could break YAML/Mermaid (such as `:` at start, unbalanced
quotes) so the resulting frontmatter line `title: ...` cannot inject extra
frontmatter or attributes; apply the same sanitization logic wherever `title` is
used later (the other append location referenced in the diff). Ensure the
sanitized variable replaces `title` in the existing `lines.append(f"title:
{title}")` call.
- Around line 132-148: The validation branches for diagram_type and
output_format currently return a ToolExecutionResult error without logging;
before returning from the checks in generate_diagram (or the function containing
these branches), emit a warning-level log that includes the rejected value and
allowed set and also log the DESIGN_DIAGRAM_GENERATION_FAILED event (or call the
existing error metric/logger) with context; update the branches that check
diagram_type against _DIAGRAM_TYPES and output_format against _OUTPUT_FORMATS so
they call the module logger (or metrics) to record the invalid value, the
expected options, and any request id/context, then return the same
ToolExecutionResult as before.
In `@src/synthorg/tools/design/image_generator.py`:
- Around line 184-188: Validate the extracted style and quality values in
execute(): after reading style = arguments.get("style", "realistic") and quality
= arguments.get("quality", "standard"), check them against the allowed enums
(style ∈ {"realistic","sketch","diagram","icon"} and quality ∈
{"draft","standard","high"}); if a value is invalid, raise a clear ValueError
(or fallback to the default) with a descriptive message including the invalid
value and allowed options so the provider never receives unexpected strings.
Ensure the checks live near the top of execute() where
prompt/style/width/height/quality are declared and use the variable names style
and quality in the error messages.
In `@src/synthorg/tools/factory.py`:
- Around line 182-185: The factory currently always registers
ImageGeneratorTool, NotificationSenderTool, DataAggregatorTool, and
MetricCollectorTool even when their backends (image_provider, dispatcher,
provider, metric_sink) are None; update the builder to only include each tool in
the returned tuple when its required backend is present (e.g., wrap creation of
ImageGeneratorTool in an if image_provider is not None, similarly gate
NotificationSenderTool on dispatcher, DataAggregatorTool on provider,
MetricCollectorTool on metric_sink) and otherwise omit the tool or emit a
factory-level warning; apply the same gating to the other similar builder return
blocks in this module (the blocks analogous to the one that constructs
ImageGeneratorTool/DiagramGeneratorTool/AssetManagerTool) so the registry never
advertises tools that will fail at runtime.
In `@tests/unit/tools/communication/test_email_sender.py`:
- Around line 60-64: The test test_execute_too_many_recipients is using
pytest.importorskip for synthorg.tools.communication.config which masks
failures; remove the pytest.importorskip call and reference EmailConfig directly
when constructing CommunicationToolsConfig, and instead add a real import for
EmailConfig at the top of the test module (so the module import fails fast).
Update the test to use EmailConfig directly in the call site where
CommunicationToolsConfig(...) is created and delete the importorskip usage.
---
Outside diff comments:
In `@src/synthorg/config/schema.py`:
- Around line 547-587: The RootConfig class docstring's Attributes section is
missing documentation for the new public fields design_tools,
communication_tools, and analytics_tools; update the Google-style docstring for
RootConfig to add concise entries for these three attributes (describe type and
purpose similarly to existing entries), placing them alongside other top-level
attributes in the Attributes list and using the same formatting as fields like
providers and workflow so the docstring remains complete and 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: 696723ae-18cc-4802-b47a-777e1c06155d
📒 Files selected for processing (46)
CLAUDE.mddocs/reference/claude-reference.mdsrc/synthorg/config/defaults.pysrc/synthorg/config/schema.pysrc/synthorg/observability/events/analytics.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/analytics/base_analytics_tool.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/communication/template_formatter.pysrc/synthorg/tools/design/__init__.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/base_design_tool.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/factory.pytests/unit/observability/test_events.pytests/unit/tools/analytics/__init__.pytests/unit/tools/analytics/conftest.pytests/unit/tools/analytics/test_config.pytests/unit/tools/analytics/test_data_aggregator.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/__init__.pytests/unit/tools/communication/conftest.pytests/unit/tools/communication/test_config.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/communication/test_notification_sender.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/__init__.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/test_factory.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). (5)
- GitHub Check: Agent
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
No
from __future__ import annotations-- Python 3.14 has PEP 649PEP 758 except syntax: use
except A, B:(no parentheses) -- ruff enforces this on Python 3.14Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,
BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundariesConfig vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using
model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one modelModels: Pydantic v2 (
BaseModel,model_validator,computed_field,ConfigDict). Useallow_inf_nan=Falsein allConfigDictdeclarations; use@computed_fieldfor derived values instead of storing + validating redundant fields; useNotBlankStrfor all identifier/name fieldsAsync concurrency: prefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new code. Prefer structured concurrency over barecreate_taskLine length: 88 characters (ruff)
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Validate: at system boundaries (user input, external APIs, config files)
Variable name: always
logger(not_logger, notlog)Structured kwargs: always
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 for object creation, internal flow, entry/exit of key functions
Files:
tests/unit/observability/test_events.pytests/unit/tools/test_factory.pytests/unit/tools/design/test_config.pysrc/synthorg/tools/design/__init__.pysrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/observability/events/communication.pysrc/synthorg/config/schema.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/analytics/base_analytics_tool.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/test_notification_sender.pysrc/synthorg/tools/design/base_design_tool.pytests/unit/tools/communication/test_email_sender.pysrc/synthorg/observability/events/design.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/communication/test_template_formatter.pysrc/synthorg/tools/analytics/config.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/conftest.pysrc/synthorg/tools/communication/template_formatter.pytests/unit/tools/analytics/test_data_aggregator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/report_generator.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/conftest.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/config/defaults.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/factory.pytests/unit/tools/design/test_diagram_generator.pysrc/synthorg/observability/events/analytics.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/analytics/metric_collector.pytests/unit/tools/communication/test_config.pytests/unit/tools/communication/conftest.pysrc/synthorg/tools/design/image_generator.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Markers:
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowParametrize: Prefer
@pytest.mark.parametrizefor testing similar casesTests must use
test-provider,test-small-001, etc. instead of real vendor namesProperty-based testing: Python uses Hypothesis (
@given+@settings). Hypothesis profiles configured intests/conftest.py:ci(10 examples, derandomized),dev(1000 examples),fuzz(10,000 examples, no deadline),extreme(500,000 examples, no deadline). Controlled viaHYPOTHESIS_PROFILEenv varFlaky tests: NEVER skip, dismiss, or ignore flaky tests -- always fix them fully. For timing-sensitive tests, mock
time.monotonic()andasyncio.sleep(). For blocking tasks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/observability/test_events.pytests/unit/tools/test_factory.pytests/unit/tools/design/test_config.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/test_notification_sender.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/conftest.pytests/unit/tools/analytics/test_data_aggregator.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/conftest.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/communication/test_config.pytests/unit/tools/communication/conftest.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/observability/test_events.pytests/unit/tools/test_factory.pytests/unit/tools/design/test_config.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/test_notification_sender.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/conftest.pytests/unit/tools/analytics/test_data_aggregator.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/conftest.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/communication/test_config.pytests/unit/tools/communication/conftest.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Every module with business logic MUST have:
from synthorg.observability import get_loggerthenlogger = get_logger(__name__)Never use
import logging/logging.getLogger()/print()in application code. Exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py,observability/http_handler.py, andobservability/otlp_handler.pyEvent names: always use constants from domain-specific modules under
synthorg.observability.events. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANTVendor-agnostic everywhere: NEVER use real vendor names in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001. Vendor names only appear in: Operations design page,.claude/files, third-party imports, and provider presets
Files:
src/synthorg/tools/design/__init__.pysrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/observability/events/communication.pysrc/synthorg/config/schema.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/analytics/base_analytics_tool.pysrc/synthorg/tools/design/base_design_tool.pysrc/synthorg/observability/events/design.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/communication/template_formatter.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/config/defaults.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/factory.pysrc/synthorg/observability/events/analytics.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/image_generator.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/tools/design/__init__.pysrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/observability/events/communication.pysrc/synthorg/config/schema.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/analytics/base_analytics_tool.pysrc/synthorg/tools/design/base_design_tool.pysrc/synthorg/observability/events/design.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/communication/template_formatter.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/config/defaults.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/factory.pysrc/synthorg/observability/events/analytics.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/image_generator.py
🧠 Learnings (55)
📓 Common learnings
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Pure data models, enums, and re-exports do NOT need logging
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Coverage: 80% minimum (enforced in CI)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Async: `asyncio_mode = 'auto'` -- no manual `pytest.mark.asyncio` needed
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Timeout: 30 seconds per test (global in `pyproject.toml` -- do not add per-file `pytest.mark.timeout(30)` markers; non-default overrides like `timeout(60)` are allowed)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Parallelism: `pytest-xdist` via `-n 8` -- ALWAYS include `-n 8` when running pytest locally, never run tests sequentially
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Commits: `<type>: <description>` -- types: feat, fix, refactor, docs, test, chore, perf, ci. Enforced by commitizen
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Signed commits: required on `main` via branch protection -- all commits must be GPG/SSH signed
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Branches: `<type>/<slug>` from main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: ALWAYS read the relevant `docs/design/` page before implementing any feature. Design spec is the starting point for architecture, data models, and behavior
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: If implementation deviates from the spec, alert the user and explain why -- user decides whether to proceed or update the spec. Do NOT silently diverge
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: At every phase of planning and implementation, be critical -- actively look for ways to improve the design. Surface improvements as suggestions, not silent changes
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Prioritize issues by dependency order, not priority labels -- unblocked dependencies come first
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: NEVER use `cd` in Bash commands -- the working directory is already set to the project root. Use absolute paths or run commands directly. Exception: `bash -c 'cd <dir> && <cmd>'` is safe
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: NEVER use Bash to write or modify files -- use the Write or Edit tools. Do not use `cat >`, `cat << EOF`, `echo >`, `echo >>`, `sed -i`, or `tee` to create or modify files
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: NEVER create a PR directly -- `gh pr create` is blocked by hookify. ALWAYS use `/pre-pr-review` to create PRs. For trivial/docs-only changes: `/pre-pr-review quick`
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: After finishing an issue implementation: always create a feature branch (`<type>/<slug>`), commit, and push -- do NOT create a PR automatically. Do NOT leave work uncommitted on main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T17:33:37.188Z
Learning: Fix everything valid from review agents -- never skip. When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
tests/unit/observability/test_events.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.md
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to src/synthorg/**/*.py : Event names must always use constants from domain-specific modules under `synthorg.observability.events`. Import directly from specific domain modules.
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/analytics.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/observability/events/analytics.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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 Learning: 2026-04-02T12:07:44.443Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T12:07:44.443Z
Learning: Applies to src/synthorg/**/*.py : Always use structured logging: `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Use structured logging: always `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
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:
CLAUDE.md
📚 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:
tests/unit/tools/design/test_config.pysrc/synthorg/config/schema.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/analytics/config.pytests/unit/tools/analytics/test_config.pysrc/synthorg/tools/communication/config.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
tests/unit/tools/design/test_config.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/communication/config.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
tests/unit/tools/design/test_config.pysrc/synthorg/config/schema.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/communication/config.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:
tests/unit/tools/design/test_config.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/communication/config.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/tools/design/__init__.pydocs/reference/claude-reference.mdsrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/config/schema.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/factory.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to {pyproject.toml,src/synthorg/__init__.py} : Update version in `pyproject.toml` (`[tool.commitizen].version`) and `src/synthorg/__init__.py` (`__version__`)
Applied to files:
src/synthorg/tools/design/__init__.pydocs/reference/claude-reference.mdsrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/communication/__init__.py
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
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/reference/claude-reference.md
📚 Learning: 2026-03-31T14:28:28.895Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:28:28.895Z
Learning: Web dashboard: see `web/CLAUDE.md` for commands, design system, and component inventory
Applied to files:
docs/reference/claude-reference.md
📚 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: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-04-06T16:35:12.934Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-06T16:35:12.934Z
Learning: All project conventions, commands, and standards are defined in CLAUDE.md - refer to it for project structure, package layout, code conventions, quick commands, git workflow, testing standards, design specifications, logging, resilience, and security patterns
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: CLI (Go binary): see `cli/CLAUDE.md` for commands, flags, and reference; use `go -C cli` (never `cd cli`)
Applied to files:
docs/reference/claude-reference.md
📚 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: Settings: Runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge, ConfigResolver (typed composed reads for controllers), validation, registry, change notifications via message bus. Per-namespace setting definitions in definitions/ submodule (api, company, providers, memory, budget, security, coordination, observability, backup).
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
docs/reference/claude-reference.mdsrc/synthorg/config/schema.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to docs/** : Docs source in docs/ (Markdown, built with Zensical); design spec in docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/reference/claude-reference.md
📚 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: Security: SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies: disabled/weighted/per-category/milestone), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume).
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
docs/reference/claude-reference.mdsrc/synthorg/config/schema.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/communication/**/*.py : Communication package (communication/): message bus, dispatcher, messenger, channels, delegation, loop prevention, conflict resolution; meeting/ subpackage for meeting protocol (round-robin, position papers, structured phases), scheduler (frequency, participant resolver), orchestrator
Applied to files:
src/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/tools/communication/notification_sender.pytests/unit/tools/communication/conftest.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Set `RetryConfig` and `RateLimiterConfig` per-provider in `ProviderConfig`.
Applied to files:
src/synthorg/config/schema.pysrc/synthorg/tools/factory.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/config/schema.py
📚 Learning: 2026-03-31T21:07:37.469Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.469Z
Learning: Applies to src/synthorg/providers/**/*.py : Set `RetryConfig` and `RateLimiterConfig` per-provider in `ProviderConfig`
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to src/synthorg/**/*.py : RetryConfig and RateLimiterConfig are set per-provider in `ProviderConfig`
Applied to files:
src/synthorg/config/schema.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/tools/design/config.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/communication/config.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/tools/design/config.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/communication/config.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to **/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models for runtime state. Never mix static config fields with mutable runtime fields.
Applied to files:
src/synthorg/tools/design/config.pysrc/synthorg/tools/communication/config.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to **/*.py : Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/tools/design/config.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/tools/design/config.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/tools/design/config.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 : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
tests/unit/tools/design/conftest.pytests/unit/tools/analytics/test_data_aggregator.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/tools/design/conftest.pytests/unit/tools/analytics/conftest.pytests/unit/tools/communication/conftest.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of real vendor names
Applied to files:
tests/unit/tools/design/conftest.py
📚 Learning: 2026-04-01T09:37:49.451Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:37:49.451Z
Learning: Applies to **/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models with `model_copy(update=...)` for runtime state that evolves
Applied to files:
src/synthorg/tools/communication/config.py
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/tools/test_factory.py (1)
351-408: 🧹 Nitpick | 🔵 TrivialAdd
build_default_tools_from_config()coverage for the new categories.The new suites exercise
build_default_tools()only. A regression whereRootConfig.design_tools,communication_tools, oranalytics_toolsis ignored bybuild_default_tools_from_config()would still pass, even though this PR explicitly wires both factory paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/operations.md`:
- Around line 694-695: Clarify that the wiring performed by
build_default_tools_from_config for VERSION_CONTROL, DESIGN, COMMUNICATION, and
ANALYTICS is conditional: state that each category is opt-in and only registered
when its configuration is present and the required runtime dependency is
injected (e.g., DESIGN requires a design provider, COMMUNICATION requires a
dispatcher, ANALYTICS requires a sink); update the sentence to explicitly note
the dependency-gated behavior so the docs match the implementation.
In `@src/synthorg/tools/analytics/data_aggregator.py`:
- Around line 183-197: The execute() function must validate the incoming
arguments dict before destructuring: check required keys exist and types are
correct (e.g., ensure "metrics" is a list/iterable of strings, "start_date" and
"end_date" are strings or None) and return a ToolExecutionResult with
is_error=True and a clear message on any malformed input rather than letting
KeyError/TypeError propagate; specifically, in data_aggregator.execute()
validate presence and types of metrics, start_date, end_date and only then call
datetime.fromisoformat on the date strings (or return a structured error if
dates are non-string or invalid), and apply the same validation pattern to the
later block handling the other date range (lines 237-251).
- Around line 298-316: The loop is mutating the provider-returned dict `data` in
place before passing it into `ToolExecutionResult(metadata=...)`; instead,
create a new sanitized copy to enforce `self._config.max_rows` without altering
the original provider object (e.g., build a new dict `sanitized = {k:
(v[:self._config.max_rows] if isinstance(v, list) and len(v) >
self._config.max_rows else v) for k,v in data.items()}`) and use `sanitized` for
presentation and `metadata` (or keep `metadata=data` if you must preserve the
original) so the provider's result isn't mutated; update references in this
scope (the aggregation method in data_aggregator.py that sets `lines` and
returns `ToolExecutionResult`) to use the new copy.
In `@src/synthorg/tools/analytics/report_generator.py`:
- Around line 180-187: The code returns a ToolExecutionResult error when
output_format is invalid but doesn’t log a warning; before returning from the
output_format check in report_generator.py, add a warning log (e.g.
logger.warning) that includes the invalid output_format value and the allowed
values (sorted(_OUTPUT_FORMATS)), then return the existing ToolExecutionResult;
update the branch that references output_format, _OUTPUT_FORMATS and
ToolExecutionResult to emit the warning first (mirror the existing report_type
validation logging style).
- Around line 157-164: The invalid report_type branch in report_generator.py
doesn't log before returning an error; update the validation around report_type
(checking against _REPORT_TYPES) to call the module logger (e.g.,
logger.warning) with context including the invalid report_type value and
available options, then return the same ToolExecutionResult error as before;
ensure you reference the same symbols (report_type, _REPORT_TYPES,
ToolExecutionResult) and keep the log message concise and informative.
In `@src/synthorg/tools/communication/config.py`:
- Around line 52-59: Add a Pydantic `@model_validator` on the config model that
defines the use_tls and use_implicit_tls Fields to enforce mutual exclusivity:
if both use_tls and use_implicit_tls are True, raise a ValueError with a clear
message (e.g., "use_tls and use_implicit_tls are mutually exclusive"). Place the
validator on the model containing use_tls/use_implicit_tls (use the
`@model_validator` decorator) so the invalid combination is rejected early; this
matches the runtime check in email_sender (which expects not use_implicit_tls
and use_tls).
In `@src/synthorg/tools/communication/email_sender.py`:
- Around line 136-144: The error-return branch that checks email_config (the "if
email_config is None" block returning a ToolExecutionResult) must first log a
warning with context; update that block to call the logger (e.g.,
self._logger.warning or the module logger) before returning the
ToolExecutionResult, including a message that SMTP/email is not configured and
mentioning "CommunicationToolsConfig.email" so logs and the return are
consistent with other tools.
In `@src/synthorg/tools/communication/notification_sender.py`:
- Around line 160-176: Validation branches in send_notification are returning
ToolExecutionResult on invalid category/severity without logging; add a warning
log before each return using the same logging pattern and constant used
elsewhere (e.g., COMM_TOOL_NOTIFICATION_SEND_FAILED) to provide context.
Specifically, in the invalid-category check that references _VALID_CATEGORIES
and returns ToolExecutionResult, call the module/logger warning (matching
existing logger usage in notification_sender) with a message including
category_str and the valid options, then return the ToolExecutionResult; do the
same in the invalid-severity branch that references _VALID_SEVERITIES. Ensure
the log level matches other error paths (WARNING or ERROR) and include the same
contextual fields/tags used in the dispatcher-not-configured path for
consistency.
In `@src/synthorg/tools/design/asset_manager.py`:
- Around line 136-150: The execute() implementation treats entries in arguments
(e.g., arguments["action"], arguments["query"]) as strings and can raise
KeyError or attribute errors; guard the raw arguments at the boundary by using
arguments.get("action") and isinstance checks (or explicit type casting) before
using string methods or membership tests against _VALID_ACTIONS, and return a
ToolExecutionResult with is_error=True and a clear message when action is
missing/invalid or when query is not a string, updating both the action handling
block (where action is read/validated) and the query handling code path (where
.lower() is called).
- Around line 47-50: _search currently ignores the "tags" field in the incoming
request; update _handle_search (and the similar list/search block) to read
message.get("tags") (if present and non-empty) and filter the candidate assets
so that each asset's tags include the requested tags (e.g., require every tag in
the request to be present in asset["tags"] or apply the chosen matching rule),
ensuring you handle missing/None/empty tag lists and assets without a tags key;
place this filtering after the initial query match but before returning results
so both _handle_search() and the list/search logic at the other occurrence apply
the tags filter consistently.
In `@src/synthorg/tools/design/image_generator.py`:
- Around line 270-285: The size check is currently using the base64-encoded
string length (result.data) against a byte limit, which inflates size; decode
the base64 payload to raw bytes and compare len(decoded_bytes) to
self._config.max_image_size_bytes instead, then log the actual decoded byte size
in the DESIGN_IMAGE_GENERATION_FAILED logger call and return message
(references: result.data, self._config.max_image_size_bytes,
DESIGN_IMAGE_GENERATION_FAILED, ToolExecutionResult).
🪄 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: b01a8758-18e2-456a-8d94-62ed66cb8acc
📒 Files selected for processing (19)
CLAUDE.mddocs/design/operations.mdsrc/synthorg/config/schema.pysrc/synthorg/observability/events/design.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/factory.pytests/unit/config/conftest.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/test_factory.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python 3.14+ (PEP 649 native lazy annotations).
Do not use
from __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations.Use PEP 758 except syntax: use
except A, B:(no parentheses) — ruff enforces this on Python 3.14.Type hints: all public functions, mypy strict mode.
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules).
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,
BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using
model_copy(update=...)) for runtime state that evolves (e.g. agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.Models: Pydantic v2 (
BaseModel,model_validator,computed_field,ConfigDict). Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time. Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens). UseNotBlankStr(fromcore.types) for all identifier/name fields—including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants—instead of manual whitespace validators.Async concurrency: prefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.Line length: 88 characters (ruff).
Functions: < 50 lines, file...
Files:
tests/unit/config/conftest.pytests/unit/tools/test_factory.pysrc/synthorg/config/schema.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/communication/test_email_sender.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/factory.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.pytests/unit/tools/design/test_diagram_generator.pysrc/synthorg/observability/events/design.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Test markers:
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow.Async:
asyncio_mode = 'auto'— no manual@pytest.mark.asyncioneeded.Timeout: 30 seconds per test (global in
pyproject.toml— do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed).Parametrize: Prefer
@pytest.mark.parametrizefor testing similar cases.Vendor-agnostic everywhere: Tests must use
test-provider,test-small-001, etc. instead of real vendor names.Property-based testing: Python uses Hypothesis (
@given+@settings). Hypothesis profiles configured intests/conftest.py:ci(deterministic,max_examples=10+derandomize=True),dev(1000 examples),fuzz(10,000 examples, no deadline),extreme(500,000 examples, no deadline). Controlled viaHYPOTHESIS_PROFILEenv var..hypothesis/is gitignored. Failing examples are persisted to~/.synthorg/hypothesis-examples/.Flaky tests: NEVER skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally. For timing-sensitive tests, mock
time.monotonic()andasyncio.sleep()to make them deterministic. For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/config/conftest.pytests/unit/tools/test_factory.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/design/test_diagram_generator.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/config/conftest.pytests/unit/tools/test_factory.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/design/test_diagram_generator.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths/module names (e.g.litellm.types.llms.openai), (4) provider presets (src/synthorg/providers/presets.py). Tests must usetest-provider,test-small-001, etc.
Files:
src/synthorg/config/schema.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/factory.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/observability/events/design.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/config/schema.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/factory.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/observability/events/design.py
🧠 Learnings (54)
📓 Common learnings
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...
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue. Read DESIGN_SPEC.md as a pointer file linking to the 12 design pages. The design spec is the starting point for architecture, data models, and behavior.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: If implementation deviates from the design spec (better approach found, scope evolved, etc.), alert the user and explain why. User decides whether to proceed or update the spec. Do not silently diverge—every deviation needs explicit user approval.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: When a spec topic is referenced (e.g. 'the Agents page' or 'the Engine page's Crash Recovery section'), read the relevant `docs/design/` page before coding.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: At every phase of planning and implementation, be critical—actively look for ways to improve the design in the spirit of what we're building (robustness, correctness, simplicity, future-proofing where it's free). Surface improvements as suggestions, not silent changes—user decides.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Prioritize issues by dependency order, not priority labels—unblocked dependencies come first.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Parallelism: `pytest-xdist` via `-n 8` — ALWAYS include `-n 8` when running pytest locally, never run tests sequentially. CI uses `-n auto` (fewer cores on runners).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Hypothesis workflow: CI runs 10 deterministic examples per property test (`derandomize=True`). Random fuzzing runs locally: `HYPOTHESIS_PROFILE=dev` (1000 examples) or `HYPOTHESIS_PROFILE=fuzz` (10,000 examples, no deadline). When Hypothesis finds a failure, it is a real bug. Fix the underlying bug and add an explicit `example(...)` decorator so the case is permanently covered in CI.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Commits: `<type>: <description>` — types: feat, fix, refactor, docs, test, chore, perf, ci. Enforced by commitizen (commit-msg hook).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Signed commits: required on `main` via branch protection — all commits must be GPG/SSH signed.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Branches: `<type>/<slug>` from main.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Pre-commit hooks enforce: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint (Dockerfile), golangci-lint + go vet (CLI), no-em-dashes, no-redundant-timeout, eslint-web (web dashboard).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Hookify rules: `block-pr-create` (blocks direct `gh pr create`), `enforce-parallel-tests` (enforces `-n 8` with pytest), `no-cd-prefix` (blocks `cd` prefix in Bash commands), `no-local-coverage` (blocks `--cov` flags locally).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Pre-push hooks: mypy type-check (affected modules only) + pytest unit tests (affected modules only) + golangci-lint + go vet + go test (CLI) + eslint-web (web dashboard) (fast gate before push, skipped in pre-commit.ci). Foundational module changes (core, config, observability) or conftest changes trigger full runs.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: GitHub issue queries: use `gh issue list` via Bash (not MCP tools) — MCP `list_issues` has unreliable field data.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Merge strategy: squash merge — PR body becomes the squash commit message on main. Trailers (e.g. `Release-As`, `Closes `#N``) must be in the PR body to land in the final commit.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: PR issue references: preserve existing `Closes `#NNN`` references — never remove unless explicitly asked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: After finishing an issue implementation: always create a feature branch (`<type>/<slug>`), commit, and push — do NOT create a PR automatically. Do not leave work uncommitted on main — branch, commit, push immediately after finishing.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Never create a PR directly — `gh pr create` is blocked by hookify. Always use `/pre-pr-review` to create PRs — it runs automated checks + review agents + fixes before creating the PR. For trivial/docs-only changes: `/pre-pr-review quick` skips agents but still runs automated checks.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: After the PR exists, use `/aurelio-review-pr` to handle external reviewer feedback. The `/commit-push-pr` command is effectively blocked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Fix everything valid — never skip: When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all. No deferring, no 'out of scope' skipping.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Never use `cd` in Bash commands — the working directory is already set to the project root. Use absolute paths or run commands directly. Exception: `bash -c 'cd <dir> && <cmd>'` is safe (runs in a child process). Use this for tools without a `-C` flag.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: Never use Bash to write or modify files — use the Write or Edit tools. Do not use `cat >`, `cat << EOF`, `echo >`, `echo >>`, `sed -i`, `python -c 'open(...).write(...)'`, or `tee` to create or modify files. Read-only/inspection uses like piping to stdout are fine.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:24:30.862Z
Learning: See `web/CLAUDE.md` for the full component inventory, design token rules, and post-training references. A PostToolUse hook (`scripts/check_web_design_system.py`) enforces design system rules on every Edit/Write to `web/src/`.
📚 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 docker/Dockerfile.sandbox : Docker sandbox: `synthorg-sandbox` — Python 3.14 + Node.js + git, non-root (UID 10001), agent code execution sandbox
Applied to files:
docs/design/operations.md
📚 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/config/schema.pysrc/synthorg/tools/communication/config.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/config/schema.pysrc/synthorg/tools/communication/config.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/config/schema.pysrc/synthorg/tools/communication/config.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/config/schema.pysrc/synthorg/tools/communication/config.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/config/schema.pysrc/synthorg/tools/communication/config.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/config/schema.pysrc/synthorg/tools/factory.pysrc/synthorg/tools/communication/email_sender.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/config/schema.pysrc/synthorg/tools/communication/config.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 with adopted conventions: use computed_field for derived values instead of storing + validating redundant fields; use NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Applied to files:
src/synthorg/config/schema.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/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Set `RetryConfig` and `RateLimiterConfig` per-provider in `ProviderConfig`.
Applied to files:
src/synthorg/config/schema.pysrc/synthorg/tools/factory.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/config/schema.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/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-31T21:07:37.469Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.469Z
Learning: Applies to src/synthorg/providers/**/*.py : Set `RetryConfig` and `RateLimiterConfig` per-provider in `ProviderConfig`
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to src/synthorg/**/*.py : RetryConfig and RateLimiterConfig are set per-provider in `ProviderConfig`
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.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 : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) -- PEP 758 except syntax, enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.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 **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.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 **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/design/diagram_generator.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/communication/**/*.py : Communication package (communication/): message bus, dispatcher, messenger, channels, delegation, loop prevention, conflict resolution; meeting/ subpackage for meeting protocol (round-robin, position papers, structured phases), scheduler (frequency, participant resolver), orchestrator
Applied to files:
src/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/notification_sender.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/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/diagram_generator.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/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/diagram_generator.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/diagram_generator.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/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/diagram_generator.pyCLAUDE.md
📚 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/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/diagram_generator.pyCLAUDE.md
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
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/tools/communication/notification_sender.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/design/diagram_generator.pyCLAUDE.md
📚 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/tools/communication/notification_sender.pysrc/synthorg/tools/design/diagram_generator.pyCLAUDE.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability must use structured logging with correlation tracking and log sinks
Applied to files:
src/synthorg/tools/analytics/metric_collector.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/tools/factory.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to {src/synthorg/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx}} : NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`, `large`/`medium`/`small`
Applied to files:
src/synthorg/tools/factory.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : For tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)` -- it is cancellation-safe
Applied to files:
src/synthorg/tools/analytics/data_aggregator.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
CLAUDE.mdsrc/synthorg/observability/events/design.py
📚 Learning: 2026-04-02T12:07:44.443Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T12:07:44.443Z
Learning: Applies to src/synthorg/**/*.py : Always use structured logging: `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Use structured logging: always `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
CLAUDE.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 : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
src/synthorg/observability/events/design.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/design.py
1f26eb7 to
0e53c9d
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/tools/communication/email_sender.py`:
- Around line 294-296: The login block in email_sender.py currently only calls
smtp.login when both email_config.username and email_config.password are truthy,
which silently skips authentication if only one credential is provided; update
the logic in the function that sends emails (refer to smtp.login usage and the
EmailConfig object) to detect partial credentials (username XOR password) and
emit a clear warning via the existing logger (or raise a configuration error),
and either proceed without login or fail fast; alternatively, add cross-field
validation on EmailConfig to reject configurations with only one of
username/password so the send path never encounters partial credentials.
In `@src/synthorg/tools/design/asset_manager.py`:
- Around line 211-216: The _handle_get branch returns a ToolExecutionResult when
asset_id is missing but doesn't log; update the validation in _handle_get to log
a warning before returning by calling the module/class logger (e.g.,
self.logger.warning or logger.warning) with context including the action name,
missing asset_id and the supplied arguments, then return the existing
ToolExecutionResult; reference symbols: _handle_get, asset_id,
ToolExecutionResult, and use the same logger instance used elsewhere in this
class (self.logger or logger).
- Around line 243-248: In _handle_delete, add a warning log before returning the
error ToolExecutionResult when asset_id is missing: log a clear warning (e.g.,
using self.logger.warning or the module logger) that asset_id is required for
the 'delete' action, then return the existing
ToolExecutionResult(content="asset_id is required for 'delete' action.",
is_error=True); update the code path that checks asset_id: str | None =
arguments.get("asset_id") to call the logger warning first to mirror the
behavior in _handle_get.
- Around line 272-277: The `_handle_search` path returns an error when the
`query` is missing but doesn't log anything; add a warning log just before
returning in `_handle_search` (use the class/logger instance used elsewhere in
this file, e.g., self.logger.warning or logger.warning) that includes context
like the action name and provided arguments, then return the ToolExecutionResult
as before; reference `_handle_search` and `ToolExecutionResult` when making the
change.
In `@src/synthorg/tools/design/image_generator.py`:
- Around line 271-275: The code silently falls back to treating result.data as
raw text when base64.b64decode fails; instead, update the try/except around
base64.b64decode(result.data, validate=True) to not silently accept invalid
Base64: catch the specific decode exception (binascii.Error/ValueError), log a
warning/error referencing ImageResult.data and the provider, and either
propagate/raise an error or return a failed ImageResult rather than using
result.data.encode(); ensure decoded_bytes is only set on successful decode and
byte_size computed thereafter (use existing symbols decoded_bytes, byte_size,
and result).
In `@tests/unit/tools/analytics/test_data_aggregator.py`:
- Around line 67-85: The test test_execute_passes_all_params is missing an
assertion that the provided "end_date" is forwarded to the provider; update the
test for DataAggregatorTool to assert mock_provider.calls[0]["end_date"] ==
"2026-01-31" (alongside the existing checks for "period", "group_by", and
"start_date") so the call contract is fully verified.
In `@tests/unit/tools/communication/test_template_formatter.py`:
- Around line 135-145: The test test_sandbox_blocks_attribute_access should
assert that dangerous attribute access triggers an error instead of allowing
silent omission; update the assertion in
tests/unit/tools/communication/test_template_formatter.py (inside
test_sandbox_blocks_attribute_access) to require result.is_error is True (i.e.,
Jinja2 SandboxedEnvironment raises a SecurityError) rather than the permissive
disjunction that only checks for "__class__" in result.content; keep using
TemplateFormatterTool().execute and the existing input but change the final
assertion to assert result.is_error so the test fails if the sandbox does not
block attribute access.
In `@tests/unit/tools/design/conftest.py`:
- Around line 52-54: Remove the unused pytest fixture named default_config that
returns a DesignToolsConfig instance; locate the function def default_config()
-> DesignToolsConfig: in tests/unit/tools/design/conftest.py and delete that
function definition (and any associated imports solely used by it, e.g.,
DesignToolsConfig if unused elsewhere) to keep fixtures minimal.
In `@tests/unit/tools/design/test_config.py`:
- Around line 34-52: The tests test_image_timeout_must_be_positive,
test_image_timeout_max, test_max_image_size_must_be_positive, test_rejects_nan,
and test_rejects_inf all repeat the same pattern; consolidate them by replacing
these separate functions with a single parametrized test using
pytest.mark.parametrize that supplies tuples of invalid kwargs (e.g.,
{"image_timeout": 0}, {"image_timeout": 601.0}, {"max_image_size_bytes": 0},
{"image_timeout": float("nan")}, {"image_timeout": float("inf")}) and asserts
that DesignToolsConfig(**kwargs) raises ValidationError; ensure pytest is
imported and keep the assertion inside a with pytest.raises(ValidationError)
block and name the test (e.g., test_invalid_design_tools_config_params) to match
project conventions.
🪄 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: 85442ff8-9682-4b6c-ae3f-9c6d7ed66ae9
📒 Files selected for processing (48)
CLAUDE.mddocs/design/operations.mddocs/reference/claude-reference.mdsrc/synthorg/config/defaults.pysrc/synthorg/config/schema.pysrc/synthorg/observability/events/analytics.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/analytics/base_analytics_tool.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/communication/template_formatter.pysrc/synthorg/tools/design/__init__.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/base_design_tool.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/factory.pytests/unit/config/conftest.pytests/unit/observability/test_events.pytests/unit/tools/analytics/__init__.pytests/unit/tools/analytics/conftest.pytests/unit/tools/analytics/test_config.pytests/unit/tools/analytics/test_data_aggregator.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/__init__.pytests/unit/tools/communication/conftest.pytests/unit/tools/communication/test_config.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/communication/test_notification_sender.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/__init__.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/test_factory.py
Comment on lines
+294
to
+296
| if email_config.username and email_config.password: | ||
| smtp.login(email_config.username, email_config.password) | ||
| smtp.send_message(msg, to_addrs=all_recipients) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Login silently skipped when only one credential is provided.
The condition if email_config.username and email_config.password means if only one of username/password is configured (e.g., password-only auth or username without password due to config error), the tool silently proceeds without authentication. Consider logging a warning for this edge case.
🛡️ Proposed fix to warn on partial credentials
+ if bool(email_config.username) != bool(email_config.password):
+ # This is likely a configuration error - log but continue
+ import warnings
+ # Better: add a logger.warning here if observability context is available
if email_config.username and email_config.password:
smtp.login(email_config.username, email_config.password)Alternatively, add validation in EmailConfig itself to require both or neither (cross-field validation).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/tools/communication/email_sender.py` around lines 294 - 296, The
login block in email_sender.py currently only calls smtp.login when both
email_config.username and email_config.password are truthy, which silently skips
authentication if only one credential is provided; update the logic in the
function that sends emails (refer to smtp.login usage and the EmailConfig
object) to detect partial credentials (username XOR password) and emit a clear
warning via the existing logger (or raise a configuration error), and either
proceed without login or fail fast; alternatively, add cross-field validation on
EmailConfig to reject configurations with only one of username/password so the
send path never encounters partial credentials.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/synthorg/tools/design/asset_manager.py (2)
223-228:⚠️ Potential issue | 🟡 MinorMissing warning log for "asset not found" error path.
The error result when an asset is not found is returned without logging. Per coding guidelines, all error paths must log at WARNING or ERROR level with context.
🛡️ Proposed fix to add warning log
meta = self._assets.get(asset_id) if meta is None: + logger.warning( + DESIGN_ASSET_VALIDATION_FAILED, + action="get", + asset_id=asset_id, + reason="asset_not_found", + ) return ToolExecutionResult( content=f"Asset not found: {asset_id!r}", is_error=True, )As per coding guidelines: "All error paths must log at WARNING or ERROR with context before raising."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/design/asset_manager.py` around lines 223 - 228, The code returns a ToolExecutionResult when an asset is missing but does not log the error; update the missing-asset branch to emit a warning before returning (use the class/module logger, e.g. self.logger.warning(...) or module-level logger.warning(...)) and include contextual fields like the asset_id and that it came from the _assets lookup (referencing meta, _assets, and ToolExecutionResult) so the warning clearly states "Asset not found" plus the asset_id prior to returning the ToolExecutionResult.
306-311: 🧹 Nitpick | 🔵 TrivialConsider adding
filter_tagsto search logging for consistency.The
_handle_listmethod logsfilter_tagsin its event, but_handle_searchonly logssearch_query. For consistent observability when both query and tags are used for filtering, consider includingfilter_tagsin the search log as well.♻️ Proposed enhancement
logger.info( DESIGN_ASSET_SEARCHED, total=len(self._assets), matched=len(matching), search_query=query, + filter_tags=tags, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/design/asset_manager.py` around lines 306 - 311, The search logging in _handle_search currently logs DESIGN_ASSET_SEARCHED with total, matched, and search_query but omits filter_tags; update the logger.info call in _handle_search (the one emitting DESIGN_ASSET_SEARCHED) to include filter_tags (e.g., add filter_tags=filter_tags or the local tags variable) so the event mirrors _handle_list's payload and captures tag-based filtering for consistent observability.
🤖 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/tools/design/asset_manager.py`:
- Around line 260-264: The delete path currently returns a ToolExecutionResult
when asset_id is missing without logging; update the delete handler (the method
that checks "if asset_id not in self._assets") to call the logger (e.g.,
self._logger.warning) with contextual information including asset_id before
returning the ToolExecutionResult so the missing-asset error is recorded,
keeping the existing return value and is_error=True behavior.
- Around line 106-120: The method register_asset mutates internal state but
lacks an INFO-level state-transition log; import the DESIGN_ASSET_STORED
constant from events.design and add an info log in register_asset immediately
after updating self._assets (e.g., call logger.info or the module's logger with
the event constant and asset_id/summary of metadata) so the DESGIN_ASSET_STORED
event and the asset_id are recorded at INFO level.
In `@tests/unit/tools/analytics/test_data_aggregator.py`:
- Around line 67-85: The test test_execute_passes_all_params in
tests/unit/tools/analytics/test_data_aggregator.py is missing an assertion that
the metrics argument is forwarded; after retrieving call =
mock_provider.calls[0], add an assertion that call["metrics"] equals the
expected list (e.g., ["total_cost"]) to fully validate
DataAggregatorTool.execute's parameter forwarding to the mock_provider.
In `@tests/unit/tools/communication/test_template_formatter.py`:
- Around line 15-25: Replace the three near-identical tests
test_category_is_communication, test_action_type_is_code_read, and test_name
with a single parameterized pytest test that iterates over expected
attribute/value pairs for TemplateFormatterTool (e.g., ("category",
ToolCategory.COMMUNICATION), ("action_type", ActionType.CODE_READ), ("name",
"template_formatter")); inside the test instantiate TemplateFormatterTool once
and assert getattr(tool, attr) == expected for each case to remove duplication
and follow `@pytest.mark.parametrize` guidelines.
---
Duplicate comments:
In `@src/synthorg/tools/design/asset_manager.py`:
- Around line 223-228: The code returns a ToolExecutionResult when an asset is
missing but does not log the error; update the missing-asset branch to emit a
warning before returning (use the class/module logger, e.g.
self.logger.warning(...) or module-level logger.warning(...)) and include
contextual fields like the asset_id and that it came from the _assets lookup
(referencing meta, _assets, and ToolExecutionResult) so the warning clearly
states "Asset not found" plus the asset_id prior to returning the
ToolExecutionResult.
- Around line 306-311: The search logging in _handle_search currently logs
DESIGN_ASSET_SEARCHED with total, matched, and search_query but omits
filter_tags; update the logger.info call in _handle_search (the one emitting
DESIGN_ASSET_SEARCHED) to include filter_tags (e.g., add filter_tags=filter_tags
or the local tags variable) so the event mirrors _handle_list's payload and
captures tag-based filtering for consistent observability.
🪄 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: 21001b62-0fc2-4e8f-a31d-db3e55faa19b
📒 Files selected for processing (7)
src/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/image_generator.pytests/unit/tools/analytics/test_data_aggregator.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_config.pytests/unit/tools/design/test_image_generator.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). (4)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue. DESIGN_SPEC.md is a pointer file linking to the 12 design pages. The design spec is the starting point for architecture, data models, and behavior.If implementation deviates from the design spec (better approach found, scope evolved, etc.), alert the user and explain why. Do NOT silently diverge from the spec -- every deviation needs explicit user approval.
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.Do NOT use
from __future__ import annotations. Python 3.14+ has PEP 649 native lazy annotations.Use PEP 758 except syntax: use
except A, B:(no parentheses) on Python 3.14. Ruff enforces this.All public functions must have type hints. Use mypy strict mode.
Use Google style docstrings. Docstrings are required on all public classes and functions (enforced by ruff D rules).
Prefer immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,
BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.For
dict/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 (using
model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.Use Pydantic v2 (
BaseModel,model_validator,computed_field,ConfigDict). Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.Use
@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens).Use
NotBlankStr(fr...
Files:
tests/unit/tools/design/test_config.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/test_data_aggregator.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/image_generator.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use test markers:
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow. Coverage minimum: 80% (enforced in CI).Async test mode:
asyncio_mode = 'auto'. No manual@pytest.mark.asyncioneeded.Timeout: 30 seconds per test (global in
pyproject.toml). Do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed.Parallelism:
pytest-xdistvia-n 8. ALWAYS include-n 8when running pytest locally, never run tests sequentially. CI uses-n auto.Prefer
@pytest.mark.parametrizefor testing similar cases.NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths/module names (e.g.litellm.types.llms.openai), (4) provider presets (src/synthorg/providers/presets.py). Tests must usetest-provider,test-small-001, etc.When Hypothesis finds a failure, fix the underlying bug and add an explicit
@example(...)decorator to the test so the case is permanently covered in CI. Do NOT just rerun and move on.NEVER skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mock
time.monotonic()andasyncio.sleep()to make them deterministic. For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/tools/design/test_config.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/test_data_aggregator.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/tools/design/test_config.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/test_data_aggregator.py
{src/synthorg,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Commit messages follow format:
<type>: <description>. Types: feat, fix, refactor, docs, test, chore, perf, ci. Enforced by commitizen (commit-msg hook). Signed commits required onmain.
Files:
tests/unit/tools/design/test_config.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/analytics/test_data_aggregator.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/image_generator.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.). Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Vendor names only appear in: (1) Operations design page provider list, (2).claude/files, (3) third-party imports, (4) provider presets insrc/synthorg/providers/presets.py.
Files:
src/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/image_generator.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/image_generator.py
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: At every phase of planning and implementation, be critical -- actively look for ways to improve the design in the spirit of what we're building (robustness, correctness, simplicity, future-proofing where it's free). Surface improvements as suggestions, not silent changes -- user decides.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Prioritize issues by dependency order, not priority labels. Unblocked dependencies come first.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: NEVER use `cd` in Bash commands. The working directory is already set to the project root. Use absolute paths or run commands directly. Exception: `bash -c 'cd <dir> && <cmd>'` is safe (runs in child process). Use this for tools without a `-C` flag.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: NEVER use Bash to write or modify files. Use the Write or Edit tools. Do not use `cat >`, `cat << EOF`, `echo >`, `echo >>`, `sed -i`, `python -c 'open(...).write(...)'`, or `tee` to create or modify files. Read-only/inspection uses like piping to stdout are fine.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Use branch naming: `<type>/<slug>` from main.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Pre-commit hooks enforce: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint (Dockerfile), golangci-lint + go vet (CLI, conditional), no-em-dashes, no-redundant-timeout, eslint-web (web, conditional).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Hookify rules (committed in `.claude/hookify.*.md`): (1) `block-pr-create` blocks direct `gh pr create` -- must use `/pre-pr-review`, (2) `enforce-parallel-tests` enforces `-n 8` with pytest, (3) `no-cd-prefix` blocks `cd` prefix in Bash commands, (4) `no-local-coverage` blocks `--cov` flags locally (CI handles coverage).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Pre-push hooks enforce: mypy type-check (affected modules) + pytest unit tests (affected modules) + golangci-lint + go vet + go test (CLI) + eslint-web (web). Foundational module changes (core, config, observability) or conftest changes trigger full runs.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Use `gh issue list` via Bash for GitHub issue queries -- MCP `list_issues` has unreliable field data.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: Merge strategy: squash merge. PR body becomes the squash commit message on main. Trailers (e.g. `Release-As`, `Closes `#N``) must be in the PR body to land in final commit. Preserve existing `Closes `#NNN`` references -- never remove unless explicitly asked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: After finishing an issue implementation, create a feature branch (`<type>/<slug>`), commit, and push. Do NOT create a PR automatically. Do NOT leave work uncommitted on main.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: NEVER create a PR directly -- `gh pr create` is blocked by hookify. ALWAYS use `/pre-pr-review` to create PRs. It runs automated checks + review agents + fixes before creating PR. For trivial/docs-only changes: `/pre-pr-review quick` skips agents but still runs checks. After PR exists, use `/aurelio-review-pr` for external reviewer feedback.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:59:04.975Z
Learning: When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to PR changes), fix them all. No deferring, no 'out of scope' skipping.
📚 Learning: 2026-04-08T19:05:17.824Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.824Z
Learning: Python tests have a 30-second global timeout per test (in `pyproject.toml`)—do not add per-file `pytest.mark.timeout(30)` markers; non-default overrides like `timeout(60)` are allowed
Applied to files:
tests/unit/tools/design/test_config.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:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files) in Python code
Applied to files:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/tools/design/test_config.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/tools/design/test_config.py
📚 Learning: 2026-03-31T20:29:10.177Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:29:10.177Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases in Python tests
Applied to files:
tests/unit/tools/design/test_config.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:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
tests/unit/tools/design/test_config.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
tests/unit/tools/design/test_config.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/tools/design/conftest.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/tools/design/image_generator.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 : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/tools/design/image_generator.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/design/image_generator.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 **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/tools/design/image_generator.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) for PEP 758 except syntax in Python 3.14; ruff enforces this
Applied to files:
src/synthorg/tools/design/image_generator.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/design/image_generator.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 **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/tools/design/image_generator.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/tools/design/image_generator.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/tools/design/image_generator.py
🔇 Additional comments (19)
tests/unit/tools/design/test_config.py (1)
1-59: LGTM! Comprehensive test coverage for DesignToolsConfig.The tests thoroughly cover:
- Default values matching the model definition
- Frozen model immutability enforcement
- Custom value preservation
- Numeric constraint validation via parametrized tests (boundary conditions and special float values)
- NotBlankStr validation for
asset_storage_pathThe parametrization for validation tests (lines 36-55) improves maintainability as previously suggested.
tests/unit/tools/design/conftest.py (1)
10-58: LGTM!The
MockImageProviderclass correctly mirrors theImageProviderprotocol signature with proper type hints and async behavior. The fixtures are appropriately minimal and focused. Thecallslist for capturing invocations is a clean pattern for test assertions.tests/unit/tools/design/test_image_generator.py (1)
15-124: LGTM!Comprehensive test coverage for
ImageGeneratorTool:
- Identity fields (category, action_type, name)
- Schema validation
- Execute success path with metadata verification
- Parameter passthrough
- Missing provider handling
- Provider failure propagation
- Default values
- Protocol conformance
- Custom result handling
The defensive test for
provider=None(lines 78-82) is valuable even though the factory omits the tool when no provider is configured.src/synthorg/tools/design/image_generator.py (6)
1-49: LGTM!Clean module header with proper imports and a well-structured
ImageResultmodel:
- Frozen Pydantic model with
allow_inf_nan=Falseper guidelines- Appropriate field constraints (
min_length=1,gt=0)- Clear docstring with attribute descriptions
52-80: LGTM!The
ImageProviderprotocol is properly defined:
@runtime_checkableenablesisinstance()checks (used in tests)- Async signature matches the tool's async execution model
- Sensible defaults for optional parameters
83-124: LGTM!Well-defined JSON Schema with:
- Appropriate constraints (
minimum/maximumfor dimensions)- Enum validation for
styleandqualityadditionalProperties: falsefor strict validation- Corresponding
_VALID_STYLESand_VALID_QUALITIESfrozensets for runtime validation
127-163: LGTM!
ImageGeneratorTool.__init__properly:
- Passes
copy.deepcopy(_PARAMETERS_SCHEMA)for immutability- Sets
action_type=ActionType.DOCS_WRITEas appropriate for asset generation- Stores provider for execution-time use
165-269: LGTM!The
execute()method handles all validation and error cases correctly:
- Missing provider check with structured logging
- Style/quality validation against allowed sets (addresses past review)
- Timeout handling via
asyncio.wait_for()- PEP 758 exception syntax at line 257 (
except MemoryError, RecursionError: raise)- All error paths log at WARNING level with structured kwargs
271-321: LGTM!Post-generation validation is properly implemented:
- Base64 validation with
validate=Truereturns an error on invalid data (addresses past review about silent fallback)- Size check uses decoded byte size (addresses past review about base64 string length vs byte size)
- Success path logs with structured kwargs and returns comprehensive metadata
tests/unit/tools/analytics/test_data_aggregator.py (3)
1-17: LGTM!Imports are clean, and the test class is properly marked with
@pytest.mark.unitas required.
19-38: LGTM!Static metadata tests are simple and verify the expected tool properties correctly.
195-209: LGTM!The protocol compliance test correctly uses
isinstancewith the@runtime_checkableprotocol, and the schema test properly verifies required fields.tests/unit/tools/communication/test_template_formatter.py (2)
135-146: Nice hardening of the sandbox security assertion.Requiring
result.is_errorfor__class__access is the right strict expectation for this threat case.
147-173: Good coverage of escaping behavior by output format.The paired HTML/text tests clearly verify auto-escaping in HTML mode and raw rendering in text mode.
src/synthorg/tools/design/asset_manager.py (5)
1-24: LGTM!Module setup is clean: proper docstring, correct logger initialization using
get_logger(__name__), and event constants imported from the domain-specificevents.designmodule as required by guidelines.
26-59: LGTM!Constants are properly defined with
Finalannotations. Thefrozensetfor_VALID_ACTIONSensures immutability, and the parameter schema correctly documents all supported parameters with appropriate types.
82-104: LGTM!Constructor properly deep-copies both the parameters schema and the assets dictionary, following the immutability guidelines. The
ActionType.DOCS_WRITEis a conservative choice given the tool supports both read and write operations.
122-167: LGTM!The
executemethod properly validates theactionparameter at the boundary with type checking and membership validation. Warning logs are emitted with structured context before returning error results, following the guidelines.
169-204: LGTM!Tag filtering is correctly implemented with "all provided tags must be present" semantics. The logging includes structured context with total count, matched count, and the filter tags used.
34541b6 to
512ce48
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
…ionError handling
… port parametrize
…d assertions, parametrize credentials
…_recipients tests
…d and prompt assertions
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
tests/unit/tools/communication/test_notification_sender.py (1)
69-99: 🧹 Nitpick | 🔵 TrivialParametrize invalid category/severity cases to remove duplicated test flow.
Lines 69-99 repeat the same setup/execute/assert pattern and can be merged into one parameterized async test.
As per coding guidelines: `tests/**/*.py`: Prefer `@pytest.mark.parametrize` for testing similar cases.♻️ Proposed refactor
- async def test_execute_invalid_category( - self, - mock_dispatcher: MockNotificationDispatcher, - ) -> None: - tool = NotificationSenderTool(dispatcher=mock_dispatcher) - result = await tool.execute( - arguments={ - "category": "invalid", - "severity": "info", - "title": "Test", - "source": "test", - } - ) - assert result.is_error - assert "Invalid category" in result.content - - async def test_execute_invalid_severity( - self, - mock_dispatcher: MockNotificationDispatcher, - ) -> None: - tool = NotificationSenderTool(dispatcher=mock_dispatcher) - result = await tool.execute( - arguments={ - "category": "system", - "severity": "invalid", - "title": "Test", - "source": "test", - } - ) - assert result.is_error - assert "Invalid severity" in result.content + `@pytest.mark.parametrize`( + ("category", "severity", "expected_error"), + [ + ("invalid", "info", "Invalid category"), + ("system", "invalid", "Invalid severity"), + ], + ids=["invalid_category", "invalid_severity"], + ) + async def test_execute_invalid_inputs( + self, + mock_dispatcher: MockNotificationDispatcher, + category: str, + severity: str, + expected_error: str, + ) -> None: + tool = NotificationSenderTool(dispatcher=mock_dispatcher) + result = await tool.execute( + arguments={ + "category": category, + "severity": severity, + "title": "Test", + "source": "test", + } + ) + assert result.is_error + assert expected_error in result.content🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/tools/communication/test_notification_sender.py` around lines 69 - 99, Merge the two duplicate async tests test_execute_invalid_category and test_execute_invalid_severity into a single parameterized async test using pytest.mark.parametrize; call NotificationSenderTool(dispatcher=mock_dispatcher) once per parameter set and invoke NotificationSenderTool.execute with argument dictionaries for each case, and assert result.is_error and that result.content contains the expected substring (e.g., "Invalid category" or "Invalid severity") based on the parameter; ensure the test name reflects both cases and keep the same mock_dispatcher fixture and async signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/operations.md`:
- Around line 695-696: The sentence that groups "git, file system" under
`VERSION_CONTROL` is incorrect; update the phrasing so file system and version
control are distinct categories (e.g., replace the phrase "git, file system are
part of the default toolset and always registered" with wording like "the
core/default toolset includes file system and version control tools (e.g., git)
which are always registered"), ensuring you reference `File System` and
`VERSION_CONTROL` as separate categories to avoid category drift.
In `@tests/unit/tools/analytics/test_report_generator.py`:
- Around line 43-93: Consolidate the three nearly-identical tests
(test_execute_markdown_report, test_execute_text_report,
test_execute_json_report) into a single pytest.mark.parametrize test that calls
ReportGeneratorTool(provider=mock_provider).execute once per case; parametrize
over the differing inputs (report_type, period, format) and expected assertions
(e.g., substring checks for markdown/text or JSON keys for json). In the new
test use the same mock_provider fixture, run the async execute call, assert
result.is_error is False, then branch on the format parameter to perform the
appropriate content checks (markdown header and metadata check for "markdown",
substring checks for "text", json.loads and key/value assertions for "json").
Update or remove the old three test functions (test_execute_markdown_report,
test_execute_text_report, test_execute_json_report) and keep the new
parameterized test named clearly (e.g., test_execute_report_variants).
In `@tests/unit/tools/communication/test_email_sender.py`:
- Around line 135-149: The test test_execute_rejects_newline_in_address should
assert the SMTP send path was not invoked: after calling EmailSenderTool.execute
and asserting result.is_error and the error message, add an assertion that the
mocked send function (mock_send) was not called so the _send_sync path is
short-circuited; reference the mock_send fixture and
EmailSenderTool.execute/_send_sync to locate where to add
mock_send.assert_not_called().
In `@tests/unit/tools/design/test_diagram_generator.py`:
- Around line 108-129: Combine the two duplicate tests into one parameterized
test using pytest.mark.parametrize: create a single async test (e.g.,
test_execute_invalid_inputs) that parametrizes tuples of arguments and expected
error substrings, referencing DiagramGeneratorTool and its execute method; for
each case call tool.execute(arguments=...) and assert result.is_error and that
the expected substring (e.g., "Invalid diagram_type" or "Invalid output_format")
is in result.content. Ensure the parameter table includes both the
{"diagram_type":"invalid","description":"test"} case and the
{"diagram_type":"flowchart","description":"test","output_format":"pdf"} case.
In `@tests/unit/tools/design/test_image_generator.py`:
- Around line 58-77: The test test_execute_passes_all_params (and the similar
test around lines 94-105) indexes mock_provider.calls[0] without verifying the
call count; add an assertion like assert len(mock_provider.calls) == 1
immediately before accessing mock_provider.calls[0] to ensure exactly one
invocation occurred and make failures clearer; update both tests that reference
mock_provider.calls to include this check (reference MockImageProvider and the
mock_provider variable in ImageGeneratorTool tests).
---
Duplicate comments:
In `@tests/unit/tools/communication/test_notification_sender.py`:
- Around line 69-99: Merge the two duplicate async tests
test_execute_invalid_category and test_execute_invalid_severity into a single
parameterized async test using pytest.mark.parametrize; call
NotificationSenderTool(dispatcher=mock_dispatcher) once per parameter set and
invoke NotificationSenderTool.execute with argument dictionaries for each case,
and assert result.is_error and that result.content contains the expected
substring (e.g., "Invalid category" or "Invalid severity") based on the
parameter; ensure the test name reflects both cases and keep the same
mock_dispatcher fixture and async signature.
🪄 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: 0ae8fb52-0f75-4afd-912e-18b96590eaf4
📒 Files selected for processing (8)
docs/design/operations.mdtests/unit/tools/analytics/test_config.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/communication/test_notification_sender.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect new reality when approved deviations occur
Files:
docs/design/operations.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
No
from __future__ import annotations-- Python 3.14 has PEP 649Use
except A, B:(no parentheses) for PEP 758 except syntax on Python 3.14All public functions require type hints and mypy strict mode
Docstrings must use Google style and are required on public classes/functions (enforced by ruff D rules)
Create new objects, never mutate existing ones. For non-Pydantic internal collections, use
copy.deepcopy()at construction +MappingProxyTypewrappingUse frozen Pydantic models for config/identity; separate mutable-via-copy models for runtime state that evolves
Use Pydantic v2 with
allow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fieldsUse
@computed_fieldfor derived values instead of storing + validating redundant fieldsUse
NotBlankStr(fromcore.types) for all identifier/name fields instead of manual whitespace validatorsPrefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new codeLine length must be 88 characters (ruff)
Functions must be less than 50 lines, files less than 800 lines
Handle errors explicitly, never silently swallow them
Validate at system boundaries (user input, external APIs, config files)
Files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/communication/test_email_sender.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
test-provider,test-small-001, etc. in tests instead of real vendor names (no Anthropic, OpenAI, Claude, GPT, etc.)Use
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowmarkers in testsCoverage must be 80% minimum (enforced in CI)
Async tests use
asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneededPrefer
@pytest.mark.parametrizefor testing similar casesUse Hypothesis with
@given+@settingsfor property-based testing in PythonWhen Hypothesis finds a failure, it is a real bug -- fix the underlying bug and add an
@example(...) decorator permanentlyNever skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally
Mock
time.monotonic()andasyncio.sleep()in timing-sensitive tests instead of widening timing marginsUse
asyncio.Event().wait()instead ofasyncio.sleep(large_number)for indefinite blocking that must be cancellation-safe
Files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/communication/test_email_sender.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/communication/test_email_sender.py
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: If implementation deviates from the design spec, alert the user and explain why -- user decides whether to proceed or update the spec
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: At every phase of planning and implementation, be critical and actively look for ways to improve the design
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Always include `-n 8` when running pytest locally for parallelism, never run tests sequentially
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Git commit format: `<type>: <description>` with types: feat, fix, refactor, docs, test, chore, perf, ci
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Signed commits are required on `main` via branch protection -- all commits must be GPG/SSH signed
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Branches must follow format `<type>/<slug>` from main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: NEVER use `cd` in Bash commands -- use absolute paths or run commands directly
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: `bash -c "cd <dir> && <cmd>"` is safe for running in a child process without cwd side effects
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: NEVER use Bash to write or modify files -- use Write or Edit tools instead
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: After finishing an issue implementation, create a feature branch, commit, and push -- do NOT create a PR automatically
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Do NOT leave work uncommitted on main -- branch, commit, push immediately after finishing
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: NEVER create a PR directly -- ALWAYS use `/pre-pr-review` to create PRs
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: For trivial/docs-only changes, use `/pre-pr-review quick` to skip agents but still run automated checks
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Fix everything valid from review agents -- never skip, no deferring, no "out of scope" skipping
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T07:27:31.930Z
Learning: Preserve existing `Closes `#NNN`` references in PR issue references -- never remove unless explicitly asked
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue — DESIGN_SPEC.md is a pointer file linking to 7 design pages (Agents, Organization, Communication, Engine, Memory, Operations)
Applied to files:
docs/design/operations.md
📚 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: Always read the relevant `docs/design/` page before implementing any feature or planning any issue. DESIGN_SPEC.md is a pointer file linking to the 7 design pages (index, agents, organization, communication, engine, memory, operations).
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
docs/design/operations.md
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to src/synthorg/**/*.py : If implementation deviates from the design spec (better approach found, scope evolved), alert the user and explain why - user decides whether to proceed or update the spec
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to docs/design/*.md : Update the relevant `docs/design/` page when approved deviations occur to reflect the new reality
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Update the relevant `docs/design/` page to reflect new reality when approved deviations from spec occur
Applied to files:
docs/design/operations.md
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to docs/design/**/*.md : Always read the relevant docs/design/ page before implementing a feature or planning an issue - DESIGN_SPEC.md is a pointer to the 12 design pages
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to tests/**/*.py : Use pytest.mark.parametrize for testing similar cases instead of multiple separate test functions
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-03-31T20:29:10.177Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:29:10.177Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.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/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.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/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.pytests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_config.pytests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; mock `time.monotonic()` and `asyncio.sleep()` for determinism; use `asyncio.Event().wait()` for indefinite blocking instead of `asyncio.sleep(large_number)`
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_config.pytests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to tests/**/*.py : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests - use generic names: test-provider, test-small-001, etc.
Applied to files:
tests/unit/tools/communication/test_notification_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.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 : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
tests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/design/test_image_generator.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/analytics/test_config.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of real vendor names
Applied to files:
tests/unit/tools/analytics/test_report_generator.pytests/unit/tools/design/test_image_generator.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to **/*.py : Use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf values at validation time
Applied to files:
tests/unit/tools/analytics/test_config.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:
tests/unit/tools/analytics/test_config.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to tests/**/*.py : Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mocking `time.monotonic()` and `asyncio.sleep()` for determinism or using `asyncio.Event().wait()` for indefinite blocking
Applied to files:
tests/unit/tools/communication/test_email_sender.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to tests/**/*.py : For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/tools/communication/test_email_sender.py
🔇 Additional comments (7)
tests/unit/tools/design/test_diagram_generator.py (2)
13-25: Nice metadata coverage with compact parametrization.Clean and maintainable assertions for core tool identity fields.
92-106: Great hardening of Graphviz title escaping behavior.The escaped-positive plus unescaped-negative assertions make this security-relevant case robust.
tests/unit/tools/design/test_image_generator.py (3)
46-57: Strong success-path assertion coverage.This test validates both output semantics and provider invocation side effects clearly.
127-165: Validation-path tests are well structured and regression-resistant.Good use of parametrization plus explicit “provider not called” assertions for invalid inputs.
19-45: 🧹 Nitpick | 🔵 TrivialParameterize descriptor-invariant tests to reduce repetition.
test_category_is_design,test_action_type_is_docs_write, andtest_nameshare identical setup and can be consolidated into one parametrized test for maintainability.As per coding guidelines: "Prefer
@pytest.mark.parametrizefor testing similar cases."⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-17T22:08:13.456Z Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-09T07:26:54.377Z Learning: Applies to tests/**/*.py : Use pytest.mark.parametrize for testing similar cases instead of multiple separate test functionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-15T18:28:13.207Z Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T20:29:10.177Z Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar casesLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-02T07:18:02.381Z Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar casestests/unit/tools/analytics/test_config.py (1)
11-61: Looks good — validation coverage is strong and well-structured.This suite cleanly covers defaults, immutability, invalid numeric inputs, and metric-name validation for
AnalyticsToolsConfig.tests/unit/tools/analytics/test_metric_collector.py (1)
150-154: No issue found. TheMetricSinkprotocol is properly decorated with@runtime_checkable, making theisinstance()check on line 154 valid.> Likely an incorrect or invalid review comment.
Comment on lines
+43
to
+93
| async def test_execute_markdown_report( | ||
| self, | ||
| mock_provider: MockAnalyticsProvider, | ||
| ) -> None: | ||
| tool = ReportGeneratorTool(provider=mock_provider) | ||
| result = await tool.execute( | ||
| arguments={ | ||
| "report_type": "budget_summary", | ||
| "period": "30d", | ||
| "format": "markdown", | ||
| } | ||
| ) | ||
| assert not result.is_error | ||
| assert "# Budget Summary Report" in result.content | ||
| assert "**Period:** 30d" in result.content | ||
| assert result.metadata["format"] == "markdown" | ||
|
|
||
| async def test_execute_text_report( | ||
| self, | ||
| mock_provider: MockAnalyticsProvider, | ||
| ) -> None: | ||
| tool = ReportGeneratorTool(provider=mock_provider) | ||
| result = await tool.execute( | ||
| arguments={ | ||
| "report_type": "performance", | ||
| "period": "7d", | ||
| "format": "text", | ||
| } | ||
| ) | ||
| assert not result.is_error | ||
| assert "Performance Report" in result.content | ||
| assert "Period: 7d" in result.content | ||
|
|
||
| async def test_execute_json_report( | ||
| self, | ||
| mock_provider: MockAnalyticsProvider, | ||
| ) -> None: | ||
| tool = ReportGeneratorTool(provider=mock_provider) | ||
| result = await tool.execute( | ||
| arguments={ | ||
| "report_type": "cost_breakdown", | ||
| "period": "90d", | ||
| "format": "json", | ||
| } | ||
| ) | ||
| assert not result.is_error | ||
| parsed = json.loads(result.content) | ||
| assert parsed["report_type"] == "cost_breakdown" | ||
| assert parsed["period"] == "90d" | ||
| assert "data" in parsed | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Parametrize the three format-success execution tests to reduce duplication.
test_execute_markdown_report, test_execute_text_report, and test_execute_json_report share the same setup and only vary by input/expectations. Consolidating them into one parameterized test will improve maintainability.
As per coding guidelines, tests/**/*.py: Prefer @pytest.mark.parametrize for testing similar cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/tools/analytics/test_report_generator.py` around lines 43 - 93,
Consolidate the three nearly-identical tests (test_execute_markdown_report,
test_execute_text_report, test_execute_json_report) into a single
pytest.mark.parametrize test that calls
ReportGeneratorTool(provider=mock_provider).execute once per case; parametrize
over the differing inputs (report_type, period, format) and expected assertions
(e.g., substring checks for markdown/text or JSON keys for json). In the new
test use the same mock_provider fixture, run the async execute call, assert
result.is_error is False, then branch on the format parameter to perform the
appropriate content checks (markdown header and metadata check for "markdown",
substring checks for "text", json.loads and key/value assertions for "json").
Update or remove the old three test functions (test_execute_markdown_report,
test_execute_text_report, test_execute_json_report) and keep the new
parameterized test named clearly (e.g., test_execute_report_variants).
7231366 to
8871f29
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (12)
src/synthorg/tools/design/base_design_tool.py (1)
7-13:⚠️ Potential issue | 🟠 MajorAdd required module logger initialization.
This business-logic module is missing the standard logger setup.
♻️ Proposed fix
from abc import ABC from typing import Any +from synthorg.observability import get_logger from synthorg.core.enums import ToolCategory from synthorg.tools.base import BaseTool from synthorg.tools.design.config import DesignToolsConfig +logger = get_logger(__name__) +As per coding guidelines:
src/synthorg/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/design/base_design_tool.py` around lines 7 - 13, Module is missing the required logger initialization; add "from synthorg.observability import get_logger" to the imports and create a module-level logger with "logger = get_logger(__name__)" (placed after imports) in src/synthorg/tools/design/base_design_tool.py so that this business-logic module follows the project logging convention used by classes like BaseTool and the DesignToolsConfig-using code.src/synthorg/tools/communication/base_communication_tool.py (1)
7-13:⚠️ Potential issue | 🟠 MajorAdd required module logger initialization.
This business-logic module is missing the standard logger setup.
♻️ Proposed fix
from abc import ABC from typing import Any +from synthorg.observability import get_logger from synthorg.core.enums import ToolCategory from synthorg.tools.base import BaseTool from synthorg.tools.communication.config import CommunicationToolsConfig +logger = get_logger(__name__) +As per coding guidelines:
src/synthorg/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/communication/base_communication_tool.py` around lines 7 - 13, The module is missing the required logger initialization; add the import "from synthorg.observability import get_logger" at the top of src/synthorg/tools/communication/base_communication_tool.py and then initialize "logger = get_logger(__name__)" (near the existing imports that include ToolCategory, BaseTool, CommunicationToolsConfig) so the module follows the project guideline for logging.src/synthorg/tools/analytics/base_analytics_tool.py (1)
8-14:⚠️ Potential issue | 🟠 MajorAdd the required module logger initialization.
This business-logic module is missing the standard
get_loggerimport andlogger = get_logger(__name__)bootstrap.As per coding guidelines: `src/synthorg/**/*.py`: Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`.♻️ Proposed fix
from abc import ABC from typing import Any from synthorg.core.enums import ToolCategory +from synthorg.observability import get_logger from synthorg.tools.analytics.config import AnalyticsToolsConfig from synthorg.tools.base import BaseTool +logger = get_logger(__name__) +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/analytics/base_analytics_tool.py` around lines 8 - 14, The module is missing the observability logger initialization; add an import for get_logger from synthorg.observability and initialize logger = get_logger(__name__) near the other top-level imports in src/synthorg/tools/analytics/base_analytics_tool.py so that the module (which defines AnalyticsToolsConfig, ToolCategory and BaseTool) uses the standard logger bootstrap.tests/unit/tools/analytics/test_report_generator.py (1)
43-93: 🧹 Nitpick | 🔵 TrivialParameterize the three format-success tests to reduce duplication.
test_execute_markdown_report,test_execute_text_report, andtest_execute_json_reportfollow the same arrange/act/assert shape and can be collapsed into one table-driven test.As per coding guidelines: "
tests/**/*.py: Prefer@pytest.mark.parametrizefor testing similar cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/tools/analytics/test_report_generator.py` around lines 43 - 93, Collapse the three nearly identical tests (test_execute_markdown_report, test_execute_text_report, test_execute_json_report) into a single parametrized pytest using `@pytest.mark.parametrize` that iterates over cases for "format" (markdown, text, json), the report_type and period for each case, and expected assertions; call ReportGeneratorTool(provider=mock_provider).execute(...) inside the single test and branch assertions by format (for "markdown" check "# Budget Summary Report" and metadata["format"] == "markdown"; for "text" check "Performance Report" and "Period: 7d"; for "json" call json.loads(result.content) and assert parsed["report_type"], parsed["period"], and "data" present), ensuring result.is_error is asserted false for every case and reusing mock_provider for setup.src/synthorg/tools/communication/template_formatter.py (1)
98-203: 🛠️ Refactor suggestion | 🟠 MajorRefactor
execute()into smaller helpers to meet function-size policy.
execute()currently bundles validation, template compilation, rendering, and response assembly in one long method. Splitting those branches into focused helpers will improve readability and keep policy compliance.As per coding guidelines: "Keep functions under 50 lines and files under 800 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/communication/template_formatter.py` around lines 98 - 203, The execute method is too large and should be split into focused helpers: create a validate_arguments(arguments) helper to validate "template", "variables", and "format" and return normalized (template_str, variables, output_format) or a ToolExecutionResult error; a compile_template(env, template_str) helper that wraps env.from_string and converts TemplateSyntaxError into the same ToolExecutionResult error used now; a render_template(tmpl, variables) helper that calls tmpl.render(**variables) and preserves the MemoryError/RecursionError re-raise while converting other exceptions into the existing COMM_TOOL_TEMPLATE_RENDER_FAILED ToolExecutionResult; and a build_success_result(rendered, output_format) helper that returns the final ToolExecutionResult including metadata. Keep existing logging calls (COMM_TOOL_TEMPLATE_RENDER_START/INVALID/FAILED/SUCCESS), use self._env or self._env_autoesc based on output_format exactly as now, and ensure execute orchestrates these helpers and returns the same ToolExecutionResult values.src/synthorg/tools/analytics/report_generator.py (1)
173-215:⚠️ Potential issue | 🟠 MajorValidate
formatbefore the enum check.
report_typeandperiodare guarded, butformatis still treated asAny. A payload like{"format": []}raisesTypeErrorinoutput_format not in _OUTPUT_FORMATSinstead of returning aToolExecutionResult.🛡️ Suggested fix
- output_format: str = arguments.get("format", "markdown") + output_format = arguments.get("format", "markdown") + if not isinstance(output_format, str): + logger.warning( + ANALYTICS_TOOL_REPORT_FAILED, + error="missing_or_invalid_output_format", + ) + return ToolExecutionResult( + content="'format' must be a string.", + is_error=True, + )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/tools/analytics/report_generator.py` around lines 173 - 215, The output_format retrieved via arguments.get("format", "markdown") isn't type-checked before doing "output_format not in _OUTPUT_FORMATS", so non-string inputs (e.g. list) raise TypeError; add a guard that checks isinstance(output_format, str) (or otherwise coerces to str) before the membership check, and if the type is invalid, log ANALYTICS_TOOL_REPORT_FAILED with error="invalid_output_format_type" and return a ToolExecutionResult with an appropriate error message (similar style to the other guards) referencing output_format and _OUTPUT_FORMATS; update the validation logic around output_format to perform the type check first, then the membership check.src/synthorg/tools/communication/notification_sender.py (1)
221-221:⚠️ Potential issue | 🟡 MinorUse the repo's PEP 758 multi-except form here.
This handler is still using the parenthesized form, while the rest of the file already uses the repo's Python 3.14 convention.
🔎 Read-only verification
#!/bin/bash rg -n 'except \(' src/synthorg/tools/communication/notification_sender.pyAs per coding guidelines, "Use PEP 758 except syntax:
except A, B:(no parentheses) -- ruff enforces this on Python 3.14."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/communication/notification_sender.py` at line 221, Replace the parenthesized multi-except in notification_sender.py (the line currently reading "except (ValueError, TypeError, ValidationError) as exc:") with the repo's PEP 758 form: "except ValueError, TypeError, ValidationError as exc:" so it matches the rest of the file and ruff expectations; update that exception handler in the function/method where ValidationError is caught and run tests/lint to verify.src/synthorg/tools/design/image_generator.py (1)
195-245:⚠️ Potential issue | 🟠 MajorValidate
promptand argument types before the range/enum checks.
arguments["prompt"]can still raiseKeyError,width/heightcomparisons will raise on strings or lists, andboolslips through as anint. This tool should reject malformed payloads with a normalToolExecutionResultbefore the provider call.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/tools/design/image_generator.py` around lines 195 - 245, The code is not validating input types and presence before performing range/enum checks; update the beginning of the handler that reads arguments (the arguments dict and prompt extraction) so it first verifies that "prompt" exists and is a non-empty string, that "width" and "height" are integers (reject booleans and non-numeric types) and within _MIN_DIMENSION.._MAX_DIMENSION, and that "style" and "quality" are strings before checking membership in _VALID_STYLES/_VALID_QUALITIES; on any malformed input return a ToolExecutionResult(is_error=True) with a clear message and call logger.warning(DESIGN_IMAGE_GENERATION_FAILED, error=..., ...) referencing the same error codes used elsewhere so malformed payloads are rejected before any provider calls (update the code paths around prompt, width, height, style, quality checks and the ToolExecutionResult returns).src/synthorg/tools/communication/email_sender.py (1)
150-203:⚠️ Potential issue | 🟠 MajorReject malformed recipient and body fields before using them.
cc/bcccan still be passed as strings or mixed-type lists, andbody/body_is_htmlare trusted without type checks. Inputs like{"cc": "x@y.com"}or{"to": [123]}can fail inall_recipients = ...,_UNSAFE_ADDR_RE.search(addr), or_send_sync()before the tool reaches its intended validation error path.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/tools/communication/email_sender.py` around lines 150 - 203, Normalize and validate recipient and body fields before any use: ensure "to" (to_addrs) is a list and every element is a string (reject and return ToolExecutionResult with COMM_TOOL_EMAIL_VALIDATION_FAILED if not), accept "cc"/"bcc" when passed as a single string by converting to a single-item list or reject if they are non-list/non-string or contain non-string elements, validate "body" is a str and "body_is_html" is a bool before using them, and only after normalization build all_recipients and run the _UNSAFE_ADDR_RE.search(addr) loop; log the appropriate reason codes ("invalid_to", "invalid_cc", "invalid_bcc", "invalid_body", "invalid_body_is_html") and include counts/limits consistent with existing pattern so malformed inputs never reach _send_sync() or cause type errors when checking email_config.from_address or computing len(all_recipients).src/synthorg/tools/analytics/data_aggregator.py (1)
262-272:⚠️ Potential issue | 🟡 MinorMetrics list items are not validated as strings.
Line 264 checks that
metricsis a non-empty list, but doesn't validate that all items are strings. Non-string values (e.g.,[123, None]) would be passed to the provider'squery()method, which expectslist[str].🛡️ Proposed fix to validate metrics items
metrics = arguments.get("metrics") period = arguments.get("period") - if not isinstance(metrics, list) or not metrics: + if ( + not isinstance(metrics, list) + or not metrics + or not all(isinstance(m, str) and m.strip() for m in metrics) + ): logger.warning( ANALYTICS_TOOL_QUERY_FAILED, error="missing_or_invalid_metrics", ) return ToolExecutionResult( content="'metrics' must be a non-empty list of strings.", is_error=True, )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/tools/analytics/data_aggregator.py` around lines 262 - 272, The metrics argument is checked only for being a non-empty list but individual items aren't validated as strings; update the validation in data_aggregator.py around the metrics handling (the variables metrics, arguments, and the logger/ANALYTICS_TOOL_QUERY_FAILED branch) to ensure every item in metrics is an instance of str (and not empty) before calling the provider.query() method, and if any item fails validation return a ToolExecutionResult with is_error=True and a clear message like "'metrics' must be a non-empty list of strings" while logging the ANALYTICS_TOOL_QUERY_FAILED error with context; ensure the provider.query() is only invoked after this validation passes.src/synthorg/tools/analytics/metric_collector.py (1)
172-173:⚠️ Potential issue | 🟡 MinorTags dict values are not validated as strings.
The type annotation specifies
dict[str, str], but line 173 only checks ifraw_tagsis adict— it doesn't validate that all values are strings. Non-string values would be passed toMetricSink.record(), which expectsdict[str, str].🛡️ Proposed fix to validate tag values
raw_tags = arguments.get("tags") - tags: dict[str, str] = raw_tags if isinstance(raw_tags, dict) else {} + if isinstance(raw_tags, dict): + tags: dict[str, str] = { + k: v for k, v in raw_tags.items() + if isinstance(k, str) and isinstance(v, str) + } + else: + tags = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/analytics/metric_collector.py` around lines 172 - 173, The code assigns raw_tags to tags without ensuring values are strings, violating the dict[str, str] contract and risking passes to MetricSink.record with non-string values; update the block that reads arguments.get("tags") so that if raw_tags is a dict you produce tags = {k: str(v) for k, v in raw_tags.items() if k is not None} (or alternatively filter out non-string keys/values) to guarantee all values are strings before passing tags to MetricSink.record; reference raw_tags, tags, arguments.get("tags") and MetricSink.record when making the change.src/synthorg/tools/design/asset_manager.py (1)
107-132:⚠️ Potential issue | 🟡 MinorValidate
asset_idtype before calling.strip()inregister_asset.The method signature declares
asset_id: str, but runtime callers could pass non-string values. Calling.strip()on a non-string (e.g.,None,int) will raiseAttributeErrorinstead of a clearValueError.🛡️ Proposed fix to add type validation
def register_asset( self, asset_id: str, metadata: dict[str, Any], ) -> None: ... - if not asset_id.strip(): + if not isinstance(asset_id, str) or not asset_id.strip(): msg = "asset_id must not be empty" raise ValueError(msg) + if not isinstance(metadata, dict): + msg = "metadata must be a dictionary" + raise TypeError(msg) self._assets[asset_id] = copy.deepcopy(metadata)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/tools/design/asset_manager.py` around lines 107 - 132, The register_asset method should validate asset_id is a string before calling .strip() to avoid AttributeError for non-string inputs; update register_asset to first check isinstance(asset_id, str) (or explicitly reject None/int/etc.) and raise a ValueError with a clear message if not a str, then proceed to call asset_id.strip() and store a deepcopy of metadata into self._assets and log with DESIGN_ASSET_STORED via logger.info as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/tools/analytics/data_aggregator.py`:
- Around line 197-217: The validation loop using datetime.fromisoformat in
data_aggregator.py can raise TypeError for non-string inputs but only catches
ValueError; update the validation in the loop (the for date_label, date_val ...
block that calls datetime.fromisoformat) to handle both ValueError and TypeError
(or coerce/verify that date_val is a str before calling) so non-string inputs
return the same structured ToolExecutionResult and logger.warning
(ANALYTICS_TOOL_QUERY_FAILED with error="invalid_date") instead of crashing.
In `@src/synthorg/tools/communication/email_sender.py`:
- Around line 283-289: The recipient headers are being assigned lists directly
to the EmailMessage (msg["To"] = to_addrs and msg["Cc"] = cc_addrs) instead of
following the project convention of joining addresses into a single
comma-separated string; update the block around safe_subject / msg to join
to_addrs with ", " before assigning to msg["To"] and, if cc_addrs is truthy,
join cc_addrs with ", " before assigning to msg["Cc"] so the headers are
explicit and consistent with the pattern used in EmailMessage and in
src/synthorg/notifications/adapters/email.py.
In `@src/synthorg/tools/design/diagram_generator.py`:
- Around line 127-158: The handler currently indexes arguments["diagram_type"]
and ["description"] and does set-membership checks against
_DIAGRAM_TYPES/_OUTPUT_FORMATS without validating input shape or types; this can
raise KeyError/TypeError. Fix by first validating that required keys
("diagram_type" and "description") exist and that diagram_type, output_format
(and title if present) are strings (use isinstance checks) before any membership
checks; if missing or wrong type, log DESIGN_DIAGRAM_GENERATION_FAILED via
logger with an appropriate error code and return a ToolExecutionResult
describing the problem instead of proceeding to the membership tests in
diagram_generator.py (check symbols: diagram_type, description, title,
output_format, _DIAGRAM_TYPES, _OUTPUT_FORMATS, ToolExecutionResult,
DESIGN_DIAGRAM_GENERATION_FAILED).
In `@tests/unit/tools/analytics/test_metric_collector.py`:
- Around line 66-80: In test_execute_with_tags, assert the sink was invoked
exactly once before indexing into mock_sink.recorded: add assert
len(mock_sink.recorded) == 1 immediately before recorded = mock_sink.recorded[0]
to lock the expected call count and make failures clearer; this touches the
test_execute_with_tags function and the mock_sink.recorded usage for the
MetricCollectorTool sink invocation.
In `@tests/unit/tools/communication/test_template_formatter.py`:
- Around line 54-101: Add unit tests that assert TemplateFormatterTool.execute
validates argument types: create new async tests in
tests/unit/tools/communication/test_template_formatter.py that call
TemplateFormatterTool().execute with (1) template as non-str (e.g., 123)
expecting result.is_error and an error message about template type, (2)
variables as non-dict (e.g., "notadict") expecting result.is_error and an error
message about variables type, and (3) format as non-str (e.g., 123) expecting
result.is_error and an error message about format type; reference
TemplateFormatterTool.execute in your tests and assert both is_error and that
result.content contains the appropriate "Invalid" or type-specific message to
lock in the defensive validation paths.
In `@tests/unit/tools/test_factory.py`:
- Around line 469-487: The test test_from_config_wires_analytics_tools builds
tools via build_default_tools_from_config and checks the set names = {t.name for
t in tools} but only asserts absence of "data_aggregator" and
"metric_collector"; add a third negative assertion assert "report_generator" not
in names to ensure no analytics tool named "report_generator" is created when
AnalyticsToolsConfig has no backends. Update the test function
test_from_config_wires_analytics_tools to include this additional assertion.
---
Duplicate comments:
In `@src/synthorg/tools/analytics/base_analytics_tool.py`:
- Around line 8-14: The module is missing the observability logger
initialization; add an import for get_logger from synthorg.observability and
initialize logger = get_logger(__name__) near the other top-level imports in
src/synthorg/tools/analytics/base_analytics_tool.py so that the module (which
defines AnalyticsToolsConfig, ToolCategory and BaseTool) uses the standard
logger bootstrap.
In `@src/synthorg/tools/analytics/data_aggregator.py`:
- Around line 262-272: The metrics argument is checked only for being a
non-empty list but individual items aren't validated as strings; update the
validation in data_aggregator.py around the metrics handling (the variables
metrics, arguments, and the logger/ANALYTICS_TOOL_QUERY_FAILED branch) to ensure
every item in metrics is an instance of str (and not empty) before calling the
provider.query() method, and if any item fails validation return a
ToolExecutionResult with is_error=True and a clear message like "'metrics' must
be a non-empty list of strings" while logging the ANALYTICS_TOOL_QUERY_FAILED
error with context; ensure the provider.query() is only invoked after this
validation passes.
In `@src/synthorg/tools/analytics/metric_collector.py`:
- Around line 172-173: The code assigns raw_tags to tags without ensuring values
are strings, violating the dict[str, str] contract and risking passes to
MetricSink.record with non-string values; update the block that reads
arguments.get("tags") so that if raw_tags is a dict you produce tags = {k:
str(v) for k, v in raw_tags.items() if k is not None} (or alternatively filter
out non-string keys/values) to guarantee all values are strings before passing
tags to MetricSink.record; reference raw_tags, tags, arguments.get("tags") and
MetricSink.record when making the change.
In `@src/synthorg/tools/analytics/report_generator.py`:
- Around line 173-215: The output_format retrieved via arguments.get("format",
"markdown") isn't type-checked before doing "output_format not in
_OUTPUT_FORMATS", so non-string inputs (e.g. list) raise TypeError; add a guard
that checks isinstance(output_format, str) (or otherwise coerces to str) before
the membership check, and if the type is invalid, log
ANALYTICS_TOOL_REPORT_FAILED with error="invalid_output_format_type" and return
a ToolExecutionResult with an appropriate error message (similar style to the
other guards) referencing output_format and _OUTPUT_FORMATS; update the
validation logic around output_format to perform the type check first, then the
membership check.
In `@src/synthorg/tools/communication/base_communication_tool.py`:
- Around line 7-13: The module is missing the required logger initialization;
add the import "from synthorg.observability import get_logger" at the top of
src/synthorg/tools/communication/base_communication_tool.py and then initialize
"logger = get_logger(__name__)" (near the existing imports that include
ToolCategory, BaseTool, CommunicationToolsConfig) so the module follows the
project guideline for logging.
In `@src/synthorg/tools/communication/email_sender.py`:
- Around line 150-203: Normalize and validate recipient and body fields before
any use: ensure "to" (to_addrs) is a list and every element is a string (reject
and return ToolExecutionResult with COMM_TOOL_EMAIL_VALIDATION_FAILED if not),
accept "cc"/"bcc" when passed as a single string by converting to a single-item
list or reject if they are non-list/non-string or contain non-string elements,
validate "body" is a str and "body_is_html" is a bool before using them, and
only after normalization build all_recipients and run the
_UNSAFE_ADDR_RE.search(addr) loop; log the appropriate reason codes
("invalid_to", "invalid_cc", "invalid_bcc", "invalid_body",
"invalid_body_is_html") and include counts/limits consistent with existing
pattern so malformed inputs never reach _send_sync() or cause type errors when
checking email_config.from_address or computing len(all_recipients).
In `@src/synthorg/tools/communication/notification_sender.py`:
- Line 221: Replace the parenthesized multi-except in notification_sender.py
(the line currently reading "except (ValueError, TypeError, ValidationError) as
exc:") with the repo's PEP 758 form: "except ValueError, TypeError,
ValidationError as exc:" so it matches the rest of the file and ruff
expectations; update that exception handler in the function/method where
ValidationError is caught and run tests/lint to verify.
In `@src/synthorg/tools/communication/template_formatter.py`:
- Around line 98-203: The execute method is too large and should be split into
focused helpers: create a validate_arguments(arguments) helper to validate
"template", "variables", and "format" and return normalized (template_str,
variables, output_format) or a ToolExecutionResult error; a
compile_template(env, template_str) helper that wraps env.from_string and
converts TemplateSyntaxError into the same ToolExecutionResult error used now; a
render_template(tmpl, variables) helper that calls tmpl.render(**variables) and
preserves the MemoryError/RecursionError re-raise while converting other
exceptions into the existing COMM_TOOL_TEMPLATE_RENDER_FAILED
ToolExecutionResult; and a build_success_result(rendered, output_format) helper
that returns the final ToolExecutionResult including metadata. Keep existing
logging calls (COMM_TOOL_TEMPLATE_RENDER_START/INVALID/FAILED/SUCCESS), use
self._env or self._env_autoesc based on output_format exactly as now, and ensure
execute orchestrates these helpers and returns the same ToolExecutionResult
values.
In `@src/synthorg/tools/design/asset_manager.py`:
- Around line 107-132: The register_asset method should validate asset_id is a
string before calling .strip() to avoid AttributeError for non-string inputs;
update register_asset to first check isinstance(asset_id, str) (or explicitly
reject None/int/etc.) and raise a ValueError with a clear message if not a str,
then proceed to call asset_id.strip() and store a deepcopy of metadata into
self._assets and log with DESIGN_ASSET_STORED via logger.info as before.
In `@src/synthorg/tools/design/base_design_tool.py`:
- Around line 7-13: Module is missing the required logger initialization; add
"from synthorg.observability import get_logger" to the imports and create a
module-level logger with "logger = get_logger(__name__)" (placed after imports)
in src/synthorg/tools/design/base_design_tool.py so that this business-logic
module follows the project logging convention used by classes like BaseTool and
the DesignToolsConfig-using code.
In `@src/synthorg/tools/design/image_generator.py`:
- Around line 195-245: The code is not validating input types and presence
before performing range/enum checks; update the beginning of the handler that
reads arguments (the arguments dict and prompt extraction) so it first verifies
that "prompt" exists and is a non-empty string, that "width" and "height" are
integers (reject booleans and non-numeric types) and within
_MIN_DIMENSION.._MAX_DIMENSION, and that "style" and "quality" are strings
before checking membership in _VALID_STYLES/_VALID_QUALITIES; on any malformed
input return a ToolExecutionResult(is_error=True) with a clear message and call
logger.warning(DESIGN_IMAGE_GENERATION_FAILED, error=..., ...) referencing the
same error codes used elsewhere so malformed payloads are rejected before any
provider calls (update the code paths around prompt, width, height, style,
quality checks and the ToolExecutionResult returns).
In `@tests/unit/tools/analytics/test_report_generator.py`:
- Around line 43-93: Collapse the three nearly identical tests
(test_execute_markdown_report, test_execute_text_report,
test_execute_json_report) into a single parametrized pytest using
`@pytest.mark.parametrize` that iterates over cases for "format" (markdown, text,
json), the report_type and period for each case, and expected assertions; call
ReportGeneratorTool(provider=mock_provider).execute(...) inside the single test
and branch assertions by format (for "markdown" check "# Budget Summary Report"
and metadata["format"] == "markdown"; for "text" check "Performance Report" and
"Period: 7d"; for "json" call json.loads(result.content) and assert
parsed["report_type"], parsed["period"], and "data" present), ensuring
result.is_error is asserted false for every case and reusing mock_provider for
setup.
🪄 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: ac992774-b277-4b4e-b4d6-467692f0679c
📒 Files selected for processing (48)
CLAUDE.mddocs/design/operations.mddocs/reference/claude-reference.mdsrc/synthorg/config/defaults.pysrc/synthorg/config/schema.pysrc/synthorg/observability/events/analytics.pysrc/synthorg/observability/events/communication.pysrc/synthorg/observability/events/design.pysrc/synthorg/tools/analytics/__init__.pysrc/synthorg/tools/analytics/base_analytics_tool.pysrc/synthorg/tools/analytics/config.pysrc/synthorg/tools/analytics/data_aggregator.pysrc/synthorg/tools/analytics/metric_collector.pysrc/synthorg/tools/analytics/report_generator.pysrc/synthorg/tools/communication/__init__.pysrc/synthorg/tools/communication/base_communication_tool.pysrc/synthorg/tools/communication/config.pysrc/synthorg/tools/communication/email_sender.pysrc/synthorg/tools/communication/notification_sender.pysrc/synthorg/tools/communication/template_formatter.pysrc/synthorg/tools/design/__init__.pysrc/synthorg/tools/design/asset_manager.pysrc/synthorg/tools/design/base_design_tool.pysrc/synthorg/tools/design/config.pysrc/synthorg/tools/design/diagram_generator.pysrc/synthorg/tools/design/image_generator.pysrc/synthorg/tools/factory.pytests/unit/config/conftest.pytests/unit/observability/test_events.pytests/unit/tools/analytics/__init__.pytests/unit/tools/analytics/conftest.pytests/unit/tools/analytics/test_config.pytests/unit/tools/analytics/test_data_aggregator.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/analytics/test_report_generator.pytests/unit/tools/communication/__init__.pytests/unit/tools/communication/conftest.pytests/unit/tools/communication/test_config.pytests/unit/tools/communication/test_email_sender.pytests/unit/tools/communication/test_notification_sender.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/design/__init__.pytests/unit/tools/design/conftest.pytests/unit/tools/design/test_asset_manager.pytests/unit/tools/design/test_config.pytests/unit/tools/design/test_diagram_generator.pytests/unit/tools/design/test_image_generator.pytests/unit/tools/test_factory.py
Comment on lines
+197
to
+217
| for date_label, date_val in ( | ||
| ("start_date", start_date), | ||
| ("end_date", end_date), | ||
| ): | ||
| if date_val is not None: | ||
| try: | ||
| datetime.fromisoformat(date_val) | ||
| except ValueError: | ||
| logger.warning( | ||
| ANALYTICS_TOOL_QUERY_FAILED, | ||
| error="invalid_date", | ||
| field=date_label, | ||
| value=date_val, | ||
| ) | ||
| return ToolExecutionResult( | ||
| content=( | ||
| f"Invalid {date_label}: {date_val!r}. " | ||
| f"Must be ISO 8601 format." | ||
| ), | ||
| is_error=True, | ||
| ) |
There was a problem hiding this comment.
Date validation may raise TypeError for non-string values.
datetime.fromisoformat() raises TypeError (not ValueError) when passed a non-string value. Since only ValueError is caught, a non-string date like {"start_date": 123} would cause an unhandled exception instead of returning a structured error.
🛡️ Proposed fix to handle non-string dates
for date_label, date_val in (
("start_date", start_date),
("end_date", end_date),
):
if date_val is not None:
+ if not isinstance(date_val, str):
+ logger.warning(
+ ANALYTICS_TOOL_QUERY_FAILED,
+ error="invalid_date_type",
+ field=date_label,
+ )
+ return ToolExecutionResult(
+ content=f"{date_label} must be a string.",
+ is_error=True,
+ )
try:
datetime.fromisoformat(date_val)
except ValueError:As per coding guidelines: "Validate at system boundaries (user input, external APIs, config files)."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for date_label, date_val in ( | |
| ("start_date", start_date), | |
| ("end_date", end_date), | |
| ): | |
| if date_val is not None: | |
| try: | |
| datetime.fromisoformat(date_val) | |
| except ValueError: | |
| logger.warning( | |
| ANALYTICS_TOOL_QUERY_FAILED, | |
| error="invalid_date", | |
| field=date_label, | |
| value=date_val, | |
| ) | |
| return ToolExecutionResult( | |
| content=( | |
| f"Invalid {date_label}: {date_val!r}. " | |
| f"Must be ISO 8601 format." | |
| ), | |
| is_error=True, | |
| ) | |
| for date_label, date_val in ( | |
| ("start_date", start_date), | |
| ("end_date", end_date), | |
| ): | |
| if date_val is not None: | |
| if not isinstance(date_val, str): | |
| logger.warning( | |
| ANALYTICS_TOOL_QUERY_FAILED, | |
| error="invalid_date_type", | |
| field=date_label, | |
| ) | |
| return ToolExecutionResult( | |
| content=f"{date_label} must be a string.", | |
| is_error=True, | |
| ) | |
| try: | |
| datetime.fromisoformat(date_val) | |
| except ValueError: | |
| logger.warning( | |
| ANALYTICS_TOOL_QUERY_FAILED, | |
| error="invalid_date", | |
| field=date_label, | |
| value=date_val, | |
| ) | |
| return ToolExecutionResult( | |
| content=( | |
| f"Invalid {date_label}: {date_val!r}. " | |
| f"Must be ISO 8601 format." | |
| ), | |
| is_error=True, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/tools/analytics/data_aggregator.py` around lines 197 - 217, The
validation loop using datetime.fromisoformat in data_aggregator.py can raise
TypeError for non-string inputs but only catches ValueError; update the
validation in the loop (the for date_label, date_val ... block that calls
datetime.fromisoformat) to handle both ValueError and TypeError (or
coerce/verify that date_val is a str before calling) so non-string inputs return
the same structured ToolExecutionResult and logger.warning
(ANALYTICS_TOOL_QUERY_FAILED with error="invalid_date") instead of crashing.
Comment on lines
+127
to
+158
| diagram_type: str = arguments["diagram_type"] | ||
| description: str = arguments["description"] | ||
| title: str = arguments.get("title", "") | ||
| output_format: str = arguments.get("output_format", "mermaid") | ||
|
|
||
| if diagram_type not in _DIAGRAM_TYPES: | ||
| logger.warning( | ||
| DESIGN_DIAGRAM_GENERATION_FAILED, | ||
| error="invalid_diagram_type", | ||
| diagram_type=diagram_type, | ||
| ) | ||
| return ToolExecutionResult( | ||
| content=( | ||
| f"Invalid diagram_type: {diagram_type!r}. " | ||
| f"Must be one of: {sorted(_DIAGRAM_TYPES)}" | ||
| ), | ||
| is_error=True, | ||
| ) | ||
|
|
||
| if output_format not in _OUTPUT_FORMATS: | ||
| logger.warning( | ||
| DESIGN_DIAGRAM_GENERATION_FAILED, | ||
| error="invalid_output_format", | ||
| output_format=output_format, | ||
| ) | ||
| return ToolExecutionResult( | ||
| content=( | ||
| f"Invalid output_format: {output_format!r}. " | ||
| f"Must be one of: {sorted(_OUTPUT_FORMATS)}" | ||
| ), | ||
| is_error=True, | ||
| ) |
There was a problem hiding this comment.
Validate request shape before indexing arguments.
Missing diagram_type / description still raise KeyError, and non-string diagram_type or output_format can raise TypeError in the set-membership checks before you return a ToolExecutionResult. Guard the boundary up front instead of indexing raw arguments.
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/tools/design/diagram_generator.py` around lines 127 - 158, The
handler currently indexes arguments["diagram_type"] and ["description"] and does
set-membership checks against _DIAGRAM_TYPES/_OUTPUT_FORMATS without validating
input shape or types; this can raise KeyError/TypeError. Fix by first validating
that required keys ("diagram_type" and "description") exist and that
diagram_type, output_format (and title if present) are strings (use isinstance
checks) before any membership checks; if missing or wrong type, log
DESIGN_DIAGRAM_GENERATION_FAILED via logger with an appropriate error code and
return a ToolExecutionResult describing the problem instead of proceeding to the
membership tests in diagram_generator.py (check symbols: diagram_type,
description, title, output_format, _DIAGRAM_TYPES, _OUTPUT_FORMATS,
ToolExecutionResult, DESIGN_DIAGRAM_GENERATION_FAILED).
… type tests, report_generator assertion
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/synthorg/tools/communication/email_sender.py (2)
160-161:⚠️ Potential issue | 🟡 MinorMissing type validation for
ccandbccparameters.The
cc_addrsandbcc_addrsassignments useor []which doesn't guard against non-list types. A caller providing{"cc": "user@example.com"}(string instead of list) would cause aTypeErroron line 175 when concatenatingto_addrs + cc_addrs.As per coding guidelines: "Validate at system boundaries (user input, external APIs, config files)."
🛡️ Proposed fix to validate cc/bcc types
- cc_addrs: list[str] = arguments.get("cc") or [] - bcc_addrs: list[str] = arguments.get("bcc") or [] + cc_raw = arguments.get("cc") + bcc_raw = arguments.get("bcc") + if cc_raw is not None and not isinstance(cc_raw, list): + logger.warning( + COMM_TOOL_EMAIL_VALIDATION_FAILED, + reason="invalid_cc", + ) + return ToolExecutionResult( + content="'cc' must be a list of email addresses.", + is_error=True, + ) + if bcc_raw is not None and not isinstance(bcc_raw, list): + logger.warning( + COMM_TOOL_EMAIL_VALIDATION_FAILED, + reason="invalid_bcc", + ) + return ToolExecutionResult( + content="'bcc' must be a list of email addresses.", + is_error=True, + ) + cc_addrs: list[str] = cc_raw or [] + bcc_addrs: list[str] = bcc_raw or []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/communication/email_sender.py` around lines 160 - 161, The cc_addrs and bcc_addrs variables are taken from arguments without type checking and can be a string (or other types), which will break when later concatenated with to_addrs; modify the assignment site where cc_addrs and bcc_addrs are extracted (the code using arguments.get("cc") / arguments.get("bcc")) to validate types: if the value is None set to [], if it's a list accept it, if it's a single string either wrap it as [value] or reject it consistently (preferably raise a TypeError with a clear message), and otherwise raise TypeError; ensure the error message references "cc" / "bcc" and the function that sends the email (the code that performs to_addrs + cc_addrs concatenation) so callers know to pass lists.
172-173:⚠️ Potential issue | 🟡 MinorMissing type validation for
bodyandbody_is_htmlparameters.Neither
bodynorbody_is_htmlare type-checked. Ifbodyis a non-string (e.g., an integer),msg.set_content()may fail unexpectedly. Ifbody_is_htmlis a truthy string like"false", it would be incorrectly treated asTrue.🛡️ Proposed fix to add type validation
- body: str = arguments.get("body", "") - body_is_html: bool = arguments.get("body_is_html", False) + body_raw = arguments.get("body", "") + if not isinstance(body_raw, str): + logger.warning( + COMM_TOOL_EMAIL_VALIDATION_FAILED, + reason="invalid_body", + ) + return ToolExecutionResult( + content="'body' must be a string.", + is_error=True, + ) + body: str = body_raw + body_is_html_raw = arguments.get("body_is_html", False) + if not isinstance(body_is_html_raw, bool): + logger.warning( + COMM_TOOL_EMAIL_VALIDATION_FAILED, + reason="invalid_body_is_html", + ) + return ToolExecutionResult( + content="'body_is_html' must be a boolean.", + is_error=True, + ) + body_is_html: bool = body_is_html_raw🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/tools/communication/email_sender.py` around lines 172 - 173, Validate types for the extracted arguments before using them: ensure `body` (from arguments.get("body")) is a str and `body_is_html` (from arguments.get("body_is_html")) is a bool; in the function where `body` and `body_is_html` are defined (the block that calls msg.set_content()), add explicit checks that raise a TypeError if `body` is not a string and convert or validate `body_is_html` so truthy strings like "false" are not treated as True (accept a bool or parse canonical string values like "true"/"false" only), then proceed to call msg.set_content() with the validated `body` and `body_is_html`.
🤖 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/tools/test_factory.py`:
- Around line 425-468: The tests test_from_config_wires_design_tools and
test_from_config_wires_communication_tools currently only assert presence of
opt-in tools; add negative assertions to ensure backend-dependent tools remain
excluded when backends are not provided: after computing names = {t.name for t
in tools} in test_from_config_wires_design_tools assert "image_generator" not in
names (or any other design tools that require external backends), and in
test_from_config_wires_communication_tools assert "notification_sender" not in
names (or other communication backends) to enforce opt-in dependency behavior
when using build_default_tools_from_config.
- Around line 425-488: Tests for build_default_tools_from_config only cover
config fields and miss verifying that injected backend instances are forwarded;
add a new unit test in tests/unit/tools/test_factory.py that calls
build_default_tools_from_config with explicit mock/dummy instances for
image_provider, communication_dispatcher, analytics_provider, and metric_sink
and asserts the returned tools reference or wrap those exact instances (use
identity or an exposed attribute) to lock in forwarding behavior; locate
build_default_tools_from_config and the existing test_from_config_* helpers to
mirror their workspace/config setup and assert that specific tools (e.g.,
image/communication/analytics-related tool objects) have the injected backend
objects attached.
---
Duplicate comments:
In `@src/synthorg/tools/communication/email_sender.py`:
- Around line 160-161: The cc_addrs and bcc_addrs variables are taken from
arguments without type checking and can be a string (or other types), which will
break when later concatenated with to_addrs; modify the assignment site where
cc_addrs and bcc_addrs are extracted (the code using arguments.get("cc") /
arguments.get("bcc")) to validate types: if the value is None set to [], if it's
a list accept it, if it's a single string either wrap it as [value] or reject it
consistently (preferably raise a TypeError with a clear message), and otherwise
raise TypeError; ensure the error message references "cc" / "bcc" and the
function that sends the email (the code that performs to_addrs + cc_addrs
concatenation) so callers know to pass lists.
- Around line 172-173: Validate types for the extracted arguments before using
them: ensure `body` (from arguments.get("body")) is a str and `body_is_html`
(from arguments.get("body_is_html")) is a bool; in the function where `body` and
`body_is_html` are defined (the block that calls msg.set_content()), add
explicit checks that raise a TypeError if `body` is not a string and convert or
validate `body_is_html` so truthy strings like "false" are not treated as True
(accept a bool or parse canonical string values like "true"/"false" only), then
proceed to call msg.set_content() with the validated `body` and `body_is_html`.
🪄 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: e9bd3033-4fca-47f1-ac0a-be26b3e674e5
📒 Files selected for processing (4)
src/synthorg/tools/communication/email_sender.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/communication/test_template_formatter.pytests/unit/tools/test_factory.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). (4)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use
from __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotationsUse PEP 758 except syntax: use
except A, B:(no parentheses) -- ruff enforces this on Python 3.14Type hints: all public functions must be type-hinted, with mypy strict mode enforced
Docstrings: Google style required on public classes and functions (enforced by ruff D rules)
Never mutate existing objects -- create new objects instead. 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).Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using
model_copy(update=...)) for runtime state that evolves (e.g. agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.Models: Pydantic v2 (
BaseModel,model_validator,computed_field,ConfigDict). Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time. Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens). UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.Async concurrency: prefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.Line length: 88 characters (enforced by ruff)
Functions: < 50 l...
Files:
tests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_metric_collector.pysrc/synthorg/tools/communication/email_sender.pytests/unit/tools/test_factory.py
tests/**/*.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/test_factory.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/tools/communication/email_sender.py
🧠 Learnings (34)
📓 Common learnings
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts. At every phase of planning and implementation, be critical and actively look for ways to improve the design. Surface improvements as suggestions, not silent changes -- user decides. Prioritize issues by dependency order, not priority labels -- unblocked dependencies come first.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: Git commits: `<type>: <description>` -- types: feat, fix, refactor, docs, test, chore, perf, ci. Enforced by commitizen (commit-msg hook). Signed commits required on `main` via branch protection -- all commits must be GPG/SSH signed.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: Branches: `<type>/<slug>` from main. After finishing an issue implementation: always create a feature branch, commit, and push -- do NOT create a PR automatically. Do NOT leave work uncommitted on main -- branch, commit, push immediately after finishing.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: Pre-commit hooks: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint, golangci-lint + go vet (CLI), no-em-dashes, no-redundant-timeout, eslint-web (web dashboard). Hookify rules: block-pr-create (blocks direct `gh pr create`), enforce-parallel-tests (enforces `-n 8`), no-cd-prefix (blocks `cd` prefix in Bash), no-local-coverage (blocks `--cov` flags locally). Pre-push hooks: mypy, pytest unit tests, golangci-lint, go vet, go test (CLI), eslint-web (fast gate before push).
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: Pre-commit.ci: autoupdate disabled (`autoupdate_schedule: never`) -- Dependabot owns hook version bumps via `pre-commit` ecosystem.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: GitHub issue queries: use `gh issue list` via Bash (not MCP tools) -- MCP `list_issues` has unreliable field data. Merge strategy: squash merge -- PR body becomes the squash commit message on main. Trailers (e.g. `Release-As`, `Closes `#N``) must be in the PR body to land in the final commit. PR issue references: preserve existing `Closes `#NNN`` references -- never remove unless explicitly asked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-09T08:07:27.293Z
Learning: Pre-PR Review (MANDATORY): NEVER create a PR directly -- `gh pr create` is blocked by hookify. ALWAYS use `/pre-pr-review` to create PRs -- it runs automated checks + review agents + fixes before creating the PR. For trivial/docs-only changes: `/pre-pr-review quick` skips agents but still runs automated checks. After the PR exists, use `/aurelio-review-pr` to handle external reviewer feedback. The `/commit-push-pr` command is effectively blocked. Fix everything valid -- never skip: When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all. No deferring, no "out of scope" skipping.
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_metric_collector.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_metric_collector.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to tests/**/*.py : Use pytest.mark.parametrize for testing similar cases instead of multiple separate test functions
Applied to files:
tests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_metric_collector.py
📚 Learning: 2026-03-31T20:29:10.177Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:29:10.177Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/tools/communication/test_template_formatter.pytests/unit/tools/analytics/test_metric_collector.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/tools/analytics/test_metric_collector.pytests/unit/tools/test_factory.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/tools/analytics/test_metric_collector.pytests/unit/tools/test_factory.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to tests/**/*.py : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests - use generic names: test-provider, test-small-001, etc.
Applied to files:
tests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/test_factory.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/tools/analytics/test_metric_collector.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 : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
tests/unit/tools/analytics/test_metric_collector.pytests/unit/tools/test_factory.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/tools/communication/email_sender.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 : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/communication/email_sender.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 **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-04-09T07:26:54.376Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.376Z
Learning: Applies to **/*.py : Use `except A, B:` syntax without parentheses for exception handling on Python 3.14 (PEP 758), enforced by ruff
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/tools/communication/email_sender.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 **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/tools/communication/email_sender.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/tools/communication/email_sender.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/tools/communication/email_sender.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/tools/communication/email_sender.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/tools/communication/email_sender.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/tools/communication/email_sender.py
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
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/tools/communication/email_sender.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/tools/communication/email_sender.py
📚 Learning: 2026-04-09T07:26:54.377Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T07:26:54.377Z
Learning: Applies to **/*.py : Validate input at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/tools/communication/email_sender.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/tools/communication/email_sender.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-04-01T14:22:06.315Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T14:22:06.315Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/tools/communication/email_sender.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of real vendor names
Applied to files:
tests/unit/tools/test_factory.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to tests/**/*.py : Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mocking `time.monotonic()` and `asyncio.sleep()` for determinism or using `asyncio.Event().wait()` for indefinite blocking
Applied to files:
tests/unit/tools/test_factory.py
📚 Learning: 2026-03-31T14:28:28.895Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:28:28.895Z
Learning: Applies to src/synthorg/**/*.py : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
tests/unit/tools/test_factory.py
🔇 Additional comments (7)
src/synthorg/tools/communication/email_sender.py (4)
1-77: LGTM!Imports, logger setup, security regex patterns, and parameter schema are well-structured. The schema correctly defines
toandsubjectas required, with appropriate types for all fields.
80-119: LGTM!The class structure, docstring with example, and constructor follow the established patterns. Good use of
copy.deepcopyfor the parameters schema to maintain immutability.
232-244: LGTM on exception handling.Correct use of PEP 758 syntax (
except MemoryError, RecursionError:), proper re-raise of system errors, and generic error message returned to caller without leaking SMTP details.
261-315: LGTM!The
_send_syncmethod is well-implemented with proper control character sanitization for the subject, explicit string joins for recipient headers, and correct TLS/SSL handling. The partial credentials case is now handled at theEmailConfigvalidation level via the_validate_auth_fieldsmodel validator.tests/unit/tools/communication/test_template_formatter.py (1)
104-129: Good boundary coverage for invalid argument types.This parametrized async test cleanly locks the three type-validation paths with precise failure messages.
tests/unit/tools/analytics/test_metric_collector.py (1)
66-81: Nice defensive assertion before indexing sink calls.Verifying exactly one recorded call before
mock_sink.recorded[0]makes failures clearer and avoids brittle indexing assumptions.tests/unit/tools/test_factory.py (1)
356-398: Great addition of provider-only and sink-only analytics cases.These tests lock the intended partial-registration behavior and reduce regression risk in
_build_analytics_toolswiring.
Comment on lines
+425
to
+488
| def test_from_config_wires_design_tools( | ||
| self, | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| from synthorg.tools.design.config import DesignToolsConfig | ||
|
|
||
| config = RootConfig( | ||
| company_name="test-corp", | ||
| design_tools=DesignToolsConfig(), | ||
| ) | ||
| tools = build_default_tools_from_config( | ||
| workspace=tmp_path, | ||
| config=config, | ||
| ) | ||
| names = {t.name for t in tools} | ||
| assert "diagram_generator" in names | ||
| assert "asset_manager" in names | ||
|
|
||
| def test_from_config_wires_communication_tools( | ||
| self, | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| from synthorg.tools.communication.config import ( | ||
| CommunicationToolsConfig, | ||
| EmailConfig, | ||
| ) | ||
|
|
||
| email = EmailConfig( | ||
| host="smtp.example.com", | ||
| from_address="noreply@example.com", | ||
| use_tls=False, | ||
| ) | ||
| config = RootConfig( | ||
| company_name="test-corp", | ||
| communication_tools=CommunicationToolsConfig(email=email), | ||
| ) | ||
| tools = build_default_tools_from_config( | ||
| workspace=tmp_path, | ||
| config=config, | ||
| ) | ||
| names = {t.name for t in tools} | ||
| assert "email_sender" in names | ||
| assert "template_formatter" in names | ||
|
|
||
| def test_from_config_wires_analytics_tools( | ||
| self, | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| from synthorg.tools.analytics.config import AnalyticsToolsConfig | ||
|
|
||
| config = RootConfig( | ||
| company_name="test-corp", | ||
| analytics_tools=AnalyticsToolsConfig(), | ||
| ) | ||
| tools = build_default_tools_from_config( | ||
| workspace=tmp_path, | ||
| config=config, | ||
| ) | ||
| names = {t.name for t in tools} | ||
| # Without backends, no analytics tools are created | ||
| assert "data_aggregator" not in names | ||
| assert "report_generator" not in names | ||
| assert "metric_collector" not in names | ||
|
|
There was a problem hiding this comment.
Add build_default_tools_from_config() coverage for injected backend forwarding.
Current from-config tests validate config-field extraction, but they never pass image_provider, communication_dispatcher, analytics_provider, or metric_sink. If forwarding breaks in build_default_tools_from_config, this suite can still pass.
🧪 Suggested test to lock backend forwarding
+ def test_from_config_forwards_auxiliary_backends(
+ self,
+ tmp_path: Path,
+ ) -> None:
+ from unittest.mock import AsyncMock
+
+ from synthorg.tools.analytics.config import AnalyticsToolsConfig
+ from synthorg.tools.analytics.data_aggregator import AnalyticsProvider
+ from synthorg.tools.analytics.metric_collector import MetricSink
+ from synthorg.tools.communication.config import (
+ CommunicationToolsConfig,
+ EmailConfig,
+ )
+ from synthorg.tools.communication.notification_sender import (
+ NotificationDispatcherProtocol,
+ )
+ from synthorg.tools.design.config import DesignToolsConfig
+ from synthorg.tools.design.image_generator import ImageProvider
+
+ config = RootConfig(
+ company_name="test-corp",
+ design_tools=DesignToolsConfig(),
+ communication_tools=CommunicationToolsConfig(
+ email=EmailConfig(
+ host="smtp.example.com",
+ from_address="noreply@example.com",
+ use_tls=False,
+ )
+ ),
+ analytics_tools=AnalyticsToolsConfig(),
+ )
+
+ tools = build_default_tools_from_config(
+ workspace=tmp_path,
+ config=config,
+ image_provider=AsyncMock(spec=ImageProvider),
+ communication_dispatcher=AsyncMock(
+ spec=NotificationDispatcherProtocol
+ ),
+ analytics_provider=AsyncMock(spec=AnalyticsProvider),
+ metric_sink=AsyncMock(spec=MetricSink),
+ )
+ names = {t.name for t in tools}
+ assert "image_generator" in names
+ assert "notification_sender" in names
+ assert "data_aggregator" in names
+ assert "report_generator" in names
+ assert "metric_collector" in names📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_from_config_wires_design_tools( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from synthorg.tools.design.config import DesignToolsConfig | |
| config = RootConfig( | |
| company_name="test-corp", | |
| design_tools=DesignToolsConfig(), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| ) | |
| names = {t.name for t in tools} | |
| assert "diagram_generator" in names | |
| assert "asset_manager" in names | |
| def test_from_config_wires_communication_tools( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from synthorg.tools.communication.config import ( | |
| CommunicationToolsConfig, | |
| EmailConfig, | |
| ) | |
| email = EmailConfig( | |
| host="smtp.example.com", | |
| from_address="noreply@example.com", | |
| use_tls=False, | |
| ) | |
| config = RootConfig( | |
| company_name="test-corp", | |
| communication_tools=CommunicationToolsConfig(email=email), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| ) | |
| names = {t.name for t in tools} | |
| assert "email_sender" in names | |
| assert "template_formatter" in names | |
| def test_from_config_wires_analytics_tools( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from synthorg.tools.analytics.config import AnalyticsToolsConfig | |
| config = RootConfig( | |
| company_name="test-corp", | |
| analytics_tools=AnalyticsToolsConfig(), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| ) | |
| names = {t.name for t in tools} | |
| # Without backends, no analytics tools are created | |
| assert "data_aggregator" not in names | |
| assert "report_generator" not in names | |
| assert "metric_collector" not in names | |
| def test_from_config_wires_design_tools( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from synthorg.tools.design.config import DesignToolsConfig | |
| config = RootConfig( | |
| company_name="test-corp", | |
| design_tools=DesignToolsConfig(), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| ) | |
| names = {t.name for t in tools} | |
| assert "diagram_generator" in names | |
| assert "asset_manager" in names | |
| def test_from_config_wires_communication_tools( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from synthorg.tools.communication.config import ( | |
| CommunicationToolsConfig, | |
| EmailConfig, | |
| ) | |
| email = EmailConfig( | |
| host="smtp.example.com", | |
| from_address="noreply@example.com", | |
| use_tls=False, | |
| ) | |
| config = RootConfig( | |
| company_name="test-corp", | |
| communication_tools=CommunicationToolsConfig(email=email), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| ) | |
| names = {t.name for t in tools} | |
| assert "email_sender" in names | |
| assert "template_formatter" in names | |
| def test_from_config_wires_analytics_tools( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from synthorg.tools.analytics.config import AnalyticsToolsConfig | |
| config = RootConfig( | |
| company_name="test-corp", | |
| analytics_tools=AnalyticsToolsConfig(), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| ) | |
| names = {t.name for t in tools} | |
| # Without backends, no analytics tools are created | |
| assert "data_aggregator" not in names | |
| assert "report_generator" not in names | |
| assert "metric_collector" not in names | |
| def test_from_config_forwards_auxiliary_backends( | |
| self, | |
| tmp_path: Path, | |
| ) -> None: | |
| from unittest.mock import AsyncMock | |
| from synthorg.tools.analytics.config import AnalyticsToolsConfig | |
| from synthorg.tools.analytics.data_aggregator import AnalyticsProvider | |
| from synthorg.tools.analytics.metric_collector import MetricSink | |
| from synthorg.tools.communication.config import ( | |
| CommunicationToolsConfig, | |
| EmailConfig, | |
| ) | |
| from synthorg.tools.communication.notification_sender import ( | |
| NotificationDispatcherProtocol, | |
| ) | |
| from synthorg.tools.design.config import DesignToolsConfig | |
| from synthorg.tools.design.image_generator import ImageProvider | |
| config = RootConfig( | |
| company_name="test-corp", | |
| design_tools=DesignToolsConfig(), | |
| communication_tools=CommunicationToolsConfig( | |
| email=EmailConfig( | |
| host="smtp.example.com", | |
| from_address="noreply@example.com", | |
| use_tls=False, | |
| ) | |
| ), | |
| analytics_tools=AnalyticsToolsConfig(), | |
| ) | |
| tools = build_default_tools_from_config( | |
| workspace=tmp_path, | |
| config=config, | |
| image_provider=AsyncMock(spec=ImageProvider), | |
| communication_dispatcher=AsyncMock( | |
| spec=NotificationDispatcherProtocol | |
| ), | |
| analytics_provider=AsyncMock(spec=AnalyticsProvider), | |
| metric_sink=AsyncMock(spec=MetricSink), | |
| ) | |
| names = {t.name for t in tools} | |
| assert "image_generator" in names | |
| assert "notification_sender" in names | |
| assert "data_aggregator" in names | |
| assert "report_generator" in names | |
| assert "metric_collector" in names |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/tools/test_factory.py` around lines 425 - 488, Tests for
build_default_tools_from_config only cover config fields and miss verifying that
injected backend instances are forwarded; add a new unit test in
tests/unit/tools/test_factory.py that calls build_default_tools_from_config with
explicit mock/dummy instances for image_provider, communication_dispatcher,
analytics_provider, and metric_sink and asserts the returned tools reference or
wrap those exact instances (use identity or an exposed attribute) to lock in
forwarding behavior; locate build_default_tools_from_config and the existing
test_from_config_* helpers to mirror their workspace/config setup and assert
that specific tools (e.g., image/communication/analytics-related tool objects)
have the injected backend objects attached.
Aureliolo
added a commit
that referenced
this pull request
Apr 9, 2026
🤖 I have created a release *beep* *boop* --- ## [0.6.5](v0.6.4...v0.6.5) (2026-04-09) ### Features * add control-plane API endpoints batch ([#1118](#1118), [#1119](#1119), [#1120](#1120), [#1121](#1121)) ([#1138](#1138)) ([af11f0a](af11f0a)) * engine intelligence v2 -- trace enrichment, compaction, versioning eval ([#1139](#1139)) ([ed57dfa](ed57dfa)), closes [#1123](#1123) [#1125](#1125) [#1113](#1113) * generalize versioning to VersionSnapshot[T] for all entity types ([#1155](#1155)) ([5f563ce](5f563ce)), closes [#1131](#1131) [#1132](#1132) [#1133](#1133) * implement auxiliary tool categories -- design, communication, analytics ([#1152](#1152)) ([b506ba4](b506ba4)) * implement multi-project support -- engine orchestration ([#242](#242)) ([#1153](#1153)) ([74f1362](74f1362)) * implement SharedKnowledgeStore append-only + MVCC consistency model (Phase 1.5) ([#1134](#1134)) ([965d3a1](965d3a1)), closes [#1130](#1130) * implement shutdown strategies and SUSPENDED task status ([#1151](#1151)) ([6a0db11](6a0db11)) * persistent cost aggregation for project-lifetime budgets ([#1173](#1173)) ([5c212c5](5c212c5)), closes [#1156](#1156) * Prometheus /metrics endpoint and OTLP exporter ([#1122](#1122)) ([#1135](#1135)) ([aaeaae9](aaeaae9)), closes [#1124](#1124) * Prometheus metrics -- daily budget %, per-agent cost, per-agent budget % ([#1154](#1154)) ([581c494](581c494)), closes [#1148](#1148) ### Bug Fixes * communication hardening -- meeting cooldown, circuit breaker backoff, debate fallback ([#1140](#1140)) ([fe82894](fe82894)), closes [#1115](#1115) [#1116](#1116) [#1117](#1117) ### CI/CD * bump wrangler from 4.80.0 to 4.81.0 in /.github in the all group ([#1144](#1144)) ([b7c0945](b7c0945)) ### Maintenance * bump python from `6869258` to `5e59aae` in /docker/backend in the all group ([#1141](#1141)) ([01e99c2](01e99c2)) * bump python from `6869258` to `5e59aae` in /docker/sandbox in the all group ([#1143](#1143)) ([ea755bd](ea755bd)) * bump python from `6869258` to `5e59aae` in /docker/web in the all group ([#1142](#1142)) ([5416dd9](5416dd9)) * bump the all group across 1 directory with 2 updates ([#1181](#1181)) ([d3d5adf](d3d5adf)) * bump the all group across 1 directory with 3 updates ([#1146](#1146)) ([c609e6c](c609e6c)) * bump the all group in /cli with 2 updates ([#1177](#1177)) ([afd9cde](afd9cde)) * bump the all group in /site with 3 updates ([#1178](#1178)) ([7cff82a](7cff82a)) * bump the all group with 2 updates ([#1180](#1180)) ([199a1a8](199a1a8)) * bump vitest from 4.1.2 to 4.1.3 in /site in the all group ([#1145](#1145)) ([a8c1194](a8c1194)) * consolidated web deps (11 packages + hono security + test fixes) ([#1150](#1150)) ([63a9390](63a9390)), closes [#1147](#1147) [#1136](#1136) [#1137](#1137) * pin Docker Python base image to 3.14.x ([#1182](#1182)) ([8ffdd86](8ffdd86)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements three auxiliary tool categories for specialized agent workflows, resolving the consolidated requirements from #215, #216, and #217.
Closes #1035
Tool Categories
Design Tools (
src/synthorg/tools/design/)ImageProviderprotocol (no provider shipped -- users inject their own)Communication Tools (
src/synthorg/tools/communication/)smtplib+asyncio.to_thread(same pattern asEmailNotificationSink)NotificationDispatcherviaNotificationDispatcherProtocolSandboxedEnvironmentfor safe template rendering (autoescape enabled for HTML output)Analytics Tools (
src/synthorg/tools/analytics/)AnalyticsProviderprotocol with metric whitelisting and ISO 8601 date validationMetricSinkprotocolIntegration
_build_design_tools,_build_communication_tools,_build_analytics_tools) wired intobuild_default_toolsandbuild_default_tools_from_configNoneconfig (default) = no tools createdRootConfigextended withdesign_tools,communication_tools,analytics_toolsfieldsconfig/defaults.pyupdated with new field defaultsObservability
events/design.py(14 event constants)events/communication.py(11 new tool-specific events)events/analytics.py(11 new tool-specific events)Security
SandboxedEnvironmentwith autoescape for HTML outputEmailConfigcross-field validation (username + password must be both or neither)ImageResultfield validation (width/heightgt=0,datamin_length=1)Test Plan
Review Coverage
Pre-reviewed by 13 agents, 15 valid findings addressed: