feat: implement semantic conflict detection in workspace merge#860
feat: implement semantic conflict detection in workspace merge#860
Conversation
WalkthroughAdds post-merge semantic conflict detection. Introduces a Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a semantic conflict detection framework to identify logical inconsistencies after successful textual merges. It implements AST-based checks for Python files—detecting removed references, signature mismatches, duplicate definitions, and import conflicts—and adds an LLM-based analyzer for complex logical reviews. The merge orchestration logic has been updated to integrate these checks and respect defined escalation strategies. Feedback includes a critical correction for a Python 2-style exception handling syntax and a performance recommendation to parallelize sequential git operations using asyncio.
| for file_path in files: | ||
| try: | ||
| contents[file_path] = (root / file_path).read_text(encoding="utf-8") | ||
| except FileNotFoundError, PermissionError, OSError: |
There was a problem hiding this comment.
This except block uses Python 2 syntax (a comma to separate exception types). For compatibility with Python 3, the exceptions should be grouped in a tuple.
| except FileNotFoundError, PermissionError, OSError: | |
| except (FileNotFoundError, PermissionError, OSError): |
| sources: dict[str, str] = {} | ||
| for file_path in files: | ||
| rc, stdout, _ = await self._run_git( | ||
| "show", | ||
| f"{base_sha}:{file_path}", | ||
| log_event=WORKSPACE_SEMANTIC_ANALYSIS_FAILED, | ||
| ) | ||
| if rc == 0: | ||
| sources[file_path] = stdout | ||
| return sources |
There was a problem hiding this comment.
This loop executes git show sequentially for each file, which can be slow if there are many changed files. This can be parallelized to improve performance.
While I see this is noted as a future improvement in issue #857, addressing it now in this new function would be beneficial. You can use asyncio.gather to run the _run_git calls concurrently.
tasks = [
self._run_git(
"show",
f"{base_sha}:{file_path}",
log_event=WORKSPACE_SEMANTIC_ANALYSIS_FAILED,
)
for file_path in files
]
results = await asyncio.gather(*tasks)
sources: dict[str, str] = {}
for file_path, result in zip(files, results):
rc, stdout, _ = result
if rc == 0:
sources[file_path] = stdout
return sources
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #860 +/- ##
==========================================
- Coverage 92.31% 92.16% -0.15%
==========================================
Files 585 589 +4
Lines 30745 31082 +337
Branches 2958 3026 +68
==========================================
+ Hits 28381 28647 +266
- Misses 1870 1921 +51
- Partials 494 514 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workspace/git_worktree.py`:
- Around line 714-730: The code calls _get_base_sources for every path in
changed_files while holding PlannerWorktreeStrategy._lock, causing many git show
calls before the analyzer's max_files/file_extensions filter
(AstSemanticAnalyzer._filter_files) runs; change the flow to pre-filter
changed_files by applying the same file_extensions and max_files rules (or
move/replicate _filter_files logic) immediately after _get_changed_files and
before calling _get_base_sources so only up to max_files and matching extensions
are passed into _get_base_sources, or alternatively make _get_base_sources
return lazily-loaded base contents so the analyzer can enforce
max_files/file_extensions without serializing expensive git calls.
In `@src/synthorg/engine/workspace/semantic_analyzer.py`:
- Around line 83-92: Replace the parenthesized exception tuple with the PEP 758
multi-except pipe syntax in the try/except that reads files into the sources
dict: change the except clause handling FileNotFoundError, PermissionError,
OSError (the block that logs WORKSPACE_SEMANTIC_ANALYSIS_FAILED for
workspace_id/file) to use "except FileNotFoundError | PermissionError | OSError
as exc:" so the read_text call and logger.debug remain unchanged.
In `@src/synthorg/engine/workspace/semantic_checks.py`:
- Around line 268-283: _compare_signatures currently only records positional
min/max and removed names, so adding required keyword-only params (e.g., def
f(x) -> def f(x, *, y)) is missed; update the signature diff creation (around
where _function_min_args/_function_max_args/_function_param_names are used for
base_node and merged_node) to also compute and store required keyword-only
parameter names from merged_node (kwonly args with no default) and record
per-parameter kind changes (compare each param's kind between base_node and
merged_node) into the _SigInfo structure; then ensure _SigInfo includes these
new fields so _check_call_compat() can validate new required kw-only params and
parameter-kind changes when checking calls.
- Around line 56-62: _all_name_references currently collects every ast.Name,
causing false positives/negatives; change it to collect only load-context names
(filter where isinstance(node.ctx, ast.Load)) and then perform simple
scope/binding resolution inside the same module before returning (exclude names
that are bound in the module via ast.Assign, ast.AnnAssign, ast.AugAssign,
ast.FunctionDef args, ast.ClassDef names, ast.Import/ImportFrom aliases, and
comprehension targets). Update the related logic used by
check_removed_references (and the duplicate logic around the other occurrence at
the area noted ~196-203) to use this Load-only + binding-exclusion helper so
references from the same module aren’t treated as removed and only free/used
names drive merge escalation.
In `@src/synthorg/engine/workspace/semantic_llm_prompt.py`:
- Around line 173-185: The _parse_conflicts_from_args function currently raises
TypeError when the "conflicts" argument is not a list; change this to raise
ValueError to match the exception contract described for
parse_tool_call_response and keep callers consistent. Update the raise statement
in _parse_conflicts_from_args to raise ValueError(msg) (leave the logger.warning
call and msg construction unchanged) so the function signature and error flows
align with parse_tool_call_response.
In `@src/synthorg/engine/workspace/semantic_llm.py`:
- Around line 218-223: The except clause in semantic_llm.py redundantly lists
FileNotFoundError and PermissionError along with OSError; replace the multi-type
clause with a single except OSError: to catch all I/O-related errors cleanly
(keep the existing handler that calls logger.debug with
WORKSPACE_SEMANTIC_ANALYSIS_FAILED, file=file_path, reason="read_error"). Ensure
you update the except block that currently reads "except FileNotFoundError,
PermissionError, OSError:" to "except OSError:" so only the superclass is
caught.
In `@tests/integration/engine/test_workspace_semantic_integration.py`:
- Around line 253-292: Test is nondeterministic because both branches rewrite
the same hunk in "helpers.py" so merges often produce textual conflicts and skip
semantic checks; change the fixture commits so each branch touches disjoint
hunks that auto-merge and produce duplicate definitions. Concretely, in the test
code that calls _commit_file for ws_a and ws_b, make the two commits modify
different parts of the file (e.g., insert Agent A's process() in a top section
and Agent B's process() in a bottom section or add distinct surrounding
comments/marker lines) so git can perform a clean textual merge; keep the file
name "helpers.py", the workspace setup calls (WorkspaceRequest/use of ws_a and
ws_b), and the later merge_workspace result_b semantic_conflicts assertions
intact so check_duplicate_definitions() is exercised. Ensure the changes still
result in two process definitions after merging so the semantic_conflicts
assertion triggers.
In `@tests/unit/engine/test_semantic_checks.py`:
- Around line 1-6: The module docstring incorrectly states that tests are
"parametrized tests" while the test suite uses individual test functions; update
the module-level docstring in test_semantic_checks.py to remove the reference to
parametrization and instead state that individual test methods (e.g., the
various test_* functions in this module) cover positive detections and negative
(no-conflict) cases, and remove or reword any mention of
`@pytest.mark.parametrize` so the description matches the actual implementation.
In `@tests/unit/engine/test_workspace_models.py`:
- Around line 397-417: Add the missing pytest marker to the test method:
decorate test_total_semantic_conflicts with `@pytest.mark.unit` (ensure pytest is
imported if not) so it matches other tests in the file; locate the
test_total_semantic_conflicts function in the WorkspaceGroupResult tests and add
the `@pytest.mark.unit` decorator immediately above the def line.
🪄 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: 341634c3-9087-4506-a6e3-de11c055c463
📒 Files selected for processing (20)
src/synthorg/engine/errors.pysrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/merge.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/observability/events/workspace.pytests/integration/engine/test_workspace_semantic_integration.pytests/unit/engine/conftest.pytests/unit/engine/test_semantic_analyzer.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_semantic_llm_prompt.pytests/unit/engine/test_workspace_git_worktree.pytests/unit/engine/test_workspace_merge.pytests/unit/engine/test_workspace_models.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax:except A, B:(no parentheses) instead ofexcept (A, B):-- ruff enforces this on Python 3.14
All public functions and classes must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskfor structured concurrency
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters as enforced by ruff
Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Files:
src/synthorg/engine/errors.pytests/unit/engine/test_workspace_models.pysrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/merge.pytests/unit/engine/test_workspace_git_worktree.pytests/unit/engine/conftest.pytests/unit/engine/test_semantic_llm.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/workspace/config.pytests/unit/engine/test_semantic_analyzer.pysrc/synthorg/engine/workspace/git_worktree.pytests/unit/engine/test_semantic_checks.pysrc/synthorg/engine/workspace/models.pytests/unit/engine/test_semantic_llm_prompt.pytests/integration/engine/test_workspace_semantic_integration.pytests/unit/engine/test_workspace_merge.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and use logger:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the structured logger fromsynthorg.observability
Always use the logger variable namelogger(not_logger, notlog)
Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTEDfromevents.api); import directly and always use structured kwargs:logger.info(EVENT, key=value)never string formatting
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
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields instead of manual validators
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Files:
src/synthorg/engine/errors.pysrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/merge.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowto categorize tests
Maintain 80% minimum code coverage as enforced in CI
Useasyncio_mode = 'auto'for async tests; no manual@pytest.mark.asyncioneeded
Use@pytest.mark.parametrizefor testing similar cases instead of test duplication
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, tests, or config examples; use generic names:example-provider,example-large-001,test-provider,test-small-001,large/medium/small
Use Hypothesis with@given+@settingsfor property-based testing; avoid flaky timing-based tests by mockingtime.monotonic()andasyncio.sleep()or usingasyncio.Event().wait()for indefinite blocks
Files:
tests/unit/engine/test_workspace_models.pytests/unit/engine/test_workspace_git_worktree.pytests/unit/engine/conftest.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_semantic_analyzer.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_semantic_llm_prompt.pytests/integration/engine/test_workspace_semantic_integration.pytests/unit/engine/test_workspace_merge.py
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.848Z
Learning: Git merge strategy: squash merge -- PR body becomes the squash commit message on main; preserve existing `Closes `#NNN`` trailers in PR body; add `Release-As: X.Y.Z` trailer as final paragraph for version bumping
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and always use structured kwargs: `logger.info(EVENT, key=value)` never string formatting
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/workspace.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/workspace.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.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/engine/workspace/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/engine/workspace/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/engine/workspace/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/engine/workspace/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:
src/synthorg/engine/workspace/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/engine/workspace/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/engine/workspace/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/engine/test_semantic_analyzer.py
🔇 Additional comments (26)
src/synthorg/engine/errors.py (1)
90-92: LGTM!The new
SemanticAnalysisErrorexception follows the established workspace error hierarchy pattern and is consistent with sibling exceptions likeWorkspaceMergeErrorandWorkspaceCleanupError.src/synthorg/engine/workspace/models.py (3)
92-95: LGTM!The
descriptionfield withdefault=""maintains backward compatibility with existing textual conflict creation (which doesn't provide descriptions) while allowing semantic conflicts to include meaningful descriptions.
133-136: LGTM!The
semantic_conflictsfield and its validator correctly enforce the design constraint that semantic conflicts can only exist on successful merges (since semantic analysis runs post-merge). The implementation aligns with the control flow ingit_worktree.pywhere semantic analysis only executes whenrc == 0.Also applies to: 150-152
194-201: LGTM!The
total_semantic_conflictscomputed field follows the established pattern oftotal_conflictsand correctly aggregates semantic conflicts across all merge results.tests/unit/engine/conftest.py (1)
359-359: LGTM!The
semantic_conflictsparameter addition follows the established pattern of the helper function, maintaining backward compatibility with a default empty tuple while enabling tests to constructMergeResultinstances with semantic conflicts.Also applies to: 375-375
src/synthorg/engine/workspace/merge.py (1)
138-149: LGTM!The semantic conflict handling correctly mirrors the textual conflict pattern:
- Sets escalation via
model_copyto maintain immutability- Appends result before checking break condition (ensuring result is recorded)
- Breaks on
HUMANescalation, allowingREVIEW_AGENTto continue- Skips cleanup when breaking, preserving the workspace for human review
tests/unit/engine/test_workspace_models.py (1)
419-456: LGTM!The
TestMergeResultSemanticConflictsclass properly tests both validation scenarios:
- Rejects
semantic_conflictswhensuccess=False- Accepts
semantic_conflictswhensuccess=TrueThe class-level
pytestmark = pytest.mark.unitcorrectly applies the marker to all methods in the class.src/synthorg/engine/workspace/__init__.py (1)
9-9: LGTM!The semantic analysis components are correctly exported with proper alphabetical ordering in
__all__. The exclusion ofLlmSemanticAnalyzerfrom the public API suggests it's intended for internal use viaCompositeSemanticAnalyzer, which is a reasonable encapsulation choice.Also applies to: 26-30, 36-37, 43-44
tests/unit/engine/test_workspace_git_worktree.py (3)
260-267: LGTM!The mock sequence correctly reflects the updated git command flow: checkout → pre-merge rev-parse (for semantic analysis base) → merge → post-merge rev-parse. The comments clearly distinguish between the two rev-parse calls.
289-296: LGTM!The conflict test correctly includes the pre-merge rev-parse in the mock sequence. Even when merge fails, the pre-merge SHA capture still occurs before the merge attempt.
339-346: LGTM!Both the abort failure and rev-parse failure tests correctly include the pre-merge rev-parse mock. The
test_merge_revparse_failure_raisestest specifically validates that the post-merge rev-parse failure (not pre-merge) raises the expected error.Also applies to: 366-372
src/synthorg/engine/workspace/config.py (2)
9-51: LGTM!The
SemanticAnalysisConfigfollows all coding guidelines:
- Frozen Pydantic model with
extra="forbid"- Google-style docstring with attributes
- Proper field validation constraints (
ge,le,gt)- Uses
NotBlankStr | Nonefor optional identifier field- Safe defaults (
enabled=False)
89-92: LGTM!The
semantic_analysisfield correctly usesdefault_factory=SemanticAnalysisConfigfor the nested frozen model, following the established pattern for other nested configs likeplanner_worktreesinWorkspaceIsolationConfig.tests/unit/engine/test_semantic_llm.py (1)
1-194: LGTM!Well-structured test suite covering key scenarios for
LlmSemanticAnalyzer: constructor validation, conflict parsing, filtering, provider errors, and retry behavior. The tests use appropriate mocking patterns and comply with test guidelines (module-levelpytest.mark.unit, generic model names).src/synthorg/observability/events/workspace.py (1)
25-31: LGTM!New semantic analysis event constants follow the established naming convention and are properly typed with
Final[str].tests/unit/engine/test_semantic_analyzer.py (1)
1-260: LGTM!Comprehensive test coverage for both
AstSemanticAnalyzerandCompositeSemanticAnalyzer. Tests verify protocol compliance, edge cases (empty inputs, file read errors, extension filtering), and composite behavior (aggregation, deduplication, fault isolation).tests/unit/engine/test_semantic_checks.py (1)
7-388: LGTM!Thorough test coverage for all four semantic check functions. Tests cover positive detections (removed references, signature changes, duplicates, import conflicts), negative cases (clean merges), and edge cases (empty sources, syntax errors in input).
tests/unit/engine/test_workspace_merge.py (1)
391-447: LGTM!New tests correctly verify semantic conflict escalation behavior:
HUMANescalation stops on first semantic conflict (single result), whileREVIEW_AGENTcontinues merging (two results). The assertions properly check bothsemantic_conflictscontent andescalationfield values.tests/unit/engine/test_semantic_llm_prompt.py (1)
1-188: LGTM!Comprehensive test coverage for prompt building and response parsing. Tests verify tool schema structure, message roles and content, and parsing behavior including error cases (unparseable JSON, empty responses, invalid conflict items).
src/synthorg/engine/workspace/semantic_llm_prompt.py (3)
1-83: LGTM!Well-structured module with pure functions for prompt construction. The tool definition schema properly defines required fields and the system message provides clear instructions for semantic conflict detection.
86-142: LGTM!Message builders correctly construct system and user messages with appropriate roles and structured content including diff summary and file contents.
204-231: LGTM!Content fallback parsing handles both raw JSON and fenced code blocks with proper error logging before raising exceptions.
src/synthorg/engine/workspace/semantic_llm.py (4)
38-72: LGTM!Constructor properly validates the model parameter and lazily imports
SemanticAnalysisConfigfor default config creation.
74-138: LGTM!The
analyzemethod correctly filters files by extension, respectsmax_fileslimit, and builds appropriate messages for LLM review. Logging at method entry follows observability guidelines.
140-206: LGTM!Retry logic properly handles parse failures with configurable retries, logs at appropriate levels (DEBUG for intermediate failures, WARNING for exhaustion), and returns empty tuple as a safe fallback. Success path logs completion with conflict count.
227-239: LGTM!Helper function
_build_diff_summarycorrectly identifies new vs modified files by comparing against base sources.
tests/integration/engine/test_workspace_semantic_integration.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workspace/semantic_llm_prompt.py`:
- Around line 195-199: The MergeConflict constructor call is passing
description=description but the MergeConflict model (class MergeConflict) does
not define a description field; fix this by making the call and model
consistent: either add a description attribute to the MergeConflict model
definition (class MergeConflict) in models.py and migrate any
validation/serialization accordingly, or remove description=description from the
construction and instead assign the semantic text to the existing field on
MergeConflict (e.g., message/details/metadata—use the exact field name defined
in the class) so the semantic details are preserved in the payload.
- Around line 177-193: The loop over raw_conflicts (and the handling around
conflicts: list[MergeConflict]) currently silently skips malformed items;
instead enforce strict validation by detecting non-dict items or dicts missing
required fields (file_path/description), log a clear error via logger with
WORKSPACE_SEMANTIC_ANALYSIS_FAILED and a reason like "malformed_conflict_item",
and raise a ValueError so the caller/retry loop handles provider output errors;
update the parsing in the block that iterates raw_conflicts (and any use of
MergeConflict) to either fail fast on the first malformed entry or collect
malformed counts and raise if any exist, rather than continuing silently.
🪄 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: 75356b68-cb17-4882-a2cd-14bdb6e71bbc
📒 Files selected for processing (1)
src/synthorg/engine/workspace/semantic_llm_prompt.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax:except A, B:(no parentheses) instead ofexcept (A, B):-- ruff enforces this on Python 3.14
All public functions and classes must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskfor structured concurrency
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters as enforced by ruff
Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Files:
src/synthorg/engine/workspace/semantic_llm_prompt.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and use logger:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the structured logger fromsynthorg.observability
Always use the logger variable namelogger(not_logger, notlog)
Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTEDfromevents.api); import directly and always use structured kwargs:logger.info(EVENT, key=value)never string formatting
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
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields instead of manual validators
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Files:
src/synthorg/engine/workspace/semantic_llm_prompt.py
🧠 Learnings (2)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/engine/workspace/semantic_llm_prompt.py
| raw_conflicts = arguments.get("conflicts", []) | ||
| if not isinstance(raw_conflicts, list): | ||
| msg = f"Expected list of conflicts, got {type(raw_conflicts).__name__}" | ||
| logger.warning( | ||
| WORKSPACE_SEMANTIC_ANALYSIS_FAILED, | ||
| reason="invalid_conflicts_type", | ||
| error=msg, | ||
| ) | ||
| raise ValueError(msg) # noqa: TRY004 -- must be ValueError for retry loop | ||
|
|
||
| conflicts: list[MergeConflict] = [] | ||
| for item in raw_conflicts: | ||
| if not isinstance(item, dict): | ||
| continue | ||
| file_path = item.get("file_path", "") | ||
| description = item.get("description", "") | ||
| if file_path and description: |
There was a problem hiding this comment.
Do not silently skip malformed conflict items from provider output.
Lines 189-193 quietly continue on invalid entries and missing fields. This can return partial/empty results for malformed tool output instead of triggering retry/error handling, causing false negatives in semantic conflict detection.
♻️ Suggested strict parsing change
def _parse_conflicts_from_args(
arguments: dict[str, Any],
) -> tuple[MergeConflict, ...]:
"""Parse conflicts from tool call arguments dict."""
- raw_conflicts = arguments.get("conflicts", [])
+ if "conflicts" not in arguments:
+ msg = "Missing required key: conflicts"
+ logger.warning(
+ WORKSPACE_SEMANTIC_ANALYSIS_FAILED,
+ reason="missing_conflicts_key",
+ error=msg,
+ )
+ raise ValueError(msg)
+
+ raw_conflicts = arguments["conflicts"]
if not isinstance(raw_conflicts, list):
msg = f"Expected list of conflicts, got {type(raw_conflicts).__name__}"
logger.warning(
WORKSPACE_SEMANTIC_ANALYSIS_FAILED,
reason="invalid_conflicts_type",
error=msg,
)
raise ValueError(msg) # noqa: TRY004 -- must be ValueError for retry loop
conflicts: list[MergeConflict] = []
- for item in raw_conflicts:
+ for index, item in enumerate(raw_conflicts):
if not isinstance(item, dict):
- continue
- file_path = item.get("file_path", "")
- description = item.get("description", "")
- if file_path and description:
- conflicts.append(
- MergeConflict(
- file_path=file_path,
- conflict_type=ConflictType.SEMANTIC,
- description=description,
- ),
- )
+ msg = f"Conflict at index {index} is not an object"
+ logger.warning(
+ WORKSPACE_SEMANTIC_ANALYSIS_FAILED,
+ reason="invalid_conflict_item_type",
+ error=msg,
+ )
+ raise ValueError(msg)
+ file_path = item.get("file_path")
+ description = item.get("description")
+ if not isinstance(file_path, str) or not file_path.strip():
+ msg = f"Conflict at index {index} has invalid file_path"
+ logger.warning(
+ WORKSPACE_SEMANTIC_ANALYSIS_FAILED,
+ reason="invalid_conflict_file_path",
+ error=msg,
+ )
+ raise ValueError(msg)
+ if not isinstance(description, str) or not description.strip():
+ msg = f"Conflict at index {index} has invalid description"
+ logger.warning(
+ WORKSPACE_SEMANTIC_ANALYSIS_FAILED,
+ reason="invalid_conflict_description",
+ error=msg,
+ )
+ raise ValueError(msg)
+ conflicts.append(
+ MergeConflict(
+ file_path=file_path,
+ conflict_type=ConflictType.SEMANTIC,
+ description=description,
+ ),
+ )
return tuple(conflicts)As per coding guidelines, "Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workspace/semantic_llm_prompt.py` around lines 177 - 193,
The loop over raw_conflicts (and the handling around conflicts:
list[MergeConflict]) currently silently skips malformed items; instead enforce
strict validation by detecting non-dict items or dicts missing required fields
(file_path/description), log a clear error via logger with
WORKSPACE_SEMANTIC_ANALYSIS_FAILED and a reason like "malformed_conflict_item",
and raise a ValueError so the caller/retry loop handles provider output errors;
update the parsing in the block that iterates raw_conflicts (and any use of
MergeConflict) to either fail fast on the first malformed entry or collect
malformed counts and raise if any exist, rather than continuing silently.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workspace/config.py`:
- Around line 27-30: The file_extensions Field on SemanticAnalysisConfig
currently accepts empty or malformed values (e.g. "" or empty tuple) which
breaks downstream path.endswith checks; add a pydantic validator on
SemanticAnalysisConfig.file_extensions that (1) rejects an empty tuple, and (2)
rejects any extension that is blank or malformed (e.g. empty string or not
starting with '.' and/or contains whitespace/path separators), raising
ValueError with a clear message; update the class-level validator (using
`@validator`("file_extensions") or `@root_validator`) to perform these checks so
invalid configs are rejected at load time.
In `@src/synthorg/engine/workspace/semantic_analyzer.py`:
- Around line 99-107: The except block in the semantic analyzer should be
simplified to catch OSError only and use the PEP 758 syntax without parentheses:
replace the current "except (FileNotFoundError, PermissionError, OSError) as
exc:" with "except OSError as exc:" in the function that reads sources so the
logger.debug call that emits WORKSPACE_SEMANTIC_ANALYSIS_FAILED (including
workspace_id, file=file_path, reason="read_error", error=...) remains unchanged;
this removes redundant exception types and fixes the except syntax per
guidelines.
In `@src/synthorg/engine/workspace/semantic_llm.py`:
- Around line 274-281: Change the except clause to use the PEP 758 form and
catch only OSError (since FileNotFoundError and PermissionError are subclasses):
replace the current "except (FileNotFoundError, PermissionError, OSError) as
exc:" with "except OSError as exc:" in the block around logger.warning (using
WORKSPACE_SEMANTIC_ANALYSIS_FAILED and file_path) so the handler remains the
same but adheres to the required except syntax.
In `@tests/unit/engine/test_semantic_checks.py`:
- Around line 29-388: Collapse repetitive test cases into parametrized tests:
for the removed-reference tests, replace individual functions like
test_function_removed_and_referenced, test_class_removed_and_referenced,
test_constant_removed_and_referenced, test_removed_but_not_referenced,
test_name_still_present_no_conflict, test_empty_sources_no_conflict, and
test_syntax_error_in_source_skipped with a single `@pytest.mark.parametrize` table
(each row contains base_sources, merged_sources, expected_count or predicate,
and an optional highlight string) and one test that calls
check_removed_references and asserts accordingly; do the same for signature
tests (group test_param_removed_callers_break,
test_required_param_added_callers_break, test_default_param_added_no_break,
test_no_callers_no_conflict, test_keyword_arg_removed_detected,
test_varargs_accepts_extra_positional, test_signature_unchanged_no_conflict)
into one parametrized test that calls check_signature_changes; similarly
collapse duplicate-definition tests (test_duplicate_function_same_file,
test_duplicate_class_same_file, test_no_duplicates_no_conflict,
test_nested_same_name_not_flagged, test_empty_sources,
test_syntax_error_skipped) into a parametrize for check_duplicate_definitions,
and import-conflict tests (test_import_of_removed_name, test_import_still_valid,
test_import_star_not_flagged, test_class_removed_import_breaks,
test_empty_sources) into a parametrize for check_import_conflicts; keep
references to the target functions check_removed_references,
check_signature_changes, check_duplicate_definitions, and check_import_conflicts
so reviewers can find where to change assertions and test data.
In `@tests/unit/engine/test_semantic_llm.py`:
- Around line 83-89: The tests patch _read_file_contents with a
mapping-returning value, but semantic_llm._read_file_contents(file_path,
workspace_id) should return a string for a single file; update the patches in
tests (around test_semantic_llm, lines exercising analyzer.analyze and calls
using merged_content) to use a side_effect or per-file return that returns the
file's string content keyed by the file_path argument (i.e., return "def foo():
..." for "utils.py") instead of returning {"utils.py": "..."} so merged_sources
keeps the correct shape and prompt construction is exercised; apply the same
change to the other occurrences noted (lines ~105-111, ~141-147, ~174-180).
In `@tests/unit/engine/test_workspace_models.py`:
- Around line 421-457: Add a negative test that directly exercises the
MergeConflict.description validator: attempt to construct a MergeConflict (or a
MergeResult containing one) with conflict_type=ConflictType.SEMANTIC and an
empty or whitespace description and assert it raises ValidationError (matching
an appropriate message about non-empty description). Reference the MergeConflict
class and its description field and/or the MergeResult.semantic_conflicts tuple
to locate where to add the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: de784ee1-4b09-43e3-b77b-10f62856dafa
📒 Files selected for processing (16)
CLAUDE.mddocs/design/engine.mdsrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pytests/integration/engine/test_workspace_semantic_integration.pytests/unit/engine/test_semantic_analyzer.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_workspace_git_worktree.pytests/unit/engine/test_workspace_models.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax:except A, B:(no parentheses) instead ofexcept (A, B):-- ruff enforces this on Python 3.14
All public functions and classes must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskfor structured concurrency
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters as enforced by ruff
Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Files:
src/synthorg/engine/workspace/__init__.pytests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_analyzer.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_semantic_checks.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/models.pytests/unit/engine/test_workspace_git_worktree.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.pytests/integration/engine/test_workspace_semantic_integration.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/semantic_checks.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and use logger:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the structured logger fromsynthorg.observability
Always use the logger variable namelogger(not_logger, notlog)
Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTEDfromevents.api); import directly and always use structured kwargs:logger.info(EVENT, key=value)never string formatting
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
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields instead of manual validators
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Files:
src/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/semantic_checks.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowto categorize tests
Maintain 80% minimum code coverage as enforced in CI
Useasyncio_mode = 'auto'for async tests; no manual@pytest.mark.asyncioneeded
Use@pytest.mark.parametrizefor testing similar cases instead of test duplication
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, tests, or config examples; use generic names:example-provider,example-large-001,test-provider,test-small-001,large/medium/small
Use Hypothesis with@given+@settingsfor property-based testing; avoid flaky timing-based tests by mockingtime.monotonic()andasyncio.sleep()or usingasyncio.Event().wait()for indefinite blocks
Files:
tests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_analyzer.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_workspace_git_worktree.pytests/integration/engine/test_workspace_semantic_integration.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS read the relevant design specification page in
docs/design/before implementing any feature or planning any issue; design spec is the starting point for architecture, data models, and behavior; alert user and explain if implementation deviates from spec; update the spec page when deviations are approved
Files:
docs/design/engine.md
🧠 Learnings (40)
📚 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: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
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 : 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:
CLAUDE.mdsrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
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/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:
CLAUDE.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/hr/**/*.py : HR engine must provide: hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, trend detection), promotion/demotion
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.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: Applies to src/synthorg/hr/**/*.py : HR package (hr/): hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, LLM calibration, collaboration overrides, trend detection), promotion/demotion (criteria evaluation, approval strategies, model mapping)
Applied to files:
CLAUDE.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/engine/coordination/**/*.py : Task coordination uses multi-agent pipeline with 4 dispatchers (SAS/centralized/decentralized/context-dependent), wave execution, and workspace lifecycle integration.
Applied to files:
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: 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:
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 docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
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 tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).
Applied to files:
tests/unit/engine/test_workspace_models.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.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/engine/workspace/config.pysrc/synthorg/engine/workspace/models.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/engine/workspace/config.pysrc/synthorg/engine/workspace/models.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_llm_prompt.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/engine/workspace/config.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/models.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/engine/workspace/config.pysrc/synthorg/engine/workspace/models.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/engine/workspace/models.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 **/*.py : Models: Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use computed_field for derived values 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/engine/workspace/models.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/engine/workspace/models.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 with `computed_field` for derived values instead of storing redundant fields; use `NotBlankStr` (from `core.types`) for all identifier/name fields instead of manual validators
Applied to files:
src/synthorg/engine/workspace/models.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:
tests/unit/engine/test_workspace_git_worktree.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Applied to files:
tests/unit/engine/test_workspace_git_worktree.pysrc/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-15T21:49:53.264Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:49:53.264Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
src/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to **/*.py : Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Applied to files:
src/synthorg/engine/workspace/semantic_llm_prompt.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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 : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) instead of `except (A, B):` -- ruff enforces this on Python 3.14
Applied to files:
src/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-19T21:11:37.538Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T21:11:37.538Z
Learning: Applies to **/*.py : Errors: handle explicitly, never silently swallow.
Applied to files:
src/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/workspace/git_worktree.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/engine/workspace/git_worktree.py
🔇 Additional comments (26)
src/synthorg/engine/workspace/semantic_llm_prompt.py (4)
188-213: Silently skipping malformed conflict items can cause false negatives.Lines 189-205 log warnings but
continuepast invalid entries (non-dict items, missingfile_path/description). This can return partial or empty results from malformed LLM output instead of triggering retry/error handling, potentially missing real semantic conflicts.Consider raising
ValueErroron malformed items to trigger the retry loop in the caller, or at minimum tracking and surfacing malformed item counts so callers can decide whether to trust partial results.
34-83: LGTM! Well-structured tool definition with clear JSON schema.The
build_semantic_review_tool()function properly defines the tool schema with requiredconflicts(array) andsummaryfields, matching the expected tool-call interface.
86-142: LGTM! Clear prompt construction for semantic review.The system and review message builders provide focused instructions for detecting semantic conflicts without style issues, and properly format the code context for LLM review.
216-243: LGTM! Robust JSON fallback parsing.The
_parse_conflicts_from_content()function properly handles:
- Markdown-fenced JSON extraction
- Direct JSON parsing fallback
- Both dict and list structures
- Proper error logging and
ValueErrorpropagation for retry handlingtests/unit/engine/test_semantic_analyzer.py (1)
1-248: LGTM! Well-structured unit tests for semantic analyzers.The test module properly covers:
- Protocol compliance for both
AstSemanticAnalyzerandCompositeSemanticAnalyzer- Edge cases (empty files, non-Python files, max_files limit)
- Core functionality (conflict detection, clean merge, deduplication)
- Error resilience (single analyzer failure doesn't block others)
The use of
pytest.mark.unitand mocking patterns align with project conventions.src/synthorg/engine/workspace/models.py (3)
92-103: LGTM! Well-designed semantic conflict validation.The
descriptionfield with the_validate_semantic_descriptionmodel validator correctly enforces that semantic conflicts must have meaningful descriptions while allowing textual conflicts to have empty descriptions (since their content is captured inours_content/theirs_content).
141-161: LGTM! Consistent semantic_conflicts validation.The validation logic correctly ensures:
semantic_conflictscan only exist on successful merges (since semantic analysis runs post-merge)- Failed merges with textual conflicts cannot have semantic conflicts attached
203-209: LGTM! Consistent computed field for semantic conflicts aggregation.The
total_semantic_conflictscomputed property follows the same pattern astotal_conflicts, providing a convenient aggregate for merge group results.src/synthorg/engine/workspace/semantic_llm.py (3)
56-75: LGTM! Proper constructor with validation.The constructor validates the model parameter and provides a sensible default for
SemanticAnalysisConfigwhen not provided.
77-119: LGTM! Well-structured analyze method.The
analyze()method properly:
- Logs the analysis start event
- Delegates context preparation to a separate method
- Returns early when nothing to review
- Uses retry logic for LLM calls
172-240: LGTM! Robust retry logic with proper exception handling.The
_call_with_retry()method correctly:
- Retries on
ValueError(parse failures)- Re-raises
CancelledErrorfor proper cancellation handling- Logs appropriate events at different severity levels
- Returns empty tuple on exhaustion rather than failing the merge
src/synthorg/engine/workspace/git_worktree.py (4)
55-70: LGTM! Conservative path validation for git arguments.The
_validate_file_path()function properly rejects:
- Empty strings and flag-like arguments (
-)- Directory traversal (
..)- Absolute paths
- Characters outside a conservative allowlist
This prevents command injection in git path arguments.
307-351: LGTM! Clean separation of merge phases.The refactored
merge_workspace()properly:
- Executes git merge under lock
- Releases lock before semantic analysis (read-only SHA operations)
- Attaches semantic conflicts via
model_copy()for immutabilityThis addresses the concurrency concern from past reviews.
744-788: LGTM! Parallel base source fetching with TaskGroup.The
_get_base_sources()method usesasyncio.TaskGroupfor structured concurrency when fetching file contents via parallelgit showcalls, following the coding guideline preference for TaskGroup over barecreate_task.
831-888: LGTM! Robust semantic analysis with pre-filtering.The
_do_semantic_analysis()method correctly:
- Pre-filters files by extension and max_files before expensive git operations
- Falls back to
pre_merge_shaif merge-base fails- Catches and logs exceptions without failing the merge
- Re-raises
CancelledErrorfor proper cancellation handlingsrc/synthorg/engine/workspace/semantic_checks.py (4)
63-69: LGTM! Proper Load-context filtering for name references.The
_all_name_references()function now correctly filters toast.Loadcontext only (line 67), avoiding false positives from name bindings in other contexts.
153-180: LGTM! Proper handling of deleted files in removed-name collection.The
_collect_removed_names()function now correctly handles the case wheremerged_src is None(file deleted), treating all top-level names from the base as removed rather than silently skipping.
230-329: LGTM! Comprehensive signature change detection.The
_SigInfoNamedTuple now includesrequired_kwonly(line 236), and_compare_signatures()properly tracks new required keyword-only parameters (lines 291-309). The_check_call_compat()function validates calls against these constraints (lines 367-380).
493-522: LGTM! Deleted files handled in export collection.The
_collect_removed_exports()function properly handles deleted files (lines 505-512), marking all their exports as removed for import conflict detection.src/synthorg/engine/workspace/semantic_analyzer.py (3)
35-63: LGTM! Clean protocol definition for semantic analyzers.The
SemanticAnalyzerprotocol is properly decorated with@runtime_checkableand defines the expectedanalyze()signature with clear documentation.
140-225: LGTM! Well-structured AST semantic analyzer.The
AstSemanticAnalyzerclass properly:
- Filters files by extension and limits to max_files
- Offloads blocking I/O to thread via
asyncio.to_thread()- Runs all AST checks and aggregates results
- Logs appropriate observability events
228-297: LGTM! Robust composite analyzer with proper error isolation.The
CompositeSemanticAnalyzercorrectly:
- Runs analyzers sequentially and aggregates results
- Isolates failures (logs and skips failing analyzers)
- Re-raises
CancelledErrorfor proper cancellation- Deduplicates by
(file_path, description)to avoid duplicates from multiple analyzerstests/integration/engine/test_workspace_semantic_integration.py (4)
1-110: LGTM! Well-structured integration test helpers.The helper functions properly:
- Initialize git repos with required config (user.email, user.name)
- Commit files with proper git add/commit flow
- Create strategy with configurable semantic analysis
120-186: LGTM! Comprehensive renamed-function semantic conflict test.This test properly exercises the core semantic conflict scenario:
- Agent A renames a function
- Agent B adds a caller using the old name
- After merging both, the semantic conflict is correctly detected
236-326: LGTM! Duplicate definition test with disjoint hunks.The test now properly uses a spacer block to ensure git can perform a clean textual merge while producing duplicate
process()definitions for semantic analysis to catch. This addresses the past review concern about nondeterministic test behavior.
328-362: LGTM! Semantic disabled test ensures graceful degradation.This test verifies that when semantic analysis is disabled, merges complete successfully without any semantic conflicts being reported, even when potential issues exist.
| with patch(_READ_FN, return_value={"utils.py": merged_content}): | ||
| result = await analyzer.analyze( | ||
| workspace=_make_workspace(), | ||
| changed_files=("utils.py",), | ||
| repo_root="/tmp/repo", # noqa: S108 | ||
| base_sources={"utils.py": "def old_func():\n pass\n"}, | ||
| ) |
There was a problem hiding this comment.
Patch _read_file_contents() with strings, not mapping objects.
semantic_llm._read_file_contents(file_path, workspace_id) returns the contents of a single file as str. Returning {"foo.py": "..."} here changes the shape of merged_sources, so these tests either exercise an impossible code path or miss prompt-construction bugs entirely. Use a side_effect keyed by file_path instead.
🧪 Representative fix
- with patch(_READ_FN, return_value={"utils.py": merged_content}):
+ with patch(
+ _READ_FN,
+ side_effect=lambda file_path, _workspace_id: {
+ "utils.py": merged_content,
+ }[file_path],
+ ):
result = await analyzer.analyze(
workspace=_make_workspace(),
changed_files=("utils.py",),
repo_root="/tmp/repo", # noqa: S108
base_sources={"utils.py": "def old_func():\n pass\n"},Also applies to: 105-111, 141-147, 174-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/engine/test_semantic_llm.py` around lines 83 - 89, The tests patch
_read_file_contents with a mapping-returning value, but
semantic_llm._read_file_contents(file_path, workspace_id) should return a string
for a single file; update the patches in tests (around test_semantic_llm, lines
exercising analyzer.analyze and calls using merged_content) to use a side_effect
or per-file return that returns the file's string content keyed by the file_path
argument (i.e., return "def foo(): ..." for "utils.py") instead of returning
{"utils.py": "..."} so merged_sources keeps the correct shape and prompt
construction is exercised; apply the same change to the other occurrences noted
(lines ~105-111, ~141-147, ~174-180).
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workspace/config.py`:
- Around line 12-21: The class docstring for the semantic conflict detection
configuration is missing documentation for two fields: add entries for
llm_max_retries and max_file_bytes to the existing docstring near the other
attributes; describe llm_max_retries as the maximum number of retry attempts for
LLM-based semantic analysis (used with llm_model/llm_temperature/llm_max_tokens)
and max_file_bytes as the maximum allowed file size in bytes to analyze per file
(complements max_files and file_extensions), keeping the same docstring style as
the other attribute lines.
In `@src/synthorg/engine/workspace/semantic_analyzer.py`:
- Around line 176-180: The initial WORKSPACE_SEMANTIC_ANALYSIS_START log
currently uses file_count=len(changed_files) which is the unfiltered set; change
it to report the filtered/actual-to-be-analyzed count (e.g. the number of files
that match file_extensions or the length of merged_sources) so the start event
matches the completion metric. Locate the logger.info call that emits
WORKSPACE_SEMANTIC_ANALYSIS_START and replace file_count=len(changed_files) with
the filtered count (compute filtered_files by applying the same file_extensions
filter used later or reuse len(merged_sources) if available at that point) so
both start and completion logs reflect the same set size.
In `@src/synthorg/engine/workspace/semantic_llm.py`:
- Around line 243-281: Add an inline comment above
LlmSemanticAnalyzer._read_file_contents explaining that the byte-size limit
(max_file_bytes, default ~512 KB) is intentional to prevent exceeding LLM token
limits during semantic analysis and that the method intentionally logs read
errors at WARNING (not DEBUG) because LLM ingestion failures are operationally
more significant than AST-only analysis; reference the function name
_read_file_contents, the parameter max_file_bytes, and the differing behavior vs
_read_sources so future maintainers understand this is deliberate, not a bug to
be made consistent.
In `@tests/unit/engine/test_semantic_checks.py`:
- Line 18: The module already sets pytestmark = pytest.mark.unit to apply the
unit marker to all tests; remove the redundant `@pytest.mark.unit` decorators from
each individual test function in this file (i.e., delete the decorator lines
currently applied directly above the test functions) while leaving the
module-level pytestmark unchanged so all tests keep the unit marker.
In `@tests/unit/engine/test_workspace_models.py`:
- Around line 426-457: Combine the two tests
test_failed_merge_with_semantic_conflicts_rejected and
test_successful_merge_with_semantic_conflicts_accepted into a single
parametrized pytest using `@pytest.mark.parametrize` over the success boolean and
the expected outcome (raises ValidationError with "semantic conflicts" vs
accepts and has one semantic_conflict). Create the same MergeConflict
(file_path="a.py", conflict_type=ConflictType.SEMANTIC, description="issue")
inside the test and call MergeResult with workspace_id="ws-1", branch_name="b",
duration_seconds=0.1, semantic_conflicts=(sc,), varying success and
merged_commit_sha when success is True; assert accordingly (use pytest.raises
for the failure case and check len(mr.semantic_conflicts) == 1 for the success
case). Ensure the parametrized test name reflects the scenario.
🪄 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: d3f6e952-f517-47e8-9a04-6c7ee25f33d5
📒 Files selected for processing (5)
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_workspace_models.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax:except A, B:(no parentheses) instead ofexcept (A, B):-- ruff enforces this on Python 3.14
All public functions and classes must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskfor structured concurrency
Keep functions under 50 lines and files under 800 lines
Line length must be 88 characters as enforced by ruff
Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Files:
tests/unit/engine/test_semantic_checks.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.pytests/unit/engine/test_workspace_models.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowto categorize tests
Maintain 80% minimum code coverage as enforced in CI
Useasyncio_mode = 'auto'for async tests; no manual@pytest.mark.asyncioneeded
Use@pytest.mark.parametrizefor testing similar cases instead of test duplication
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, tests, or config examples; use generic names:example-provider,example-large-001,test-provider,test-small-001,large/medium/small
Use Hypothesis with@given+@settingsfor property-based testing; avoid flaky timing-based tests by mockingtime.monotonic()andasyncio.sleep()or usingasyncio.Event().wait()for indefinite blocks
Files:
tests/unit/engine/test_semantic_checks.pytests/unit/engine/test_workspace_models.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import and use logger:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the structured logger fromsynthorg.observability
Always use the logger variable namelogger(not_logger, notlog)
Use event name constants fromsynthorg.observability.events.<domain>modules (e.g.,API_REQUEST_STARTEDfromevents.api); import directly and always use structured kwargs:logger.info(EVENT, key=value)never string formatting
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
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields instead of manual validators
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Files:
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
🧠 Learnings (28)
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to tests/**/*.py : Use `pytest.mark.parametrize` for testing similar cases instead of test duplication
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.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/engine/workspace/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/engine/workspace/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/engine/workspace/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/engine/workspace/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:
src/synthorg/engine/workspace/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/engine/workspace/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:
src/synthorg/engine/workspace/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:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to **/*.py : Validate explicitly at system boundaries (user input, external APIs, config files); never silently swallow errors
Applied to files:
src/synthorg/engine/workspace/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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to src/synthorg/**/*.py : Validate at system boundaries: user input, external APIs, config files; handle errors explicitly, never silently swallow
Applied to files:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/engine/workspace/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/engine/workspace/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 **/*.py : Models: Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use computed_field for derived values 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/engine/workspace/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 **/*.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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 : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-26T21:34:31.847Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T21:34:31.847Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) instead of `except (A, B):` -- ruff enforces this on Python 3.14
Applied to files:
src/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-19T21:11:37.538Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T21:11:37.538Z
Learning: Applies to **/*.py : Errors: handle explicitly, never silently swallow.
Applied to files:
src/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).
Applied to files:
tests/unit/engine/test_workspace_models.py
🔇 Additional comments (6)
tests/unit/engine/test_workspace_models.py (1)
397-418: Good semantic-model coverage additions.
total_semantic_conflictsaggregation and bothMergeConflict.descriptionvalidation paths are now explicitly covered and align with model contracts.Also applies to: 459-478
tests/unit/engine/test_semantic_checks.py (1)
1-476: LGTM!The test file is well-structured with comprehensive parametrized test cases covering positive detections, negative cases, empty sources, and syntax error handling for all four AST semantic check functions. The use of
@pytest.mark.parametrizeaddresses the earlier review feedback effectively.src/synthorg/engine/workspace/config.py (1)
65-78: LGTM on the validation logic.The
file_extensionsvalidator correctly rejects empty tuples, blank strings, extensions missing the leading dot, and extensions containing spaces. This addresses the earlier review feedback.src/synthorg/engine/workspace/semantic_llm.py (1)
172-240: LGTM on retry and error handling logic.The
_call_with_retrymethod correctly:
- Retries on
ValueErrorfrom parse failures up tomax_retries- Re-raises
asyncio.CancelledErrorto respect cancellation- Logs provider errors with
exc_info=Truefor debugging- Returns empty tuple on exhaustion rather than raising
src/synthorg/engine/workspace/semantic_analyzer.py (2)
35-63: LGTM on theSemanticAnalyzerprotocol definition.The protocol is well-defined with
@runtime_checkable, comprehensive docstrings, and appropriate type hints includingMapping[str, str]forbase_sources(allowing immutable views).
228-297: LGTM onCompositeSemanticAnalyzer.The implementation correctly:
- Runs analyzers sequentially and aggregates results
- Re-raises
asyncio.CancelledErrorto respect structured concurrency- Logs and skips failing analyzers with full exception info
- Deduplicates conflicts by
(file_path, description)tuple
| def test_failed_merge_with_semantic_conflicts_rejected(self) -> None: | ||
| """Failed merge cannot have semantic conflicts.""" | ||
| sc = MergeConflict( | ||
| file_path="a.py", | ||
| conflict_type=ConflictType.SEMANTIC, | ||
| description="issue", | ||
| ) | ||
| with pytest.raises(ValidationError, match="semantic conflicts"): | ||
| MergeResult( | ||
| workspace_id="ws-1", | ||
| branch_name="b", | ||
| success=False, | ||
| duration_seconds=0.1, | ||
| semantic_conflicts=(sc,), | ||
| ) | ||
|
|
||
| def test_successful_merge_with_semantic_conflicts_accepted(self) -> None: | ||
| """Successful merge can have semantic conflicts.""" | ||
| sc = MergeConflict( | ||
| file_path="a.py", | ||
| conflict_type=ConflictType.SEMANTIC, | ||
| description="issue", | ||
| ) | ||
| mr = MergeResult( | ||
| workspace_id="ws-1", | ||
| branch_name="b", | ||
| success=True, | ||
| merged_commit_sha="abc", | ||
| duration_seconds=0.1, | ||
| semantic_conflicts=(sc,), | ||
| ) | ||
| assert len(mr.semantic_conflicts) == 1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Refactor similar semantic-conflict cases into one parametrized test.
These two tests duplicate setup/constructor paths and differ mainly by success and expected outcome. Converting to @pytest.mark.parametrize will reduce drift and keep intent tighter.
♻️ Proposed refactor
class TestMergeResultSemanticConflicts:
"""Tests for semantic_conflicts field and validator."""
pytestmark = pytest.mark.unit
- def test_failed_merge_with_semantic_conflicts_rejected(self) -> None:
- """Failed merge cannot have semantic conflicts."""
- sc = MergeConflict(
- file_path="a.py",
- conflict_type=ConflictType.SEMANTIC,
- description="issue",
- )
- with pytest.raises(ValidationError, match="semantic conflicts"):
- MergeResult(
- workspace_id="ws-1",
- branch_name="b",
- success=False,
- duration_seconds=0.1,
- semantic_conflicts=(sc,),
- )
-
- def test_successful_merge_with_semantic_conflicts_accepted(self) -> None:
- """Successful merge can have semantic conflicts."""
+ `@pytest.mark.parametrize`(
+ ("success", "expect_error"),
+ [
+ (False, True),
+ (True, False),
+ ],
+ )
+ def test_semantic_conflicts_success_consistency(
+ self,
+ success: bool,
+ expect_error: bool,
+ ) -> None:
+ """semantic_conflicts are rejected on failed merges and allowed on success."""
sc = MergeConflict(
file_path="a.py",
conflict_type=ConflictType.SEMANTIC,
description="issue",
)
- mr = MergeResult(
- workspace_id="ws-1",
- branch_name="b",
- success=True,
- merged_commit_sha="abc",
- duration_seconds=0.1,
- semantic_conflicts=(sc,),
- )
- assert len(mr.semantic_conflicts) == 1
+ if expect_error:
+ with pytest.raises(ValidationError, match="semantic conflicts"):
+ MergeResult(
+ workspace_id="ws-1",
+ branch_name="b",
+ success=success,
+ duration_seconds=0.1,
+ semantic_conflicts=(sc,),
+ )
+ return
+ mr = MergeResult(
+ workspace_id="ws-1",
+ branch_name="b",
+ success=success,
+ merged_commit_sha="abc",
+ duration_seconds=0.1,
+ semantic_conflicts=(sc,),
+ )
+ assert len(mr.semantic_conflicts) == 1As per coding guidelines: "Use @pytest.mark.parametrize for testing similar cases instead of test duplication".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/engine/test_workspace_models.py` around lines 426 - 457, Combine
the two tests test_failed_merge_with_semantic_conflicts_rejected and
test_successful_merge_with_semantic_conflicts_accepted into a single
parametrized pytest using `@pytest.mark.parametrize` over the success boolean and
the expected outcome (raises ValidationError with "semantic conflicts" vs
accepts and has one semantic_conflict). Create the same MergeConflict
(file_path="a.py", conflict_type=ConflictType.SEMANTIC, description="issue")
inside the test and call MergeResult with workspace_id="ws-1", branch_name="b",
duration_seconds=0.1, semantic_conflicts=(sc,), varying success and
merged_commit_sha when success is True; assert accordingly (use pytest.raises
for the failure case and check len(mr.semantic_conflicts) == 1 for the success
case). Ensure the parametrized test name reflects the scenario.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workspace/config.py`:
- Around line 73-79: The validator loop over self.file_extensions currently
allows the single-character extension "." which would match all filenames;
update the check in the loop that iterates over ext so it rejects "." (e.g.,
require len(ext) >= 2 or explicitly check ext != ".") in the same place where
you validate emptiness/leading dot/space, and keep raising the ValueError(msg)
with the same message when the new condition fails; this ensures the ext
variable (in the for ext in self.file_extensions loop) cannot be just ".".
🪄 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: 600bfc69-a23c-4e01-a2e8-9dec65a23c17
📒 Files selected for processing (4)
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.pytests/unit/engine/test_semantic_checks.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: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax (no parentheses) for multiple exception types — PEP 758 style enforced by ruff on Python 3.14
Type hints required on all public functions — must pass mypy strict mode
Google-style docstrings required on all public classes and functions — enforced by ruff D rules
Create new objects instead of mutating existing ones — use copy.deepcopy() at construction for non-Pydantic internal collections; use MappingProxyType wrapping for read-only enforcement
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state that evolves — never mix static config fields with mutable runtime fields
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models
Use NotBlankStr (from core.types) for all identifier/name fields in Pydantic models — including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants — instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g. multiple tool invocations, parallel agent calls) over bare create_task
Line length: 88 characters (ruff-enforced)
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files) in Python code
Files:
tests/unit/engine/test_semantic_checks.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.py
{src,tests,cli,web}/**/*.{py,ts,tsx,go}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small aliases. Vendor names allowed only in: (1) Operations design page, (2) .claude/ files, (3) third-party import paths
Files:
tests/unit/engine/test_semantic_checks.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowtest markers
Use asyncio_mode = 'auto' for async tests — no manual@pytest.mark.asyncioneeded
Global test timeout: 30 seconds per test (in pyproject.toml) — non-default overrides like@pytest.mark.timeout(60) are allowed
Prefer@pytest.mark.parametrizefor testing similar cases in Python tests
Usetest-provider,test-small-001, etc. for vendor-agnostic test data in Python and React tests
Use Hypothesis for property-based testing in Python (@given+@settings); control with HYPOTHESIS_PROFILE env var (ci: 50 examples default, dev: 1000 examples)
Never skip, dismiss, or ignore flaky tests — always fix them fully. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic. For tasks blocking indefinitely, use asyncio.Event().wait() instead of asyncio.sleep(large_number)
Files:
tests/unit/engine/test_semantic_checks.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code — exception: observability/setup.py and observability/sinks.py may use stdlib logging and stderr print for bootstrap
Always name logger variablelogger(not_logger, notlog)
Always use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) — import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured logging with kwargs:logger.info(EVENT, key=value)— never use string formatting likelogger.info('msg %s', val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
DEBUG logging for object creation, internal flow, entry/exit of key functions
Maintain 80% minimum code coverage (enforced in CI)
Files:
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.py
🧠 Learnings (30)
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to tests/**/*.py : Prefer pytest.mark.parametrize for testing similar cases in Python tests
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to tests/**/*.py : Use pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow test markers
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/engine/test_semantic_checks.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/engine/workspace/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/engine/workspace/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/engine/workspace/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:
src/synthorg/engine/workspace/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/engine/workspace/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/engine/workspace/config.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to src/synthorg/config/**/*.py : YAML company config loading and validation in src/synthorg/config/ — never hardcode configuration
Applied to files:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to **/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state that evolves — never mix static config fields with mutable runtime fields
Applied to files:
src/synthorg/engine/workspace/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:
src/synthorg/engine/workspace/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:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files) in Python code
Applied to files:
src/synthorg/engine/workspace/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 : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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 **/*.py : Models: Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use computed_field for derived values 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/engine/workspace/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/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for multiple exception types — PEP 758 style enforced by ruff on Python 3.14
Applied to files:
src/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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 : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.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/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_llm.py
🔇 Additional comments (4)
tests/unit/engine/test_semantic_checks.py (1)
1-467: LGTM! Well-structured parametrized tests.The test module properly uses:
- Module-level
pytestmark = pytest.mark.unit(no redundant decorators on individual methods)@pytest.mark.parametrizefor similar test cases- Edge case coverage for empty sources and syntax error handling
Minor observation:
TestCheckImportConflictslacks atest_syntax_error_skippedcase that other classes have, but this is a minor coverage gap since the underlying behavior is consistent across all check functions.src/synthorg/engine/workspace/config.py (1)
118-121: LGTM!Correctly uses
default_factory=SemanticAnalysisConfigfor the nested configuration, following Pydantic best practices for mutable default values.src/synthorg/engine/workspace/semantic_llm.py (1)
1-302: LGTM! Well-implemented LLM semantic analyzer.The implementation correctly:
- Validates model parameter on construction
- Filters and limits files per configuration
- Implements retry logic with proper attempt counting
- Protects against path traversal attacks
- Enforces file size limits for LLM token constraints
- Documents intentional divergence from AST analyzer's file reading
- Uses structured logging throughout
src/synthorg/engine/workspace/semantic_analyzer.py (1)
1-297: LGTM! Clean implementation of semantic analyzer hierarchy.The module provides:
- Well-defined
SemanticAnalyzerprotocol with@runtime_checkableAstSemanticAnalyzerwith proper file filtering, path traversal protection, and structured loggingCompositeSemanticAnalyzerwith correct exception handling (re-raisesCancelledError, logs and continues on other exceptions) and sensible deduplicationThe deduplication by
(file_path, description)works correctly since theMergeConflictmodel validates thatSEMANTICconflicts must have non-empty descriptions.
Add hybrid AST + LLM semantic conflict detection after workspace merges. Detects removed-name references, signature mismatches, duplicate definitions, and import conflicts that git's textual merge cannot catch. New modules: - semantic_checks.py: Pure AST-based check functions - semantic_analyzer.py: SemanticAnalyzer protocol + AstSemanticAnalyzer - semantic_llm.py: LlmSemanticAnalyzer for deeper review - semantic_llm_prompt.py: Prompt building and response parsing Key changes: - MergeConflict: add description field - MergeResult: add semantic_conflicts field (separate from textual) - PlannerWorktreesConfig: add SemanticAnalysisConfig - PlannerWorktreeStrategy: accept optional SemanticAnalyzer, run post-merge analysis using merge-base for cross-branch detection - MergeOrchestrator: handle semantic conflict escalation Closes #611 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 11 agents, 18 findings addressed: - Replace bare string event with WORKSPACE_SEMANTIC_PARSE_SKIP constant - Add exc_info=True to all except Exception warning logs - Fix _function_min_args/_function_max_args to include posonlyargs - Fix module stem derivation to strip src/ prefix for import checks - Replace _SigInfo tuple alias with NamedTuple + frozenset - Replace contextlib.suppress with explicit logging in _read_file_contents - Add warning logs before all ValueError/TypeError raises in prompt parser - Use WORKSPACE_SEMANTIC_ANALYSIS_FAILED for semantic git helper log events - Split long functions (analyze, _run_semantic_analysis) into helpers - Add MergeResult.semantic_conflicts to docstring - Add tests: merge orchestrator semantic escalation (HUMAN/REVIEW_AGENT) - Add tests: MergeResult validator, total_semantic_conflicts, keyword args Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TypeError bypassed the retry loop in _call_with_retry which only catches ValueError. This caused malformed LLM output to be treated as a fatal provider error instead of a retryable parse error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements all findings from 14 local review agents, CodeRabbit, and Gemini: - Security: path traversal validation, file path sanitization in git show args - Concurrency: move semantic analysis outside merge lock, use TaskGroup for parallel git show, add CancelledError re-raise before broad except handlers - Config: enforce enabled flag, add max_file_bytes and llm_max_retries fields, pre-filter changed files by extension/max_files before git show calls - AST checks: Load-only name references, handle deleted files in collectors, track required keyword-only params, fix variadic sentinel false positives, fix __init__.py module stem, allow same-file reference detection - Types: Mapping[str, str] in Protocol/check functions, MergeConflict semantic description validator, replace assert with SemanticAnalysisError guard - Logging: add structured logging to git helpers, capture exc details in error handlers, log malformed LLM conflict items - Functions: refactor to stay under 50-line limit, extract _prepare_review_context and _merge_under_lock helpers - Docs: update design spec protocol signature, add semantic analysis section - Tests: 26 new unit tests for git helpers, fix integration test determinism - Export LlmSemanticAnalyzer from workspace __init__.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- config: add file_extensions validator (reject empty/malformed extensions) - semantic_analyzer: simplify except to OSError (subclasses are redundant) - semantic_llm: same OSError simplification - test_semantic_checks: collapse into @pytest.mark.parametrize per CLAUDE.md - test_workspace_models: add negative test for semantic description validator Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- config: add llm_max_retries and max_file_bytes to docstring Attributes - semantic_analyzer: log filtered file_count (not unfiltered) at start - semantic_llm: add docstring explaining _read_file_contents vs _read_sources - test_semantic_checks: remove redundant @pytest.mark.unit (pytestmark covers) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevent "." from matching all filenames by requiring extensions to have at least one character after the dot. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
feda149 to
3245d5e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/synthorg/engine/workspace/semantic_llm_prompt.py (1)
188-205:⚠️ Potential issue | 🟠 MajorPartial results returned when LLM produces malformed conflict items.
Lines 188-205 log warnings but
continueon invalid entries, potentially returning an incomplete conflict list. This masks malformed provider output and can cause false negatives in semantic conflict detection, as the retry loop only triggers onValueError.Given the previous review flagged this as a major issue, consider strict validation that raises
ValueErroron any malformed item to trigger retry logic:♻️ Suggested strict validation
for idx, item in enumerate(raw_conflicts): if not isinstance(item, dict): logger.warning( WORKSPACE_SEMANTIC_ANALYSIS_FAILED, reason="unexpected_conflict_item_type", item_type=type(item).__name__, + index=idx, ) - continue + msg = f"Conflict at index {idx} is not a dict" + raise ValueError(msg) file_path = item.get("file_path", "") description = item.get("description", "") if not file_path or not description: logger.warning( WORKSPACE_SEMANTIC_ANALYSIS_FAILED, reason="incomplete_conflict_item", file_path=file_path or "(empty)", has_description=bool(description), + index=idx, ) - continue + msg = f"Conflict at index {idx} missing required fields" + raise ValueError(msg)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/workspace/semantic_llm_prompt.py` around lines 188 - 205, The current loop over raw_conflicts silently skips malformed items (in the for loop handling raw_conflicts) by logging WORKSPACE_SEMANTIC_ANALYSIS_FAILED and using continue, which leads to partial results and prevents the retry logic that depends on ValueError; change the validation inside that loop (the isinstance(item, dict) check and the file_path/description presence checks) to raise a ValueError with a clear message referencing the problematic item (include item_type or file_path/description state) instead of continuing so that the caller’s retry logic is triggered and malformed provider output is surfaced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/workspace/semantic_checks.py`:
- Around line 475-490: The module-stem conversion in _file_path_to_module_stem
only removes "src/" and "lib/" prefixes which misses other common layouts;
update this function to handle additional common prefixes (e.g., "packages/",
"app/", "src/main/python/") by replacing the hard-coded ("src/", "lib/") tuple
with a configurable or extended tuple (e.g., MODULE_ROOT_PREFIXES) and iterate
over that to strip any matching prefix before converting separators and trimming
"__init__"; ensure you keep the existing behavior for Windows separators and the
__init__ handling in _file_path_to_module_stem.
In `@src/synthorg/engine/workspace/semantic_llm_prompt.py`:
- Around line 136-137: The loop in build_review_message currently hardcodes a
"python" code fence for every changed_files entry which can mislabel non-.py
files; update the loop that iterates over changed_files (the for path, content
in changed_files.items()) to derive a fence language from the file extension
(e.g., ext = path.rsplit(".", 1)[-1] if "." in path else ""), map common
extensions to languages (e.g., {"py":"python","pyi":"python","pyx":"cython"}),
and use that mapped lang when building parts.append(f"\n###
{path}\n```{lang}\n{content}\n```\n") falling back to an empty fence if unknown.
In `@src/synthorg/engine/workspace/semantic_llm.py`:
- Around line 172-240: The retry loop in _call_with_retry currently resends
identical messages/tool_def on ValueError; change it to append a short
system/user hint on retry attempts to tell the model the previous response was
malformed (e.g., add a ChatMessage like "Previous response could not be parsed;
please return valid JSON using the specified tool schema." when attempt>0)
before calling self._provider.complete, or alternatively document in the
function docstring that successful retries depend on non-zero llm_temperature in
comp_config; update the call site that constructs messages (used by
_call_with_retry) so the hint is injected only for retry attempts and reference
_provider.complete, parse_tool_call_response, and comp_config in the change.
---
Duplicate comments:
In `@src/synthorg/engine/workspace/semantic_llm_prompt.py`:
- Around line 188-205: The current loop over raw_conflicts silently skips
malformed items (in the for loop handling raw_conflicts) by logging
WORKSPACE_SEMANTIC_ANALYSIS_FAILED and using continue, which leads to partial
results and prevents the retry logic that depends on ValueError; change the
validation inside that loop (the isinstance(item, dict) check and the
file_path/description presence checks) to raise a ValueError with a clear
message referencing the problematic item (include item_type or
file_path/description state) instead of continuing so that the caller’s retry
logic is triggered and malformed provider output is surfaced.
🪄 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: a44f7b32-c2d6-4511-8881-6a17d0389f02
📒 Files selected for processing (22)
CLAUDE.mddocs/design/engine.mdsrc/synthorg/engine/errors.pysrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/merge.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/semantic_analyzer.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/observability/events/workspace.pytests/integration/engine/test_workspace_semantic_integration.pytests/unit/engine/conftest.pytests/unit/engine/test_semantic_analyzer.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_semantic_llm_prompt.pytests/unit/engine/test_workspace_git_worktree.pytests/unit/engine/test_workspace_merge.pytests/unit/engine/test_workspace_models.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Deploy Preview
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax (no parentheses) for multiple exception types — PEP 758 style enforced by ruff on Python 3.14
Type hints required on all public functions — must pass mypy strict mode
Google-style docstrings required on all public classes and functions — enforced by ruff D rules
Create new objects instead of mutating existing ones — use copy.deepcopy() at construction for non-Pydantic internal collections; use MappingProxyType wrapping for read-only enforcement
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state that evolves — never mix static config fields with mutable runtime fields
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models
Use NotBlankStr (from core.types) for all identifier/name fields in Pydantic models — including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants — instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g. multiple tool invocations, parallel agent calls) over bare create_task
Line length: 88 characters (ruff-enforced)
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files) in Python code
Files:
tests/unit/engine/conftest.pysrc/synthorg/engine/errors.pytests/unit/engine/test_workspace_models.pysrc/synthorg/engine/workspace/__init__.pytests/unit/engine/test_semantic_llm_prompt.pytests/unit/engine/test_semantic_llm.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/config.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_workspace_git_worktree.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pytests/unit/engine/test_workspace_merge.pytests/unit/engine/test_semantic_analyzer.pysrc/synthorg/engine/workspace/merge.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/workspace/git_worktree.pytests/integration/engine/test_workspace_semantic_integration.pysrc/synthorg/engine/workspace/semantic_analyzer.py
{src,tests,cli,web}/**/*.{py,ts,tsx,go}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small aliases. Vendor names allowed only in: (1) Operations design page, (2) .claude/ files, (3) third-party import paths
Files:
tests/unit/engine/conftest.pysrc/synthorg/engine/errors.pytests/unit/engine/test_workspace_models.pysrc/synthorg/engine/workspace/__init__.pytests/unit/engine/test_semantic_llm_prompt.pytests/unit/engine/test_semantic_llm.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/config.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_workspace_git_worktree.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pytests/unit/engine/test_workspace_merge.pytests/unit/engine/test_semantic_analyzer.pysrc/synthorg/engine/workspace/merge.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/workspace/git_worktree.pytests/integration/engine/test_workspace_semantic_integration.pysrc/synthorg/engine/workspace/semantic_analyzer.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowtest markers
Use asyncio_mode = 'auto' for async tests — no manual@pytest.mark.asyncioneeded
Global test timeout: 30 seconds per test (in pyproject.toml) — non-default overrides like@pytest.mark.timeout(60) are allowed
Prefer@pytest.mark.parametrizefor testing similar cases in Python tests
Usetest-provider,test-small-001, etc. for vendor-agnostic test data in Python and React tests
Use Hypothesis for property-based testing in Python (@given+@settings); control with HYPOTHESIS_PROFILE env var (ci: 50 examples default, dev: 1000 examples)
Never skip, dismiss, or ignore flaky tests — always fix them fully. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic. For tasks blocking indefinitely, use asyncio.Event().wait() instead of asyncio.sleep(large_number)
Files:
tests/unit/engine/conftest.pytests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_llm_prompt.pytests/unit/engine/test_semantic_llm.pytests/unit/engine/test_semantic_checks.pytests/unit/engine/test_workspace_git_worktree.pytests/unit/engine/test_workspace_merge.pytests/unit/engine/test_semantic_analyzer.pytests/integration/engine/test_workspace_semantic_integration.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code — exception: observability/setup.py and observability/sinks.py may use stdlib logging and stderr print for bootstrap
Always name logger variablelogger(not_logger, notlog)
Always use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) — import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured logging with kwargs:logger.info(EVENT, key=value)— never use string formatting likelogger.info('msg %s', val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
DEBUG logging for object creation, internal flow, entry/exit of key functions
Maintain 80% minimum code coverage (enforced in CI)
Files:
src/synthorg/engine/errors.pysrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/merge.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/workspace/git_worktree.pysrc/synthorg/engine/workspace/semantic_analyzer.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Docs: Markdown in docs/ (built with Zensical, config: mkdocs.yml) — design spec: docs/design/ (11 pages), Architecture: docs/architecture/, Roadmap: docs/roadmap/, Security: docs/security.md, Licensing: docs/licensing.md, Reference: docs/reference/
Files:
docs/design/engine.md
🧠 Learnings (48)
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/engine/errors.pysrc/synthorg/engine/workspace/config.pytests/unit/engine/test_workspace_git_worktree.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
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 : 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:
CLAUDE.mdsrc/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/semantic_llm_prompt.pysrc/synthorg/engine/workspace/semantic_checks.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
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/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:
CLAUDE.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/hr/**/*.py : HR engine must provide: hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, trend detection), promotion/demotion
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.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: Applies to src/synthorg/hr/**/*.py : HR package (hr/): hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, LLM calibration, collaboration overrides, trend detection), promotion/demotion (criteria evaluation, approval strategies, model mapping)
Applied to files:
CLAUDE.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/engine/coordination/**/*.py : Task coordination uses multi-agent pipeline with 4 dispatchers (SAS/centralized/decentralized/context-dependent), wave execution, and workspace lifecycle integration.
Applied to files:
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 docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
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: 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:
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 tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).
Applied to files:
tests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to tests/**/*.py : Prefer pytest.mark.parametrize for testing similar cases in Python tests
Applied to files:
tests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/engine/test_workspace_models.pytests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to src/synthorg/config/**/*.py : YAML company config loading and validation in src/synthorg/config/ — never hardcode configuration
Applied to files:
src/synthorg/engine/workspace/__init__.pysrc/synthorg/engine/workspace/models.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/engine/workspace/models.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/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/engine/workspace/models.pysrc/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-17T11:41:02.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T11:41:02.964Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing + validating redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/engine/workspace/models.pysrc/synthorg/engine/workspace/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 **/*.py : Models: Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use computed_field for derived values 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/engine/workspace/models.pysrc/synthorg/engine/workspace/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/engine/workspace/models.pysrc/synthorg/engine/workspace/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/engine/workspace/models.pysrc/synthorg/engine/workspace/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/engine/workspace/models.pysrc/synthorg/engine/workspace/config.pysrc/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings package (settings/): runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge (JSON serialization for Pydantic/collections), ConfigResolver (typed accessors), validation, registry, change notifications via message bus, SettingsSubscriber protocol, SettingsChangeDispatcher (polls `#settings` channel, routes to subscribers, restart_required filtering)
Applied to files:
src/synthorg/engine/workspace/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:
src/synthorg/engine/workspace/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:
src/synthorg/engine/workspace/config.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files) in Python code
Applied to files:
src/synthorg/engine/workspace/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/engine/test_semantic_checks.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to tests/**/*.py : Use pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow test markers
Applied to files:
tests/unit/engine/test_semantic_checks.py
📚 Learning: 2026-03-15T21:49:53.264Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:49:53.264Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
src/synthorg/engine/workspace/semantic_llm_prompt.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for multiple exception types — PEP 758 style enforced by ruff on Python 3.14
Applied to files:
src/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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 : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.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/engine/workspace/semantic_llm.pysrc/synthorg/engine/workspace/semantic_analyzer.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) — import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/workspace.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/workspace/git_worktree.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/engine/workspace/git_worktree.py
📚 Learning: 2026-03-26T23:18:47.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T23:18:47.548Z
Learning: Applies to **/*.py : Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g. multiple tool invocations, parallel agent calls) over bare create_task
Applied to files:
src/synthorg/engine/workspace/git_worktree.py
🔇 Additional comments (54)
CLAUDE.md (1)
113-113: LGTM!The documentation update accurately captures the new workspace capabilities: git worktree isolation, merge orchestration, and semantic conflict detection. This aligns with the production code changes introduced in the PR.
src/synthorg/engine/errors.py (1)
90-92: LGTM!The
SemanticAnalysisErrorfollows the established error hierarchy pattern, correctly subclassingWorkspaceErroralongside its siblings (WorkspaceSetupError,WorkspaceMergeError, etc.). The docstring is appropriately concise.src/synthorg/engine/workspace/merge.py (1)
138-149: LGTM!The semantic conflict handling correctly mirrors the existing textual conflict pattern:
- Updates
escalationviamodel_copy()(immutable pattern)- Appends result before the break check
- Breaks only on
HUMANescalation, allowingREVIEW_AGENTto continueThe placement after line 143 ensures the semantic-conflict result is captured before early termination.
tests/unit/engine/conftest.py (1)
359-376: LGTM!The
make_merge_resulthelper is correctly extended withsemantic_conflictsdefaulting to an empty tuple, maintaining backward compatibility with existing tests while supporting new semantic conflict test scenarios.src/synthorg/engine/workspace/__init__.py (1)
26-48: LGTM!The module's public API is correctly expanded to expose the new semantic analysis components. The
__all__list maintains alphabetical ordering, and the imports align with the new semantic analyzer modules introduced in this PR.tests/unit/engine/test_workspace_models.py (1)
396-478: LGTM!Comprehensive test coverage for the new semantic conflict modeling:
test_total_semantic_conflicts: ValidatesWorkspaceGroupResultaggregationTestMergeResultSemanticConflicts: Covers the validation rules (rejected on failure, accepted on success) and theMergeConflict.descriptionconstraint forConflictType.SEMANTICAll tests have proper
@pytest.mark.unitmarkers (class-levelpytestmarkfor the new test class).docs/design/engine.md (1)
924-981: LGTM!The design documentation comprehensively covers the new semantic conflict detection feature:
- Configuration block aligns with
SemanticAnalysisConfigfieldsSemanticAnalyzerprotocol and composite pattern are clearly documented- Four AST-based checks match the implementation in
semantic_checks.py- Optional LLM-based analysis configuration is accurately described
- The merge flow through
MergeResult.semantic_conflictsis correctly documentedThis provides essential reference material for developers working with the workspace merge pipeline.
tests/unit/engine/test_workspace_git_worktree.py (8)
10-23: LGTM!The imports and
_make_strategyhelper are correctly updated to support semantic analyzer injection for testing. Thesemantic_analyzerparameter withobject | Nonetyping and type ignore comment allows flexible test doubles.
263-289: LGTM!The merge success test correctly updated to include the pre-merge
rev-parse HEADcall that captures the SHA before the merge operation. This aligns with the production code flow wherepre_merge_shais captured during_merge_under_lockand later passed to semantic analysis.
740-777: LGTM!Comprehensive coverage of
_validate_file_pathsecurity validation:
- Empty strings, dash-prefixed (flag injection), directory traversal (
..), absolute paths- Valid relative paths, paths with spaces (allowed), disallowed special characters
These tests ensure the path allowlist protects against command injection vectors.
784-826: LGTM!Good coverage of
_get_merge_baseincluding success and failure paths. The test correctly verifies thelog_eventparameter is passed through to_run_git.
833-904: LGTM!Thorough testing of
_get_changed_filesincluding the critical security test at lines 888-904 that verifies unsafe paths (flag-like--malicious, traversal../escape.py) are filtered from git diff output.
912-998: LGTM!Good coverage of
_get_base_sources:
- Successful file content retrieval
- Graceful handling of failed
git show(files omitted, not errored)- Unsafe path filtering before git operations
1005-1141: LGTM!Comprehensive control flow testing for
_run_semantic_analysis:
- No analyzer configured → returns empty
- Empty
pre_merge_sha→ returns empty- Disabled config → returns empty
- Enabled → delegates to
_do_semantic_analysis- Conflicts found → returns conflicts (warning logged)
1148-1297: LGTM!Excellent coverage of
_do_semantic_analysis:
SemanticAnalysisErrorraised when analyzer is None (defensive check)- Non-
CancelledErrorexceptions caught and swallowed (returns empty)CancelledErrorproperly re-raised (not swallowed)- Extension filtering ensures only configured file types are analyzed
The cancellation behavior is critical for cooperative shutdown - good to see it explicitly tested.
src/synthorg/observability/events/workspace.py (1)
25-31: Well-structured event constants for the semantic analysis pipeline.The new constants follow the established naming convention and value pattern used by existing workspace events. Good consistency with the domain-specific event organization.
tests/unit/engine/test_semantic_llm.py (3)
57-63: Good validation test for blank model rejection.The test correctly verifies that
LlmSemanticAnalyzerraisesValueErrorwith a "non-blank" message when given an empty model string. This ensures proper input validation at construction time.
151-183: Retry behavior test is well-structured.The test clearly validates the retry-on-parse-failure mechanism by providing an unparseable first response followed by a valid second response, then asserting both the result and that the provider was called twice.
83-89: Remove comment—the mock return type is correct.
_read_file_contents(root: Path, files: list[str], max_file_bytes: int)returnsdict[str, str], not a string. The mock correctly returns a dictionary mapping file paths to content strings, matching the production implementation.> Likely an incorrect or invalid review comment.src/synthorg/engine/workspace/models.py (3)
92-103: Well-designed validation for semantic conflict descriptions.The
descriptionfield defaults to empty string (allowing textual conflicts to omit it), while the validator correctly enforces non-empty descriptions forSEMANTICconflicts. This aligns with the relevant code snippets showing all semantic conflict instantiations provide meaningful descriptions.
141-160: Semantic conflicts validation logic is correct.The validation properly enforces that:
- Semantic conflicts can only exist on successful merges (since they run post-merge)
- Failed merges must have an empty
semantic_conflictstupleThis maintains model consistency with the merge workflow design.
203-209: Good use of@computed_fieldfor aggregated metrics.The
total_semantic_conflictsproperty follows the same pattern astotal_conflicts, maintaining consistency in the model's API. As per coding guidelines, usingcomputed_fieldfor derived values is the correct approach.src/synthorg/engine/workspace/config.py (2)
67-80: File extension validation is comprehensive.The validator correctly rejects:
- Empty tuple (
not self.file_extensions)- Empty strings (
not ext)- Single dot (
ext == ".")- Non-dot-prefixed extensions
- Extensions containing spaces
This addresses all the edge cases from past reviews.
11-65: Well-structured semantic analysis configuration.The config model follows project conventions: frozen Pydantic model with explicit bounds on numeric fields. The defaults are sensible (disabled by default, reasonable limits for max_files, llm_max_tokens, etc.).
tests/unit/engine/test_semantic_analyzer.py (3)
60-65: Good protocol compliance tests.Testing
isinstance(analyzer, SemanticAnalyzer)ensures the implementations satisfy the protocol contract, which is important for the composite pattern to work correctly.
106-129: Effective end-to-end semantic conflict detection test.This test validates the core use case: a function renamed in one file while another file still references the old name. The assertion
len(result) >= 1allows for potential multiple conflict types to be detected.
226-244: Resilience test validates fault isolation.This correctly tests that one analyzer's exception doesn't prevent other analyzers from contributing their results, matching the production behavior shown in the relevant code snippet.
tests/unit/engine/test_workspace_merge.py (2)
383-388: Semantic conflict helper correctly includes required description.The helper provides a non-empty
descriptionfield, satisfying theMergeConflictvalidator that requires descriptions forSEMANTICconflict types.
391-447: Comprehensive escalation tests for semantic conflicts.Both tests clearly validate the expected orchestrator behavior:
HUMANescalation stops processing after the first workspace with semantic conflictsREVIEW_AGENTescalation continues to subsequent workspacesThe assertions verify result counts, escalation values, and semantic conflict presence.
tests/unit/engine/test_semantic_llm_prompt.py (3)
22-36: Tool schema tests validate contract with LLM provider.These tests ensure the tool definition structure matches what's expected by the semantic review workflow, including the required
file_pathanddescriptionfields for conflict items.
133-144: Good coverage of JSON fallback parsing.Testing the
jsoncode fence fallback ensures the parser handles cases where the LLM returns JSON in content rather than through tool calls, improving robustness.
156-172: Partial parsing validation is important for resilience.This test verifies that invalid conflict items (empty file_path, non-dict entries) are skipped while valid items are still parsed, preventing a single malformed item from breaking the entire response.
tests/unit/engine/test_semantic_checks.py (4)
29-126: Excellent use of parametrized tests for removed references.The test matrix covers key scenarios: function/class/constant removal with references, removal without references, and name still present. The
description_containsparameter enables flexible assertion on conflict details.
136-149: Syntax error resilience tests are important.These tests verify that unparseable source files don't raise exceptions, ensuring the semantic checks degrade gracefully when encountering invalid Python syntax.
228-251: Good coverage of varargs and signature compatibility.The
varargs-accepts-extra-positionalandsignature-unchanged-no-conflictcases ensure the signature checker doesn't produce false positives for compatible changes.
324-338: Nested same-name case prevents false positives.Testing that a method named
helperinside a class doesn't conflict with a top-level functionhelperis important for avoiding spurious duplicate definition warnings.src/synthorg/engine/workspace/semantic_llm_prompt.py (1)
216-243: LGTM!The
_parse_conflicts_from_contentfunction correctly handles JSON fallback parsing with proper error handling - all failure paths log and raiseValueErrorto trigger the retry loop.src/synthorg/engine/workspace/semantic_llm.py (2)
56-75: LGTM!The
__init__method properly validates the model string is non-blank and uses lazy import forSemanticAnalysisConfigto avoid circular dependencies. The default config instantiation whenNoneis passed is clean.
243-287: LGTM!The
_read_file_contentsfunction correctly:
- Validates path traversal using
is_relative_to()after resolving both paths- Checks file size before reading to prevent memory exhaustion
- Uses a single
except OSErrorclause per PEP 758 guidelines- Documents the intentional divergence from
_read_sources(byte limit and log level)src/synthorg/engine/workspace/semantic_checks.py (4)
63-69: LGTM!The
_all_name_referencesfunction now correctly filters toLoadcontext only (isinstance(node.ctx, ast.Load)), addressing the previous review concern about over-reporting due to collecting allast.Namenodes.
153-180: LGTM!The
_collect_removed_namesfunction now correctly handles deleted files (lines 165-171) by treating all top-level names from the base file as removed whenmerged_src is None. This addresses the previous review concern about whole-file removals being invisible.
230-237: LGTM!The
_SigInfoNamedTuple now includesrequired_kwonly: frozenset[str], and_compare_signaturescorrectly computes added required keyword-only parameters. The_check_call_compatfunction validates these are provided at call sites. This addresses the previous review concern about signature changes likedef f(x)→def f(x, *, y)being missed.Also applies to: 290-309
213-227: LGTM!The
check_removed_referencesfunction correctly excludes names that are redefined locally in the merged file (line 216:name not in local_defs), preventing false positives from local rebindings likefoo = 1; print(foo).src/synthorg/engine/workspace/git_worktree.py (4)
58-70: LGTM!The
_validate_file_pathfunction provides robust input validation:
- Rejects empty/flag-like strings
- Blocks directory traversal (
..in path components)- Rejects absolute paths
- Uses a conservative character allowlist
This properly validates git path arguments before use.
857-866: LGTM!The pre-filtering logic correctly applies
file_extensionsandmax_fileslimits before calling_get_base_sources, addressing the previous review concern about expensive git show calls not being capped by the config limits.
785-788: LGTM!The
_get_base_sourcesmethod correctly usesasyncio.TaskGroupfor parallelgit showoperations, following the coding guideline to prefer structured concurrency over barecreate_task.
879-887: LGTM!The exception handling correctly:
- Re-raises
asyncio.CancelledErrorto support proper cancellation- Catches all other exceptions, logs with context and
exc_info=True- Returns empty tuple to avoid blocking the merge flow
This graceful degradation is appropriate since semantic analysis is informational and shouldn't prevent successful textual merges.
tests/integration/engine/test_workspace_semantic_integration.py (3)
29-63: LGTM!The
_init_test_repohelper properly initializes a test git repository with required git config and initial commit. The# noqa: S607comments acknowledge Bandit's subprocess security warnings, which are acceptable in tests using controlled inputs.
236-326: LGTM!The
test_duplicate_definition_detectedtest correctly uses disjoint hunks (Agent A adds at TOP, Agent B adds at BOTTOM) separated by a spacer block, ensuring git cleanly merges both branches and produces twoprocess()definitions. This addresses the previous review concern about nondeterministic test behavior.
120-185: LGTM!The
test_renamed_function_detectedtest provides good end-to-end coverage of the removed-reference detection scenario. The assertions correctly verify that:
- First merge succeeds without semantic conflicts
- Second merge succeeds textually but detects semantic conflict
- The conflict description references the removed function name
src/synthorg/engine/workspace/semantic_analyzer.py (4)
35-63: LGTM!The
SemanticAnalyzerprotocol is well-designed with:
@runtime_checkabledecorator enablingisinstance()checks- Keyword-only parameters for clarity
- Clear docstrings documenting arguments and return type
- Appropriate use of
Mappingfor read-onlybase_sources
79-107: LGTM!The
_read_sourcesfunction correctly:
- Resolves the root path once for comparison
- Validates each file path is relative to the resolved root
- Logs traversal attempts at WARNING level
- Catches
OSErrorper PEP 758 guidelines- Logs read errors at DEBUG level (consistent with AST analyzer's lower operational significance vs LLM analyzer)
288-296: LGTM!The deduplication logic correctly uses
(file_path, description)as the key, which is appropriate since the same conflict might be detected by both AST and LLM analyzers. The implementation preserves order while efficiently tracking seen conflicts.
156-207: LGTM!The
AstSemanticAnalyzer.analyzemethod is well-structured:
- Logs the filtered file count at start (addressing previous review comment)
- Uses
asyncio.to_threadfor blocking file I/O- Runs synchronous AST checks via
_run_ast_checks- Properly logs conflicts at WARNING and completion at INFO
| def _file_path_to_module_stem(file_path: str) -> str: | ||
| """Convert a Python file path to a dotted module stem. | ||
|
|
||
| Strips ``.py`` suffix, converts ``__init__.py`` to the parent | ||
| package name, removes common source root prefixes (``src/``, | ||
| ``lib/``), and replaces path separators with dots. | ||
| """ | ||
| stem = file_path.removesuffix(".py") | ||
| if stem.endswith(("/__init__", "\\__init__")): | ||
| sep = "/" if "/" in stem else "\\" | ||
| stem = stem.rsplit(sep, 1)[0] | ||
| for prefix in ("src/", "lib/"): | ||
| if stem.startswith(prefix): | ||
| stem = stem[len(prefix) :] | ||
| break | ||
| return stem.replace("/", ".").replace("\\", ".") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Module stem conversion has limited prefix handling.
The _file_path_to_module_stem function only strips src/ and lib/ prefixes. Projects with different layouts (e.g., packages/, app/) will have module stems that don't match import statements.
This is acceptable for initial implementation but may cause false negatives in non-standard project layouts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workspace/semantic_checks.py` around lines 475 - 490, The
module-stem conversion in _file_path_to_module_stem only removes "src/" and
"lib/" prefixes which misses other common layouts; update this function to
handle additional common prefixes (e.g., "packages/", "app/",
"src/main/python/") by replacing the hard-coded ("src/", "lib/") tuple with a
configurable or extended tuple (e.g., MODULE_ROOT_PREFIXES) and iterate over
that to strip any matching prefix before converting separators and trimming
"__init__"; ensure you keep the existing behavior for Windows separators and the
__init__ handling in _file_path_to_module_stem.
| for path, content in changed_files.items(): | ||
| parts.append(f"\n### {path}\n```python\n{content}\n```\n") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded python fence may misrepresent non-Python files.
The build_review_message function wraps all file contents in ```python fences regardless of actual file type. While file_extensions in config defaults to .py, if extended to other languages (e.g., .pyi, .pyx), the fence language hint could confuse the LLM or syntax highlighting.
Consider deriving the fence language from the file extension:
♻️ Suggested improvement
for path, content in changed_files.items():
- parts.append(f"\n### {path}\n```python\n{content}\n```\n")
+ ext = path.rsplit(".", 1)[-1] if "." in path else ""
+ lang = {"py": "python", "pyi": "python", "pyx": "cython"}.get(ext, "")
+ parts.append(f"\n### {path}\n```{lang}\n{content}\n```\n")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workspace/semantic_llm_prompt.py` around lines 136 - 137,
The loop in build_review_message currently hardcodes a "python" code fence for
every changed_files entry which can mislabel non-.py files; update the loop that
iterates over changed_files (the for path, content in changed_files.items()) to
derive a fence language from the file extension (e.g., ext = path.rsplit(".",
1)[-1] if "." in path else ""), map common extensions to languages (e.g.,
{"py":"python","pyi":"python","pyx":"cython"}), and use that mapped lang when
building parts.append(f"\n### {path}\n```{lang}\n{content}\n```\n") falling back
to an empty fence if unknown.
| async def _call_with_retry( | ||
| self, | ||
| *, | ||
| workspace: Workspace, | ||
| messages: list[ChatMessage], | ||
| tool_def: ToolDefinition, | ||
| comp_config: CompletionConfig, | ||
| max_retries: int = 2, | ||
| ) -> tuple[MergeConflict, ...]: | ||
| """Call LLM with retry on parse failure. | ||
|
|
||
| Args: | ||
| workspace: Workspace for logging context. | ||
| messages: Chat messages to send. | ||
| tool_def: Tool definition for structured output. | ||
| comp_config: Completion configuration. | ||
| max_retries: Maximum retry attempts on parse failure. | ||
|
|
||
| Returns: | ||
| Parsed conflicts, or empty tuple on exhaustion/error. | ||
| """ | ||
| for attempt in range(1 + max_retries): | ||
| try: | ||
| response = await self._provider.complete( | ||
| messages, | ||
| self._model, | ||
| tools=[tool_def], | ||
| config=comp_config, | ||
| ) | ||
| conflicts = parse_tool_call_response(response) | ||
| except ValueError: | ||
| if attempt < max_retries: | ||
| logger.debug( | ||
| WORKSPACE_SEMANTIC_ANALYSIS_FAILED, | ||
| workspace_id=workspace.workspace_id, | ||
| analyzer="llm", | ||
| attempt=attempt, | ||
| reason="parse_error", | ||
| ) | ||
| continue | ||
| logger.warning( | ||
| WORKSPACE_SEMANTIC_ANALYSIS_FAILED, | ||
| workspace_id=workspace.workspace_id, | ||
| analyzer="llm", | ||
| reason="parse_exhausted", | ||
| ) | ||
| return () | ||
| except asyncio.CancelledError: | ||
| raise | ||
| except Exception: | ||
| logger.warning( | ||
| WORKSPACE_SEMANTIC_ANALYSIS_FAILED, | ||
| workspace_id=workspace.workspace_id, | ||
| analyzer="llm", | ||
| reason="provider_error", | ||
| exc_info=True, | ||
| ) | ||
| return () | ||
| else: | ||
| logger.info( | ||
| WORKSPACE_SEMANTIC_ANALYSIS_COMPLETE, | ||
| workspace_id=workspace.workspace_id, | ||
| analyzer="llm", | ||
| conflicts=len(conflicts), | ||
| attempt=attempt, | ||
| ) | ||
| return conflicts | ||
|
|
||
| return () # pragma: no cover |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Retry logic sends identical prompts on parse failure.
The _call_with_retry method retries with the exact same messages and tool_def on ValueError. If the LLM produces deterministic output (e.g., temperature=0 or provider-level caching), retries will fail identically. Consider either:
- Documenting this relies on LLM stochasticity, or
- Adding a hint to the retry prompt indicating the previous response was malformed.
This is acceptable if llm_temperature defaults to a non-zero value, but worth documenting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/workspace/semantic_llm.py` around lines 172 - 240, The
retry loop in _call_with_retry currently resends identical messages/tool_def on
ValueError; change it to append a short system/user hint on retry attempts to
tell the model the previous response was malformed (e.g., add a ChatMessage like
"Previous response could not be parsed; please return valid JSON using the
specified tool schema." when attempt>0) before calling self._provider.complete,
or alternatively document in the function docstring that successful retries
depend on non-zero llm_temperature in comp_config; update the call site that
constructs messages (used by _call_with_retry) so the hint is injected only for
retry attempts and reference _provider.complete, parse_tool_call_response, and
comp_config in the change.
🤖 I have created a release *beep* *boop* --- #MAJOR CHANGES; We got a somewhat working webui :) ## [0.5.0](v0.4.9...v0.5.0) (2026-03-30) ### Features * add analytics trends and budget forecast API endpoints ([#798](#798)) ([16b61f5](16b61f5)) * add department policies to default templates ([#852](#852)) ([7a41548](7a41548)) * add remaining activity event types (task_started, tool_used, delegation, cost_incurred) ([#832](#832)) ([4252fac](4252fac)) * agent performance, activity, and history API endpoints ([#811](#811)) ([9b75c1d](9b75c1d)) * Agent Profiles and Detail pages (biography, career, performance) ([#874](#874)) ([62d7880](62d7880)) * app shell, Storybook, and CI/CD pipeline ([#819](#819)) ([d4dde90](d4dde90)) * Approvals page with risk grouping, urgency indicators, batch actions ([#889](#889)) ([4e9673d](4e9673d)) * Budget Panel page (P&L dashboard, breakdown charts, forecast) ([#890](#890)) ([b63b0f1](b63b0f1)) * build infrastructure layer (API client, auth, WebSocket) ([#815](#815)) ([9f01d3e](9f01d3e)) * CLI global options infrastructure, UI modes, exit codes, env vars ([#891](#891)) ([fef4fc5](fef4fc5)) * CodeMirror editor and theme preferences toggle ([#905](#905), [#807](#807)) ([#909](#909)) ([41fbedc](41fbedc)) * Company page (department/agent management) ([#888](#888)) ([cfb88b0](cfb88b0)) * comprehensive hint coverage across all CLI commands ([#900](#900)) ([937974e](937974e)) * config system extensions, per-command flags for init/start/stop/status/logs ([#895](#895)) ([32f83fe](32f83fe)) * configurable currency system replacing hardcoded USD ([#854](#854)) ([b372551](b372551)) * Dashboard page (metric cards, activity feed, budget burn) ([#861](#861)) ([7d519d5](7d519d5)) * department health, provider status, and activity feed endpoints ([#818](#818)) ([6d5f196](6d5f196)) * design tokens and core UI components ([#833](#833)) ([ed887f2](ed887f2)) * extend approval, meeting, and budget API responses ([#834](#834)) ([31472bf](31472bf)) * frontend polish -- real-time UX, accessibility, responsive, performance ([#790](#790), [#792](#792), [#791](#791), [#793](#793)) ([#917](#917)) ([f04a537](f04a537)) * implement human roles and access control levels ([#856](#856)) ([d6d8a06](d6d8a06)) * implement semantic conflict detection in workspace merge ([#860](#860)) ([d97283b](d97283b)) * interaction components and animation patterns ([#853](#853)) ([82d4b01](82d4b01)) * Login page + first-run bootstrap + Company page ([#789](#789), [#888](#888)) ([#896](#896)) ([8758e8d](8758e8d)) * Meetings page with timeline viz, token bars, contribution formatting ([#788](#788)) ([#904](#904)) ([b207f46](b207f46)) * Messages page with threading, channel badges, sender indicators ([#787](#787)) ([#903](#903)) ([28293ad](28293ad)) * Org Chart force-directed view and drag-drop reassignment ([#872](#872), [#873](#873)) ([#912](#912)) ([a68a938](a68a938)) * Org Chart page (living nodes, status, CRUD, department health) ([#870](#870)) ([0acbdae](0acbdae)) * per-command flags for remaining commands, auto-behavior wiring, help/discoverability ([#897](#897)) ([3f7afa2](3f7afa2)) * Providers page with backend rework -- health, CRUD, subscription auth ([#893](#893)) ([9f8dd98](9f8dd98)) * scaffold React + Vite + TypeScript + Tailwind project ([#799](#799)) ([bd151aa](bd151aa)) * Settings page with search, dependency indicators, grouped rendering ([#784](#784)) ([#902](#902)) ([a7b9870](a7b9870)) * Setup Wizard rebuild with template comparison, cost estimator, theme customization ([#879](#879)) ([ae8b50b](ae8b50b)) * setup wizard UX -- template filters, card metadata, provider form reuse ([#910](#910)) ([7f04676](7f04676)) * setup wizard UX overhaul -- mode choice, step reorder, provider fixes ([#907](#907)) ([ee964c4](ee964c4)) * structured ModelRequirement in template agent configs ([#795](#795)) ([7433548](7433548)) * Task Board page (rich Kanban, filtering, dependency viz) ([#871](#871)) ([04a19b0](04a19b0)) ### Bug Fixes * align frontend types with backend and debounce WS refetches ([#916](#916)) ([134c11b](134c11b)) * auto-cleanup targets newly pulled images instead of old ones ([#884](#884)) ([50e6591](50e6591)) * correct wipe backup-skip flow and harden error handling ([#808](#808)) ([c05860f](c05860f)) * improve provider setup in wizard, subscription auth, dashboard bugs ([#914](#914)) ([87bf8e6](87bf8e6)) * improve update channel detection and add config get command ([#814](#814)) ([6b137f0](6b137f0)) * resolve all ESLint warnings, add zero-warnings enforcement ([#899](#899)) ([079b46a](079b46a)) * subscription auth uses api_key, base URL optional for cloud providers ([#915](#915)) ([f0098dd](f0098dd)) ### Refactoring * semantic analyzer cleanup -- shared filtering, concurrency, extraction ([#908](#908)) ([81372bf](81372bf)) ### Documentation * brand identity and UX design system from [#765](#765) exploration ([#804](#804)) ([389a9f4](389a9f4)) * page structure and information architecture for v0.5.0 dashboard ([#809](#809)) ([f8d6d4a](f8d6d4a)) * write UX design guidelines with WCAG-verified color system ([#816](#816)) ([4a4594e](4a4594e)) ### Tests * add unit tests for agent hooks and page components ([#875](#875)) ([#901](#901)) ([1d81546](1d81546)) ### CI/CD * bump actions/deploy-pages from 4.0.5 to 5.0.0 in the major group ([#831](#831)) ([01c19de](01c19de)) * bump astral-sh/setup-uv from 7.6.0 to 8.0.0 in /.github/actions/setup-python-uv in the all group ([#920](#920)) ([5f6ba54](5f6ba54)) * bump codecov/codecov-action from 5.5.3 to 6.0.0 in the major group ([#868](#868)) ([f22a181](f22a181)) * bump github/codeql-action from 4.34.1 to 4.35.0 in the all group ([#883](#883)) ([87a4890](87a4890)) * bump sigstore/cosign-installer from 4.1.0 to 4.1.1 in the minor-and-patch group ([#830](#830)) ([7a69050](7a69050)) * bump the all group with 3 updates ([#923](#923)) ([ff27c8e](ff27c8e)) * bump wrangler from 4.76.0 to 4.77.0 in /.github in the minor-and-patch group ([#822](#822)) ([07d43eb](07d43eb)) * bump wrangler from 4.77.0 to 4.78.0 in /.github in the all group ([#882](#882)) ([f84118d](f84118d)) ### Maintenance * add design system enforcement hook and component inventory ([#846](#846)) ([15abc43](15abc43)) * add dev-only auth bypass for frontend testing ([#885](#885)) ([6cdcd8a](6cdcd8a)) * add pre-push rebase check hook ([#855](#855)) ([b637a04](b637a04)) * backend hardening -- eviction/size-caps and model validation ([#911](#911)) ([81253d9](81253d9)) * bump axios from 1.13.6 to 1.14.0 in /web in the all group across 1 directory ([#922](#922)) ([b1b0232](b1b0232)) * bump brace-expansion from 5.0.4 to 5.0.5 in /web ([#862](#862)) ([ba4a565](ba4a565)) * bump eslint-plugin-react-refresh from 0.4.26 to 0.5.2 in /web ([#801](#801)) ([7574bb5](7574bb5)) * bump faker from 40.11.0 to 40.11.1 in the minor-and-patch group ([#803](#803)) ([14d322e](14d322e)) * bump https://github.com/astral-sh/ruff-pre-commit from v0.15.7 to 0.15.8 ([#864](#864)) ([f52901e](f52901e)) * bump nginxinc/nginx-unprivileged from `6582a34` to `f99cc61` in /docker/web in the all group ([#919](#919)) ([df85e4f](df85e4f)) * bump nginxinc/nginx-unprivileged from `ccbac1a` to `6582a34` in /docker/web ([#800](#800)) ([f4e9450](f4e9450)) * bump node from `44bcbf4` to `71be405` in /docker/sandbox ([#827](#827)) ([91bec67](91bec67)) * bump node from `5209bca` to `cf38e1f` in /docker/web ([#863](#863)) ([66d6043](66d6043)) * bump picomatch in /site ([#842](#842)) ([5f20bcc](5f20bcc)) * bump recharts 2->3 and @types/node 22->25 in /web ([#802](#802)) ([a908800](a908800)) * Bump requests from 2.32.5 to 2.33.0 ([#843](#843)) ([41daf69](41daf69)) * bump smol-toml from 1.6.0 to 1.6.1 in /site ([#826](#826)) ([3e5dbe4](3e5dbe4)) * bump the all group with 3 updates ([#921](#921)) ([7bace0b](7bace0b)) * bump the minor-and-patch group across 1 directory with 2 updates ([#829](#829)) ([93e611f](93e611f)) * bump the minor-and-patch group across 1 directory with 3 updates ([#841](#841)) ([7010c8e](7010c8e)) * bump the minor-and-patch group across 1 directory with 3 updates ([#869](#869)) ([548cee5](548cee5)) * bump the minor-and-patch group in /site with 2 updates ([#865](#865)) ([9558101](9558101)) * bump the minor-and-patch group with 2 updates ([#867](#867)) ([4830706](4830706)) * consolidate Dependabot groups to 1 PR per ecosystem ([06d2556](06d2556)) * consolidate Dependabot groups to 1 PR per ecosystem ([#881](#881)) ([06d2556](06d2556)) * improve worktree skill with full dep sync and status enhancements ([#906](#906)) ([772c625](772c625)) * remove Vue remnants and document framework decision ([#851](#851)) ([bf2adf6](bf2adf6)) * update web dependencies and fix brace-expansion CVE ([#880](#880)) ([a7a0ed6](a7a0ed6)) * upgrade to Storybook 10 and TypeScript 6 ([#845](#845)) ([52d95f2](52d95f2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
SemanticAnalyzerprotocol withAstSemanticAnalyzer(deterministic),LlmSemanticAnalyzer(optional deep review), andCompositeSemanticAnalyzer(chains both)PlannerWorktreeStrategyusing merge-base for cross-branch detection, with escalation handling inMergeOrchestrator(HUMAN stops, REVIEW_AGENT continues)Test plan
Related issues
_get_base_sourcesgit show calls with TaskGroupMapping[str, str]for read-onlybase_sourcesparameterCloses #611
🤖 Generated with Claude Code