feat: personality preset discovery API and user-defined preset CRUD#952
feat: personality preset discovery API and user-defined preset CRUD#952
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2026-03-20T08:28:32.845ZApplied to files:
📚 Learning: 2026-03-16T06:24:56.341ZApplied to files:
📚 Learning: 2026-03-19T07:13:44.964ZApplied to files:
📚 Learning: 2026-03-31T14:17:24.182ZApplied to files:
📚 Learning: 2026-03-31T14:17:24.182ZApplied to files:
🔇 Additional comments (1)
WalkthroughAdds a personality preset subsystem: new Pydantic DTOs and Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a personality preset management system, enabling the discovery of built-in presets and CRUD operations for custom user-defined presets. The implementation spans a new API controller, DTOs, a service layer, and a SQLite-backed repository. Feedback highlights several scalability and reliability concerns, specifically the need for repository-level pagination to avoid high memory usage, the resolution of a race condition in the creation logic, and the removal of hardcoded limits in database queries. Improvements to code maintainability through DTO refactoring and more idiomatic use of the Litestar framework were also suggested.
| entries = await service.list_all() | ||
| summaries = tuple(_to_summary(e) for e in entries) | ||
| page, meta = paginate(summaries, offset=offset, limit=limit) |
There was a problem hiding this comment.
The current implementation of list_presets fetches all presets (built-in and custom) into memory before paginating with the paginate() utility. As the number of custom presets grows, this will lead to significant performance degradation and high memory usage. Pagination should be implemented at the repository level using SQL LIMIT and OFFSET clauses to ensure the API scales correctly.
| existing = await self._repo.get(NotBlankStr(key)) | ||
| if existing is not None: | ||
| logger.warning( | ||
| PRESET_CONFLICT, | ||
| preset_name=key, | ||
| reason="already_exists", | ||
| ) | ||
| msg = f"Custom preset {key!r} already exists" | ||
| raise ConflictError(msg) | ||
|
|
||
| validated = _validate_personality_config(config) | ||
| config_json = json.dumps(validated.model_dump(mode="json"), sort_keys=True) | ||
| description = str(config.get("description", "")) | ||
| now = datetime.now(UTC).isoformat() | ||
|
|
||
| await self._repo.save(NotBlankStr(key), config_json, description, now, now) |
There was a problem hiding this comment.
This sequence introduces a race condition (TOCTOU). The check for existence (self._repo.get) followed by self._repo.save is not atomic. Since the repository's save method is implemented as an UPSERT (ON CONFLICT DO UPDATE), two concurrent requests could both pass the existence check and the second would silently overwrite the first. For a 'create' operation, the repository should ideally provide a strict insert method that fails on conflict, allowing the service to catch the error and raise a ConflictError.
| ) -> Response[ApiResponse[PresetDetailResponse]]: | ||
| """Create a custom personality preset.""" | ||
| service = _get_service(state) | ||
| entry = await service.create(data.name, data.to_config_dict()) | ||
| return Response( | ||
| content=ApiResponse[PresetDetailResponse](data=_to_detail(entry)), | ||
| status_code=201, | ||
| ) |
There was a problem hiding this comment.
Wrapping the result in a litestar.Response is redundant here because the @post decorator already defines status_code=201. Returning the ApiResponse directly is more idiomatic for Litestar and allows the framework to handle serialization and status codes automatically.
) -> ApiResponse[PresetDetailResponse]:
"""Create a custom personality preset."""
service = _get_service(state)
entry = await service.create(data.name, data.to_config_dict())
return ApiResponse[PresetDetailResponse](data=_to_detail(entry))| class UpdatePresetRequest(BaseModel): | ||
| """PUT body for updating a custom personality preset.""" | ||
|
|
||
| model_config = ConfigDict(frozen=True, extra="forbid", allow_inf_nan=False) | ||
|
|
||
| traits: tuple[NotBlankStr, ...] = () | ||
| communication_style: NotBlankStr = Field(default="neutral", max_length=100) | ||
| risk_tolerance: RiskTolerance = RiskTolerance.MEDIUM | ||
| creativity: CreativityLevel = CreativityLevel.MEDIUM | ||
| description: str = Field(default="", max_length=500) | ||
| openness: float = Field(default=0.5, ge=0.0, le=1.0) | ||
| conscientiousness: float = Field(default=0.5, ge=0.0, le=1.0) | ||
| extraversion: float = Field(default=0.5, ge=0.0, le=1.0) | ||
| agreeableness: float = Field(default=0.5, ge=0.0, le=1.0) | ||
| stress_response: float = Field(default=0.5, ge=0.0, le=1.0) | ||
| decision_making: DecisionMakingStyle = DecisionMakingStyle.CONSULTATIVE | ||
| collaboration: CollaborationPreference = CollaborationPreference.TEAM | ||
| verbosity: CommunicationVerbosity = CommunicationVerbosity.BALANCED | ||
| conflict_approach: ConflictApproach = ConflictApproach.COLLABORATE |
There was a problem hiding this comment.
UpdatePresetRequest duplicates almost all fields from CreatePresetRequest. This duplication makes the code harder to maintain and increases the risk of the two models diverging accidentally when the personality configuration schema changes. Consider extracting the common personality configuration fields into a shared base class or using inheritance.
| try: | ||
| cursor = await self._db.execute( | ||
| "SELECT name, config_json, description, created_at, " | ||
| "updated_at FROM custom_presets ORDER BY name LIMIT 10000", |
There was a problem hiding this comment.
The hardcoded LIMIT 10000 is a magic number that can lead to silent data truncation if the number of custom presets exceeds this limit. This reinforces the need for a proper paginated API at the repository level rather than a capped 'list all' approach.
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 #952 +/- ##
==========================================
+ Coverage 91.99% 92.04% +0.04%
==========================================
Files 605 611 +6
Lines 32583 32894 +311
Branches 3147 3158 +11
==========================================
+ Hits 29975 30277 +302
- Misses 2071 2079 +8
- Partials 537 538 +1 ☔ 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/api/controllers/personalities.py`:
- Around line 109-116: The get_schema controller method is marked async but only
calls the synchronous PersonalityPresetService.get_schema(); remove the async
keyword from the get_schema declaration so it becomes a normal sync method, keep
the `@get` decorator and return type ApiResponse[dict[str, Any]] unchanged, and
ensure the function body still returns ApiResponse(data=schema) after calling
PersonalityPresetService.get_schema().
In `@src/synthorg/api/dto_personalities.py`:
- Around line 36-49: The PresetDetailResponse DTO widens validation for traits
and communication_style (currently tuple[str, ...] and str) and should use the
domain-level NotBlankStr types to prevent blank values; update
PresetDetailResponse so traits is typed as tuple[NotBlankStr, ...] and
communication_style as NotBlankStr (or NotBlankStr | None if optional) and
import NotBlankStr from core.types to match PersonalityConfig's constraints
(ensure names match the NotBlankStr symbol and preserve existing defaults if
any).
In `@src/synthorg/persistence/sqlite/preset_repo.py`:
- Around line 136-147: The hardcoded SQL "LIMIT 10000" in list_all() can
silently truncate results; update the list_all() method's docstring to
explicitly state the 10,000 row cap (why it's there and that callers may not
receive more than 10k rows), including the exact limit value, or alternatively
make the limit configurable (e.g., a parameter or class constant) and document
that configuration; locate the SQL with "SELECT ... FROM custom_presets ORDER BY
name LIMIT 10000" in list_all() and add the explanatory note referencing that
query and the PRESET_CUSTOM_LISTED/PRESET_CUSTOM_LIST_FAILED logging points.
In `@src/synthorg/templates/preset_service.py`:
- Around line 155-169: Wrap the bare json.loads(config_json) calls used when
building PresetEntry from self._repo.list_all() and the get() path in a small
helper (e.g., _decode_and_validate_preset) that takes preset_name and
config_json, calls json.loads inside a try/except, logs the preset_name and
parse error at WARNING/ERROR, re-validates the resulting dict against
PersonalityConfig (using the existing validator) and on any failure raises a
domain-specific error (not the raw JSONDecodeError/validation exception) so
callers get a controlled error; update the places creating PresetEntry to call
this helper and to pass the preset name so logs contain context.
In `@tests/unit/api/controllers/test_personalities.py`:
- Around line 153-165: In test_create_duplicate_returns_409 capture and assert
the result of the first POST before attempting the duplicate: assign the first
test_client.post call to a variable (e.g., resp1) and assert its status_code
indicates success (e.g., 201 or the expected success code for create) so the
subsequent 409 check validates a true duplicate case; reference the test
function test_create_duplicate_returns_409 and the POST to
"/api/v1/personalities/presets".
- Around line 176-196: Each test that performs a setup POST must capture and
assert the POST succeeded before continuing; e.g., in
test_created_preset_gettable (and likewise in
test_created_preset_appears_in_list, test_update_custom_preset,
test_observer_cannot_update, test_update_with_invalid_config_returns_400) assign
the result of test_client.post(...) to a variable, assert the status_code is the
expected success code (e.g., 201 or 200 used by your API), and optionally assert
the returned JSON contains expected keys (like data.id or data.name) so
downstream GET/update assertions operate on a verified resource; update the POST
calls in functions test_created_preset_gettable,
test_created_preset_appears_in_list, test_update_custom_preset,
test_observer_cannot_update, and test_update_with_invalid_config_returns_400
accordingly.
In `@tests/unit/persistence/sqlite/test_preset_repo.py`:
- Around line 68-92: The test test_save_upsert_updates_existing should also
assert that created_at is preserved after the upsert: when you call repo.save
the second time (repo.save("my_preset", updated_json, ...)), fetch the row via
result = await repo.get("my_preset") and unpack created_at (e.g., cfg_json,
description, created_at, updated_at = result), then assert created_at ==
"2026-03-31T00:00:00+00:00" to ensure the SQLite upsert did not alter
created_at.
In `@tests/unit/persistence/test_protocol.py`:
- Around line 336-358: Add an explicit protocol conformance assertion for the
new _FakePersonalityPresetRepository: in the same test module where other fake
repos are checked, instantiate _FakePersonalityPresetRepository and assert it
conforms to the repository protocol used in this file (e.g. assert
isinstance(_FakePersonalityPresetRepository(), PersonalityPresetRepository) or
the equivalent protocol check used elsewhere in the test suite) so future
signature regressions on _FakePersonalityPresetRepository are caught.
In `@tests/unit/templates/test_preset_service_properties.py`:
- Around line 32-59: The traits generator in _valid_config currently uses
st.tuples(st.text(...)) producing only single-element tuples; update the
"traits" entry in _valid_config to produce variable-length trait tuples by using
st.lists(st.text(alphabet=string.ascii_lowercase, min_size=1, max_size=10),
min_size=1, max_size=5).map(tuple) (or similar bounds) so tests using
_valid_config exercise multiple traits per preset and improve coverage for the
_valid_config strategy and any tests that consume it.
🪄 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: 46416bb4-a3f8-482d-a18a-4861dbbd9766
📒 Files selected for processing (18)
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/api/dto_personalities.pysrc/synthorg/observability/events/preset.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/templates/preset_service.pytests/unit/api/controllers/test_personalities.pytests/unit/api/fakes.pytests/unit/observability/test_events.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/persistence/test_protocol.pytests/unit/templates/test_preset_service.pytests/unit/templates/test_preset_service_properties.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: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations
Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
PEP 758 except syntax: useexcept A, B:(no parentheses) -- ruff enforces this on Python 3.14
Line length: 88 characters (ruff enforced)
Files:
tests/unit/persistence/sqlite/test_migrations.pysrc/synthorg/api/controllers/__init__.pytests/unit/observability/test_events.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/protocol.pytests/unit/api/fakes.pytests/unit/templates/test_preset_service_properties.pytests/unit/persistence/test_protocol.pysrc/synthorg/persistence/preset_repository.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/templates/test_preset_service.pysrc/synthorg/observability/events/preset.pysrc/synthorg/api/controllers/personalities.pytests/unit/api/controllers/test_personalities.pysrc/synthorg/templates/preset_service.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/api/dto_personalities.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests:asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Parametrize: Prefer@pytest.mark.parametrizefor testing similar cases
Tests must usetest-provider,test-small-001, etc. instead of real vendor names
Property-based testing in Python: use Hypothesis (@given+@settings); run withHYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties(dev profile: 1000 examples)
Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)-- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/persistence/sqlite/test_migrations.pytests/unit/observability/test_events.pytests/unit/api/fakes.pytests/unit/templates/test_preset_service_properties.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/templates/test_preset_service.pytests/unit/api/controllers/test_personalities.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict)
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code)
Variable name for logger: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Structured kwargs for logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG logging for object creation, internal flow, entry/exit of key...
Files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/observability/events/preset.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/templates/preset_service.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/api/dto_personalities.py
🧠 Learnings (30)
📚 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/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/templates/preset_service.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/api/controllers/__init__.pysrc/synthorg/persistence/protocol.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
src/synthorg/api/controllers/__init__.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
tests/unit/observability/test_events.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
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> import EVENT_CONSTANT`
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.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:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/protocol.pytests/unit/api/fakes.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/persistence/test_protocol.pysrc/synthorg/persistence/preset_repository.pytests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/templates/preset_service.pysrc/synthorg/persistence/sqlite/preset_repo.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: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/persistence/test_protocol.pysrc/synthorg/persistence/preset_repository.pytests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/sqlite/preset_repo.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/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
src/synthorg/persistence/protocol.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/persistence/protocol.pysrc/synthorg/templates/preset_service.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to tests/**/*.py : Property-based testing in Python: use Hypothesis (`given` + `settings`); run with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties` (dev profile: 1000 examples)
Applied to files:
tests/unit/templates/test_preset_service_properties.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: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/templates/test_preset_service_properties.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 tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run property tests with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties`
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
tests/unit/templates/test_preset_service_properties.pytests/unit/templates/test_preset_service.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/observability/events/preset.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/preset.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.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/api/dto_personalities.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/api/dto_personalities.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/api/dto_personalities.py
🔇 Additional comments (23)
tests/unit/persistence/sqlite/test_migrations.py (1)
32-32: Expected schema table set update is correct.Including
custom_presetsin_EXPECTED_TABLEScorrectly tracks the new migration surface.tests/unit/observability/test_events.py (1)
211-211: Domain discovery expectation is correctly extended.Adding
"preset"keeps the module discovery test aligned with the new observability domain.src/synthorg/persistence/sqlite/schema.sql (1)
298-305:custom_presetstable shape matches the new persistence contract.The new table definition is consistent with the repository access pattern (name-keyed preset records).
src/synthorg/persistence/protocol.py (1)
15-17: Persistence protocol extension is clean and complete.The new
custom_presetsrepository accessor is correctly typed and documented.Also applies to: 62-63, 187-190
tests/unit/api/fakes.py (1)
534-560: Fake preset repository and backend wiring are consistent.This test double now mirrors the new persistence surface, including deterministic
list_all()ordering.Also applies to: 569-570, 673-675
src/synthorg/persistence/sqlite/backend.py (1)
49-51: SQLite backend integration forcustom_presetsis correctly implemented.Initialization, teardown, repository construction, and guarded property access all align with existing backend patterns.
Also applies to: 105-106, 126-127, 197-198, 452-460
src/synthorg/api/controllers/__init__.py (1)
21-23: Controller registration/export update is complete.
PersonalityPresetControlleris now correctly imported, mounted, and exported from the controller package.Also applies to: 57-58, 81-82
tests/unit/persistence/test_protocol.py (1)
445-447:custom_presetsaccessor wiring in_FakeBackendlooks good.The backend stub now exposes the new repository surface expected by
PersistenceBackend.tests/unit/templates/test_preset_service_properties.py (3)
1-13: LGTM!Imports and module setup are correct. The test file properly imports Hypothesis strategies and the service under test.
65-88: LGTM!The async round-trip property test correctly:
- Uses
assume()to skip builtin preset names- Creates a fresh fake repository per test
- Validates the round-trip invariant (fetched config matches created config)
- Checks source tagging is correct
90-95: LGTM!The idempotency property test correctly verifies that normalizing an already-normalized name produces the same result.
src/synthorg/persistence/preset_repository.py (1)
1-94: LGTM!The protocol is well-designed:
@runtime_checkableenables isinstance checks for protocol compliance- Uses
NotBlankStrfor name parameters per coding guidelines- Clear docstrings document the contract including error conditions
- Upsert semantics for
save()are explicitly documented- No logging needed for pure protocol definitions
src/synthorg/observability/events/preset.py (1)
1-24: LGTM!Event constants follow the
preset.<scope>.<action>convention and provide comprehensive coverage for both repository-level operations (preset.custom.*) and service-layer events (preset.*). The separation between success and failure events enables proper observability. As per coding guidelines, pure constants modules do not require logging setup.tests/unit/persistence/sqlite/test_preset_repo.py (1)
1-209: LGTM overall!Comprehensive test coverage for the SQLite repository:
- CRUD operations verified with appropriate assertions
- Error path coverage for missing table scenarios
- Proper use of fixtures for migrated vs unmigrated databases
- Async test patterns followed correctly
tests/unit/templates/test_preset_service.py (1)
1-248: LGTM!Comprehensive test coverage for
PersonalityPresetService:
- Discovery tests verify builtin/custom merging, sorting, and source tagging
- CRUD tests cover happy paths and error conditions (conflicts, validation, not found)
- Tests verify important behaviors like name normalization and
created_atpreservation- Schema test validates JSON schema structure
- Proper use of fixtures and async patterns
src/synthorg/persistence/sqlite/preset_repo.py (2)
59-80: Upsert SQL correctly preservescreated_aton conflict.The
ON CONFLICTclause only updatesconfig_json,description, andupdated_at, which meanscreated_atis preserved from the original insert. This matches the expected behavior documented in the service layer whereupdate()preserves the original creation timestamp.
1-198: LGTM overall!The SQLite repository implementation is well-structured:
- Proper async patterns with
aiosqlite- Comprehensive error handling wrapping exceptions in
QueryError- Structured logging with domain-specific event constants
- Upsert semantics correctly preserve
created_at- Clear docstrings following Google style
src/synthorg/api/controllers/personalities.py (2)
39-61: Consider centralizing default values to avoid drift.The
_to_detailfunction defines default values (e.g.,"neutral","medium",0.5) inline. IfPersonalityConfigdefaults change, these could drift out of sync. However, validated presets should always have these fields, so defaults here are defensive fallbacks for edge cases.
1-166: LGTM overall!The controller is well-structured:
- Clear separation between discovery (Issue
#755) and CRUD (Issue#756) endpoints- Proper use of guards for read/write access control
- Correct HTTP status codes (201 for create, 200 for others)
- Helper functions keep handlers clean
- DTOs properly convert between internal and API representations
tests/unit/api/controllers/test_personalities.py (1)
1-312: LGTM overall!Comprehensive controller test coverage:
- All endpoints tested for happy paths and error conditions
- Role-based access control verified (observer can read, cannot write)
- Proper HTTP status code assertions (200, 201, 400, 403, 404, 409)
- End-to-end integration tests validate controller ↔ service ↔ repository flow
src/synthorg/templates/preset_service.py (1)
72-131: Nice validation boundary.Centralizing preset-name normalization and
PersonalityConfigvalidation keeps the controller surface thin and makes the write paths consistent.src/synthorg/api/dto_personalities.py (2)
78-126: The request DTOs line up cleanly withPersonalityConfig.The defaults,
extra="forbid", andallow_inf_nan=Falsehere should make POST/PUT payloads round-trip into service validation without extra mapping glue.
65-72: VerifyPresetSchemaResponseis instantiated with the alias.With Pydantic v2 field aliases,
schema_definitionis not accepted as input unless the model opts into name-based validation. If any caller doesPresetSchemaResponse(schema_definition=...), this endpoint will fail at runtime. Either addvalidate_by_name=Trueto the model config or ensure all call sites useschema=instead.
| @get( | ||
| "/schema", | ||
| guards=[require_read_access], | ||
| ) | ||
| async def get_schema(self) -> ApiResponse[dict[str, Any]]: | ||
| """Return the PersonalityConfig JSON schema.""" | ||
| schema = PersonalityPresetService.get_schema() | ||
| return ApiResponse[dict[str, Any]](data=schema) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Schema endpoint doesn't need async since get_schema() is synchronous.
The get_schema method is marked async but only calls PersonalityPresetService.get_schema(), which is a synchronous class method. While Litestar handles this correctly, removing async would be more accurate.
♻️ Make method synchronous
`@get`(
"/schema",
guards=[require_read_access],
)
- async def get_schema(self) -> ApiResponse[dict[str, Any]]:
+ def get_schema(self) -> ApiResponse[dict[str, Any]]:
"""Return the PersonalityConfig JSON schema."""
schema = PersonalityPresetService.get_schema()
return ApiResponse[dict[str, Any]](data=schema)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @get( | |
| "/schema", | |
| guards=[require_read_access], | |
| ) | |
| async def get_schema(self) -> ApiResponse[dict[str, Any]]: | |
| """Return the PersonalityConfig JSON schema.""" | |
| schema = PersonalityPresetService.get_schema() | |
| return ApiResponse[dict[str, Any]](data=schema) | |
| `@get`( | |
| "/schema", | |
| guards=[require_read_access], | |
| ) | |
| def get_schema(self) -> ApiResponse[dict[str, Any]]: | |
| """Return the PersonalityConfig JSON schema.""" | |
| schema = PersonalityPresetService.get_schema() | |
| return ApiResponse[dict[str, Any]](data=schema) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/personalities.py` around lines 109 - 116, The
get_schema controller method is marked async but only calls the synchronous
PersonalityPresetService.get_schema(); remove the async keyword from the
get_schema declaration so it becomes a normal sync method, keep the `@get`
decorator and return type ApiResponse[dict[str, Any]] unchanged, and ensure the
function body still returns ApiResponse(data=schema) after calling
PersonalityPresetService.get_schema().
| for ( | ||
| name, | ||
| config_json, | ||
| description, | ||
| created_at, | ||
| updated_at, | ||
| ) in await self._repo.list_all(): | ||
| entries[name] = PresetEntry( | ||
| name=NotBlankStr(name), | ||
| source="custom", | ||
| config=json.loads(config_json), | ||
| description=description, | ||
| created_at=created_at, | ||
| updated_at=updated_at, | ||
| ) |
There was a problem hiding this comment.
Guard custom-preset decoding on the read path.
Lines 165 and 199 do a bare json.loads(config_json) and then trust the result. One malformed or schema-drifted row will bubble a raw parser/validation exception out of list_all() / get() with no preset context and can turn discovery into a 500. Decode via a helper that logs preset_name, re-validates against PersonalityConfig, and raises a domain error instead of leaking raw decode failures.
As per coding guidelines, src/synthorg/**/*.py: Errors: handle explicitly, never silently swallow. All error paths must log at WARNING or ERROR with context before raising.
Also applies to: 193-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/templates/preset_service.py` around lines 155 - 169, Wrap the
bare json.loads(config_json) calls used when building PresetEntry from
self._repo.list_all() and the get() path in a small helper (e.g.,
_decode_and_validate_preset) that takes preset_name and config_json, calls
json.loads inside a try/except, logs the preset_name and parse error at
WARNING/ERROR, re-validates the resulting dict against PersonalityConfig (using
the existing validator) and on any failure raises a domain-specific error (not
the raw JSONDecodeError/validation exception) so callers get a controlled error;
update the places creating PresetEntry to call this helper and to pass the
preset name so logs contain context.
| existing = await self._repo.get(NotBlankStr(key)) | ||
| if existing is not None: | ||
| logger.warning( | ||
| PRESET_CONFLICT, | ||
| preset_name=key, | ||
| reason="already_exists", | ||
| ) | ||
| msg = f"Custom preset {key!r} already exists" | ||
| raise ConflictError(msg) | ||
|
|
||
| validated = _validate_personality_config(config) | ||
| config_json = json.dumps(validated.model_dump(mode="json"), sort_keys=True) | ||
| description = str(config.get("description", "")) | ||
| now = datetime.now(UTC).isoformat() | ||
|
|
||
| await self._repo.save(NotBlankStr(key), config_json, description, now, now) |
There was a problem hiding this comment.
create() and update() are not atomic with the repository upsert.
The get() checks at Lines 244-245 and 301-302 are TOCTOU because src/synthorg/persistence/sqlite/preset_repo.py:39-80 persists via INSERT ... ON CONFLICT DO UPDATE. Two concurrent creates can both pass the existence check and the later save will overwrite the first instead of returning 409; an update racing a delete can also recreate a preset. This needs separate atomic repository operations for create-vs-update semantics.
Also applies to: 301-323
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/synthorg/api/controllers/personalities.py (1)
100-107: 🧹 Nitpick | 🔵 TrivialMinor:
get_schemacould be synchronous.The method is marked
asyncbut only calls the synchronousPersonalityPresetService.get_schema(). While Litestar handles this correctly, removingasyncwould be more accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/personalities.py` around lines 100 - 107, The get_schema handler is declared async but only calls the synchronous PersonalityPresetService.get_schema(); change it to a regular synchronous function by removing async and using def get_schema(self) -> ApiResponse[dict[str, Any]] so it returns ApiResponse(data=schema) directly; keep the `@get` decorator and require_read_access guard and ensure imports/types (ApiResponse, Any) remain valid for the synchronous signature.tests/unit/api/controllers/test_personalities.py (2)
177-197: 🧹 Nitpick | 🔵 TrivialAssert setup POST succeeded before testing downstream behavior.
Multiple tests call
test_client.post()to set up preconditions without verifying success. If the setup POST fails (e.g., due to a regression), the subsequent GET assertions could pass incorrectly.♻️ Add assertions for setup operations
def test_created_preset_appears_in_list(self, test_client: TestClient[Any]) -> None: body = _make_valid_preset_body(name="listed_preset") - test_client.post( + create_resp = test_client.post( "/api/v1/personalities/presets", json=body, headers=make_auth_headers("ceo"), ) + assert create_resp.status_code == 201 resp = test_client.get("/api/v1/personalities/presets?limit=200") names = [p["name"] for p in resp.json()["data"]] assert "listed_preset" in names def test_created_preset_gettable(self, test_client: TestClient[Any]) -> None: body = _make_valid_preset_body(name="gettable_preset") - test_client.post( + create_resp = test_client.post( "/api/v1/personalities/presets", json=body, headers=make_auth_headers("ceo"), ) + assert create_resp.status_code == 201 resp = test_client.get("/api/v1/personalities/presets/gettable_preset")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/controllers/test_personalities.py` around lines 177 - 197, The setup POST calls in test_created_preset_appears_in_list and test_created_preset_gettable currently aren't validated; update the tests to capture the response from test_client.post("/api/v1/personalities/presets", json=body, headers=make_auth_headers("ceo")) and assert the POST succeeded (e.g., resp.status_code == 201 or at least resp.status_code in (200,201)) and optionally that resp.json()["data"]["name"] == body["name"] before performing the GET assertions so failures in test_client.post (in test_client.post(...) calls) fail fast.
241-272: 🧹 Nitpick | 🔵 TrivialAssert setup operations in observer and invalid config update tests.
Both
test_observer_cannot_update(lines 243-247) andtest_update_with_invalid_config_returns_400(lines 260-264) should assert the setup POST succeeded.♻️ Add assertions for setup operations
def test_observer_cannot_update(self, test_client: TestClient[Any]) -> None: body = _make_valid_preset_body(name="obs_update_test") - test_client.post( + create_resp = test_client.post( "/api/v1/personalities/presets", json=body, headers=make_auth_headers("ceo"), ) + assert create_resp.status_code == 201 update_body = {k: v for k, v in body.items() if k != "name"} resp = test_client.put( def test_update_with_invalid_config_returns_400( self, test_client: TestClient[Any] ) -> None: body = _make_valid_preset_body(name="invalid_update") - test_client.post( + create_resp = test_client.post( "/api/v1/personalities/presets", json=body, headers=make_auth_headers("ceo"), ) + assert create_resp.status_code == 201 update_body = {k: v for k, v in body.items() if k != "name"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/controllers/test_personalities.py` around lines 241 - 272, In both test_observer_cannot_update and test_update_with_invalid_config_returns_400, assert the initial POST setup succeeded by checking the response status code (e.g., assert resp.status_code == 201 or == 200 as appropriate) after calling test_client.post("/api/v1/personalities/presets", json=body, headers=make_auth_headers("ceo")); place the assertion immediately after that POST in each test (these tests and helper _make_valid_preset_body identify where to add the checks) so the subsequent PUT assertions run only if the setup succeeded.
🤖 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/persistence/sqlite/preset_repo.py`:
- Around line 149-181: In delete(self, name: NotBlankStr) replace the bare await
self._db.execute(...) with the same async context manager pattern used in
get/list_all/count: use "async with self._db.execute(... ) as cursor:" so the
cursor is properly closed, keep the commit after the context manager, preserve
the existing sqlite3.Error exception handling and message
(PRESET_CUSTOM_DELETE_FAILED) and compute deleted from cursor.rowcount > 0
before logging PRESET_CUSTOM_DELETED; ensure variable names (delete, cursor,
name, PRESET_CUSTOM_DELETE_FAILED, PRESET_CUSTOM_DELETED) remain unchanged.
In `@src/synthorg/templates/preset_service.py`:
- Around line 252-308: The create() (and similarly update()) flow performs a
non-atomic existence check via self._repo.get(...) followed by a save(...) which
introduces a TOCTOU race condition; add a short code comment above the check in
create() (and mirror in update()) stating this known limitation (concurrent
create/update may race and overwrite), mention it's acceptable for the current
MVP with low concurrency, and reference the functions/fields involved
(_repo.get, _repo.save, create, update, PresetEntry) so future maintainers can
consider adding transactional/locking semantics later.
In `@tests/unit/api/controllers/test_personalities.py`:
- Around line 202-219: The test setup POST in test_update_custom_preset does not
assert the creation succeeded; call the POST response into a variable and assert
its status_code (e.g., == 201 or expected) and/or check
resp.json()["data"]["name"] == "updatable" before proceeding to the PUT; update
the test function test_update_custom_preset to capture the response of the POST
to "/api/v1/personalities/presets" (using _make_valid_preset_body) and add the
minimal assert(s) to verify the preset was created so the subsequent call to the
update endpoint and the assertions on resp.json()["data"]["openness"] are valid.
- Around line 277-292: In test_delete_custom_preset capture the setup POST
response (e.g., assign the result of test_client.post(...) to post_resp) and
assert the create succeeded before deleting — for example assert
post_resp.status_code == 201 (or the expected success code for create) so the
preset existence is verified prior to calling personalities preset delete in
test_delete_custom_preset.
---
Duplicate comments:
In `@src/synthorg/api/controllers/personalities.py`:
- Around line 100-107: The get_schema handler is declared async but only calls
the synchronous PersonalityPresetService.get_schema(); change it to a regular
synchronous function by removing async and using def get_schema(self) ->
ApiResponse[dict[str, Any]] so it returns ApiResponse(data=schema) directly;
keep the `@get` decorator and require_read_access guard and ensure imports/types
(ApiResponse, Any) remain valid for the synchronous signature.
In `@tests/unit/api/controllers/test_personalities.py`:
- Around line 177-197: The setup POST calls in
test_created_preset_appears_in_list and test_created_preset_gettable currently
aren't validated; update the tests to capture the response from
test_client.post("/api/v1/personalities/presets", json=body,
headers=make_auth_headers("ceo")) and assert the POST succeeded (e.g.,
resp.status_code == 201 or at least resp.status_code in (200,201)) and
optionally that resp.json()["data"]["name"] == body["name"] before performing
the GET assertions so failures in test_client.post (in test_client.post(...)
calls) fail fast.
- Around line 241-272: In both test_observer_cannot_update and
test_update_with_invalid_config_returns_400, assert the initial POST setup
succeeded by checking the response status code (e.g., assert resp.status_code ==
201 or == 200 as appropriate) after calling
test_client.post("/api/v1/personalities/presets", json=body,
headers=make_auth_headers("ceo")); place the assertion immediately after that
POST in each test (these tests and helper _make_valid_preset_body identify where
to add the checks) so the subsequent PUT assertions run only if the setup
succeeded.
🪄 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: 2d95d483-d3c4-4599-bffa-2ea7ff5c95a9
📒 Files selected for processing (15)
CLAUDE.mddocs/design/memory.mddocs/design/operations.mddocs/design/organization.mdsrc/synthorg/api/controllers/personalities.pysrc/synthorg/api/dto_personalities.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/templates/preset_service.pytests/unit/api/controllers/test_personalities.pytests/unit/api/fakes.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/persistence/test_protocol.pytests/unit/templates/test_preset_service.pytests/unit/templates/test_preset_service_properties.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations
Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
PEP 758 except syntax: useexcept A, B:(no parentheses) -- ruff enforces this on Python 3.14
Line length: 88 characters (ruff enforced)
Files:
tests/unit/api/fakes.pytests/unit/templates/test_preset_service_properties.pytests/unit/persistence/test_protocol.pytests/unit/templates/test_preset_service.pytests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/persistence/preset_repository.pytests/unit/api/controllers/test_personalities.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests:asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Parametrize: Prefer@pytest.mark.parametrizefor testing similar cases
Tests must usetest-provider,test-small-001, etc. instead of real vendor names
Property-based testing in Python: use Hypothesis (@given+@settings); run withHYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties(dev profile: 1000 examples)
Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)-- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/api/fakes.pytests/unit/templates/test_preset_service_properties.pytests/unit/persistence/test_protocol.pytests/unit/templates/test_preset_service.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/api/controllers/test_personalities.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict)
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code)
Variable name for logger: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Structured kwargs for logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG logging for object creation, internal flow, entry/exit of key...
Files:
src/synthorg/api/controllers/personalities.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
🧠 Learnings (60)
📚 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/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
docs/design/memory.mdtests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
docs/design/memory.mdtests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/persistence/preset_repository.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/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
docs/design/memory.mdtests/unit/persistence/test_protocol.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/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
docs/design/memory.mdtests/unit/persistence/test_protocol.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:
docs/design/memory.mdsrc/synthorg/persistence/preset_repository.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
docs/design/organization.mdtests/unit/templates/test_preset_service_properties.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.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:
CLAUDE.mddocs/design/operations.mdtests/unit/persistence/test_protocol.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {docs/design/operations.md,src/synthorg/providers/presets.py,.claude/**/*.{md,yml,yaml}} : Vendor names may appear only in: (1) Operations design page, (2) `.claude/` skill/agent files, (3) third-party import paths, (4) provider presets (`src/synthorg/providers/presets.py`), (5) tests using `test-provider`, `test-small-001`, etc.
Applied to files:
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/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
CLAUDE.mddocs/design/operations.mdsrc/synthorg/api/controllers/personalities.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
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/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/api/**/*.py : REST API: Litestar framework, controllers with guards, channels for WebSocket, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint. RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST API and WebSocket API with JWT + API key + WS ticket authentication, RFC 9457 structured errors, and content negotiation.
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/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-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
CLAUDE.mdsrc/synthorg/persistence/preset_repository.pysrc/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
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/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
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: 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-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-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-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
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-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Web dashboard: see `web/CLAUDE.md` for commands, design system, and component inventory
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-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-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to tests/**/*.py : Property-based testing in Python: use Hypothesis (`given` + `settings`); run with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties` (dev profile: 1000 examples)
Applied to files:
tests/unit/templates/test_preset_service_properties.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: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/templates/test_preset_service_properties.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 tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {docs/rest-api.md,docs/_generated/api-reference.html} : REST API reference in `docs/rest-api.md` + `docs/_generated/api-reference.html` (generated by `scripts/export_openapi.py`)
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Settings: Runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge, ConfigResolver (typed composed reads for controllers), validation, registry, change notifications via message bus. Per-namespace setting definitions in definitions/ submodule (api, company, providers, memory, budget, security, coordination, observability, backup).
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to 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:
docs/design/operations.md
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
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> import EVENT_CONSTANT`
Applied to files:
docs/design/operations.md
📚 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:
docs/design/operations.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : 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:
docs/design/operations.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 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:
docs/design/operations.md
📚 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:
docs/design/operations.md
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use 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:
docs/design/operations.md
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: 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:
docs/design/operations.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to src/synthorg/**/*.py : Errors: handle explicitly, never silently swallow
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/api/dto_personalities.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/api/dto_personalities.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to src/synthorg/**/*.py : 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/api/dto_personalities.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Applied to files:
src/synthorg/api/dto_personalities.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/api/dto_personalities.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/api/dto_personalities.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `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/api/dto_personalities.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. Use `computed_field` for derived values instead of storing redundant fields. Use `NotBlankStr` for all identifier/name fields.
Applied to files:
src/synthorg/api/dto_personalities.py
🪛 LanguageTool
docs/design/organization.md
[grammar] ~294-~294: Ensure spelling is correct
Context: ...ality presets come in two flavors: - Builtin presets ship with the codebase (`temp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~297-~297: Ensure spelling is correct
Context: ...h ^[a-z][a-z0-9_]*$ and cannot shadow builtin names. All custom presets are validated...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (36)
CLAUDE.md (1)
89-89: Documentation updates accurately reflect the new preset architecture.These package-structure descriptions now align with the implemented discovery/CRUD API, preset persistence, and service layering introduced in this PR.
Also applies to: 99-99, 104-104
src/synthorg/persistence/preset_repository.py (1)
1-111: LGTM!Clean protocol definition with well-documented async methods. The use of
NamedTuplefor row types provides clear structure with named fields, and the@runtime_checkabledecorator enablesisinstancechecks. Proper use ofNotBlankStrfor identifier fields aligns with coding guidelines.tests/unit/persistence/test_protocol.py (3)
341-366: LGTM!The
_FakePersonalityPresetRepositorycorrectly implements all protocol methods with matching signatures. The stub return values (None, empty tuple,False,0) are appropriate for a minimal compliance fake.
453-455: LGTM!The
custom_presetsproperty correctly exposes the fake repository, matching thePersistenceBackendprotocol extension.
526-531: LGTM!Protocol compliance assertion for
_FakePersonalityPresetRepositoryis now present, addressing the previously identified gap. This ensures signature regressions will be caught.tests/unit/api/fakes.py (2)
535-568: LGTM!The
FakePersonalityPresetRepositorycorrectly implements the protocol with an in-memory dict. Thelist_all()method properly constructsPresetListRowby unpackingPresetRowfields with the name, maintaining sorted order as specified in the protocol.
577-577: LGTM!The
FakePersistenceBackendcorrectly initializes and exposes the_custom_presetsrepository, matching thePersistenceBackendprotocol contract.Also applies to: 681-683
src/synthorg/persistence/sqlite/preset_repo.py (3)
1-38: LGTM!Clean module setup with proper imports, logger initialization via
get_logger(__name__), and well-documented class docstring. The repository correctly receives a sharedaiosqlite.Connectioninstance.
39-80: LGTM!The
save()method correctly implements upsert semantics usingON CONFLICT(name) DO UPDATE. Error handling logs with context before raisingQueryError, and successful saves are logged at INFO level per guidelines.
183-203: LGTM!The
count()method includes a defensive check forrow is Nonewhich handles edge cases like database driver errors. Logging at ERROR level before raising is appropriate per guidelines.src/synthorg/templates/preset_service.py (3)
1-36: LGTM!Clean module setup with proper imports, logger initialization, and regex pattern for name validation.
41-61: LGTM!The
PresetEntrymodel correctly usesConfigDict(frozen=True, allow_inf_nan=False)per coding guidelines, andNotBlankStrfor the name field.
135-159: LGTM!The
_parse_config_jsonhelper properly handlesJSONDecodeErrorwith context logging and raisesNotFoundErrorfor corrupt rows. This addresses the previous review comment about guarding custom-preset decoding.src/synthorg/api/dto_personalities.py (3)
19-24: LGTM!Clean
StrEnumdefinition for preset source tagging.
29-62: LGTM!Response DTOs are properly structured with
frozen=Trueandallow_inf_nan=False. Thenamefield correctly usesNotBlankStr. Using plainstrfortraitsandcommunication_stylein response DTOs is acceptable since the data originates from already-validatedPersonalityConfiginstances.
68-104: LGTM!The
_PresetFieldsBasecorrectly usesNotBlankStrfor input validation ontraitsandcommunication_style, withextra="forbid"to reject unknown fields. Theto_config_dict()methods properly exclude/include fields as needed forPersonalityConfigvalidation.src/synthorg/api/controllers/personalities.py (3)
1-29: LGTM!Clean imports and logger setup. Proper separation of concerns with helper functions for DTO conversion and service instantiation.
32-58: LGTM!The helper functions cleanly convert between internal
PresetEntryand API response DTOs. The_to_detailfunction correctly excludes"description"from the config spread since it's set separately fromentry.description.
61-154: LGTM!The controller properly implements all endpoints with appropriate guards (
require_read_accessfor discovery,require_write_accessfor mutations). Status codes are correct: 201 for create, 200 for update/delete. Thedelete_presetcorrectly returnsApiResponse[None](data=None).docs/design/memory.md (1)
423-424: LGTM!Documentation correctly updated to reflect the new
custom_presetsrepository capability onPersistenceBackendand the new "Custom preset" entity persisted viaPersonalityPresetRepositorywith lookup by name.Also applies to: 485-485
docs/design/organization.md (1)
290-299: Documentation accurately reflects the implemented API.The new "Personality Presets" section correctly documents:
- Builtin vs custom preset sources and their management
- Custom preset naming constraints (
^[a-z][a-z0-9_]*$)- Validation against
PersonalityConfig- Source tagging in API responses
- Access control (discovery for all authenticated users, CRUD requires write access)
Note: The static analysis hints flagging "builtin" as a spelling issue are false positives — "builtin" is standard technical terminology in this context.
tests/unit/templates/test_preset_service_properties.py (3)
66-89: Property-based round-trip test is well-structured.The test correctly:
- Uses
assume()to filter out builtin preset names- Creates a fresh
FakePersonalityPresetRepositoryper test case- Validates round-trip equality through
create→get- Compares the validated config (which may normalize values)
91-96: Idempotency test is correct and useful.Testing that
_normalize_preset_name(normalize_preset_name(name)) == _normalize_preset_name(name)ensures the normalization function is stable.
34-41: No changes needed — traits strategy correctly allows empty tuples.The
PersonalityConfigmodel explicitly allows empty traits withdefault=()and has no min_length constraint. The test strategy generating empty tuples provides valid coverage and matches the actual model behavior.docs/design/operations.md (2)
1115-1115: API endpoint documentation is comprehensive and accurate.The new personalities endpoint documentation correctly covers:
- Discovery:
GET /presets,GET /presets/{name},GET /schema- CRUD:
POST,PUT,DELETEwith semantic descriptions- Name collision prevention and builtin protection
1343-1343: Event module count correctly updated.The increment from 53 to 54 reflects the new
preset.pyevent constant module underobservability/events/.tests/unit/persistence/sqlite/test_preset_repo.py (3)
43-66: Repository contract tests are thorough.Tests correctly validate:
- Save/get round-trip with all fields
Nonereturn for missing keys- Proper unpacking of
PresetRownamed tuple
68-93: Upsert test now correctly verifiescreated_atpreservation.The test properly asserts that
created_atremains unchanged after upsert (line 92), validating that the SQLON CONFLICTclause only updatesconfig_json,description, andupdated_at.
165-210: Error propagation tests provide good coverage.Using an unmigrated database (missing
custom_presetstable) to verifyQueryErroris raised for all operations is a clean approach that validates the repository's error handling contract.tests/unit/templates/test_preset_service.py (5)
13-30: Test helper produces valid config for all test scenarios.The
_make_valid_config()helper correctly provides all required fields with valid values, including tuple-formatted traits matchingPersonalityConfigexpectations.
45-80: List tests provide comprehensive coverage.Tests validate:
- All builtins are returned
- Custom presets are included when present
- Results are sorted by name
- Source tags are correct
126-178: Create tests cover all important scenarios.Good coverage of:
- Successful creation with name normalization
- Builtin name shadowing rejection (
ConflictError)- Duplicate custom name rejection (
ConflictError)- Invalid config rejection (
ApiValidationError)- Invalid name format rejection
- Blank name rejection with message matching
193-201: Timestamp preservation test is well-designed.This test correctly validates that
created_atis immutable through updates whileupdated_atchanges, which is the expected upsert behavior.
247-254: Schema test validates expected structure.The test confirms the schema contains expected properties with correct types. Note: This is a sync test calling a static/class method, which is appropriate.
tests/unit/api/controllers/test_personalities.py (2)
41-73: Discovery endpoint tests are comprehensive.Tests validate:
- Correct count of builtins
- Pagination behavior
- Required fields in response items
- Observer role read access
116-166: Create tests cover key scenarios well.Good coverage of success and error cases. The duplicate test (lines 153-166) now correctly asserts the first create succeeded before testing the duplicate case.
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 `@tests/unit/api/controllers/test_personalities.py`:
- Around line 41-319: Tests repeat the same patterns for observer 403, builtin
409 conflicts, and 404 not-found across
TestCreatePreset/TestUpdatePreset/TestDeletePreset (notable functions:
test_observer_cannot_create, test_observer_cannot_update,
test_observer_cannot_delete; test_create_with_builtin_name_returns_409,
test_update_builtin_returns_409, test_delete_builtin_returns_409;
test_get_nonexistent_returns_404, test_update_nonexistent_returns_404,
test_delete_nonexistent_returns_404). Consolidate these by adding parameterized
tests that iterate over (HTTP method, endpoint, optional payload, expected
status) and invoke test_client.request(method, endpoint, json=payload,
headers=make_auth_headers(...)) so you replace the three observer_* tests with
one parametric test, likewise collapse the builtin-409 and nonexistent-404 cases
into parameterized tests to remove duplication while keeping existing test
coverage.
- Around line 202-278: The tests repeat the pattern of constructing an update
body by removing "name" from a preset body; add a helper function named
_make_update_body(base_body: dict[str, Any], **overrides: Any) -> dict[str, Any]
(place it after _make_valid_preset_body) which returns {k: v for k, v in
base_body.items() if k != "name"} updated with overrides, then replace the
repeated comprehensions in TestUpdatePreset methods (test_update_custom_preset,
test_update_builtin_returns_409, test_update_nonexistent_returns_404,
test_observer_cannot_update, test_update_with_invalid_config_returns_400) with
calls to _make_update_body(body, ...) (e.g., pass openness=0.1 or other
overrides) to centralize construction and keep intent clear.
🪄 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: b366cd12-c573-4bbf-b3ad-307f24242cbe
📒 Files selected for processing (1)
tests/unit/api/controllers/test_personalities.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations
Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
PEP 758 except syntax: useexcept A, B:(no parentheses) -- ruff enforces this on Python 3.14
Line length: 88 characters (ruff enforced)
Files:
tests/unit/api/controllers/test_personalities.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests:asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Parametrize: Prefer@pytest.mark.parametrizefor testing similar cases
Tests must usetest-provider,test-small-001, etc. instead of real vendor names
Property-based testing in Python: use Hypothesis (@given+@settings); run withHYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties(dev profile: 1000 examples)
Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)-- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/api/controllers/test_personalities.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
🔇 Additional comments (6)
tests/unit/api/controllers/test_personalities.py (6)
12-35: LGTM: Helper function is well-structured.The helper correctly builds valid preset payloads with overrides support, uses appropriate type hints for Python 3.14, and is properly scoped for test-only use.
41-73: LGTM: Comprehensive coverage of list endpoint.Tests verify pagination, required response fields, source value constraints, and observer read permissions.
75-99: LGTM: Detail endpoint tests are sound.Tests cover success case, 404 handling, and observer read access.
101-111: LGTM: Schema endpoint test is sufficient.Verifies that the schema is returned with expected structure.
116-200: LGTM: Create endpoint tests are thorough.Tests cover success path, conflict scenarios (builtin collision, duplicate), validation, permissions, and downstream integration with list/get endpoints. Previous review feedback has been addressed.
280-319: LGTM: Delete endpoint tests are comprehensive.Tests cover success path with verification, builtin protection, 404 handling, and observer permissions. Previous review feedback has been addressed.
| @pytest.mark.unit | ||
| class TestListPresets: | ||
| def test_lists_all_builtins(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/personalities/presets") | ||
| assert resp.status_code == 200 | ||
| body = resp.json() | ||
| assert body["success"] is True | ||
| assert body["pagination"]["total"] == len(PERSONALITY_PRESETS) | ||
|
|
||
| def test_pagination_works(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/personalities/presets?offset=0&limit=5") | ||
| assert resp.status_code == 200 | ||
| body = resp.json() | ||
| assert len(body["data"]) == 5 | ||
| assert body["pagination"]["limit"] == 5 | ||
|
|
||
| def test_each_item_has_required_fields(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/personalities/presets?limit=3") | ||
| body = resp.json() | ||
| for item in body["data"]: | ||
| assert "name" in item | ||
| assert "description" in item | ||
| assert "traits" in item | ||
| assert "source" in item | ||
| assert item["source"] in ("builtin", "custom") | ||
|
|
||
| def test_observer_can_read(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get( | ||
| "/api/v1/personalities/presets", | ||
| headers=make_auth_headers("observer"), | ||
| ) | ||
| assert resp.status_code == 200 | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestGetPreset: | ||
| def test_get_builtin_preset(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/personalities/presets/visionary_leader") | ||
| assert resp.status_code == 200 | ||
| body = resp.json() | ||
| assert body["success"] is True | ||
| assert body["data"]["name"] == "visionary_leader" | ||
| assert body["data"]["source"] == "builtin" | ||
| assert "openness" in body["data"] | ||
| assert "traits" in body["data"] | ||
|
|
||
| def test_get_nonexistent_returns_404(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/personalities/presets/nonexistent_preset_xyz") | ||
| assert resp.status_code == 404 | ||
| body = resp.json() | ||
| assert body["success"] is False | ||
|
|
||
| def test_observer_can_read_detail(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get( | ||
| "/api/v1/personalities/presets/pragmatic_builder", | ||
| headers=make_auth_headers("observer"), | ||
| ) | ||
| assert resp.status_code == 200 | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestGetSchema: | ||
| def test_returns_json_schema(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/personalities/schema") | ||
| assert resp.status_code == 200 | ||
| body = resp.json() | ||
| assert body["success"] is True | ||
| schema = body["data"] | ||
| assert "properties" in schema | ||
| assert "openness" in schema["properties"] | ||
|
|
||
|
|
||
| # ── CRUD endpoints (Issue #756) ────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestCreatePreset: | ||
| def test_create_custom_preset(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body() | ||
| resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 201 | ||
| data = resp.json() | ||
| assert data["success"] is True | ||
| assert data["data"]["name"] == "my_custom_preset" | ||
| assert data["data"]["source"] == "custom" | ||
|
|
||
| def test_create_with_builtin_name_returns_409( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| body = _make_valid_preset_body(name="visionary_leader") | ||
| resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 409 | ||
|
|
||
| def test_create_with_invalid_openness_returns_400( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| body = _make_valid_preset_body(openness=2.0) | ||
| resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 400 | ||
|
|
||
| def test_create_duplicate_returns_409(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body(name="dup_test") | ||
| first_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert first_resp.status_code == 201 | ||
| resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 409 | ||
|
|
||
| def test_observer_cannot_create(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body() | ||
| resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("observer"), | ||
| ) | ||
| assert resp.status_code == 403 | ||
|
|
||
| def test_created_preset_appears_in_list(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body(name="listed_preset") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| resp = test_client.get("/api/v1/personalities/presets?limit=200") | ||
| names = [p["name"] for p in resp.json()["data"]] | ||
| assert "listed_preset" in names | ||
|
|
||
| def test_created_preset_gettable(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body(name="gettable_preset") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| resp = test_client.get("/api/v1/personalities/presets/gettable_preset") | ||
| assert resp.status_code == 200 | ||
| assert resp.json()["data"]["source"] == "custom" | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestUpdatePreset: | ||
| def test_update_custom_preset(self, test_client: TestClient[Any]) -> None: | ||
| # Create first | ||
| body = _make_valid_preset_body(name="updatable") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| # Update | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| update_body["openness"] = 0.1 | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/updatable", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 200 | ||
| assert resp.json()["data"]["openness"] == 0.1 | ||
|
|
||
| def test_update_builtin_returns_409(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body() | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/visionary_leader", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 409 | ||
|
|
||
| def test_update_nonexistent_returns_404(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body() | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/nonexistent_xyz", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 404 | ||
|
|
||
| def test_observer_cannot_update(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body(name="obs_update_test") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/obs_update_test", | ||
| json=update_body, | ||
| headers=make_auth_headers("observer"), | ||
| ) | ||
| assert resp.status_code == 403 | ||
|
|
||
| def test_update_with_invalid_config_returns_400( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| body = _make_valid_preset_body(name="invalid_update") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| update_body["openness"] = 2.0 | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/invalid_update", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 400 | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestDeletePreset: | ||
| def test_delete_custom_preset(self, test_client: TestClient[Any]) -> None: | ||
| # Create first | ||
| body = _make_valid_preset_body(name="deletable") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| resp = test_client.delete( | ||
| "/api/v1/personalities/presets/deletable", | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 200 | ||
| # Verify it's gone | ||
| get_resp = test_client.get("/api/v1/personalities/presets/deletable") | ||
| assert get_resp.status_code == 404 | ||
|
|
||
| def test_delete_builtin_returns_409(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.delete( | ||
| "/api/v1/personalities/presets/visionary_leader", | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 409 | ||
|
|
||
| def test_delete_nonexistent_returns_404(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.delete( | ||
| "/api/v1/personalities/presets/nonexistent_xyz", | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 404 | ||
|
|
||
| def test_observer_cannot_delete(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.delete( | ||
| "/api/v1/personalities/presets/visionary_leader", | ||
| headers=make_auth_headers("observer"), | ||
| ) | ||
| assert resp.status_code == 403 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: Consider parametrizing similar test patterns.
Several test patterns appear across classes:
- Observer 403 tests (create, update, delete)
- Conflict 409 tests (builtin collision, update builtin, delete builtin)
- 404 tests (get, update, delete nonexistent)
While current structure is clear, parametrization could reduce duplication. Example:
`@pytest.mark.parametrize`(
("method", "endpoint", "payload"),
[
("POST", "/api/v1/personalities/presets", _make_valid_preset_body()),
("PUT", "/api/v1/personalities/presets/test", _make_valid_preset_body()),
("DELETE", "/api/v1/personalities/presets/test", None),
],
)
def test_observer_cannot_modify(
test_client: TestClient[Any], method: str, endpoint: str, payload: dict[str, Any] | None
) -> None:
resp = test_client.request(method, endpoint, json=payload, headers=make_auth_headers("observer"))
assert resp.status_code == 403This is a minor optimization; current structure prioritizes clarity over conciseness, which is also valid.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_personalities.py` around lines 41 - 319,
Tests repeat the same patterns for observer 403, builtin 409 conflicts, and 404
not-found across TestCreatePreset/TestUpdatePreset/TestDeletePreset (notable
functions: test_observer_cannot_create, test_observer_cannot_update,
test_observer_cannot_delete; test_create_with_builtin_name_returns_409,
test_update_builtin_returns_409, test_delete_builtin_returns_409;
test_get_nonexistent_returns_404, test_update_nonexistent_returns_404,
test_delete_nonexistent_returns_404). Consolidate these by adding parameterized
tests that iterate over (HTTP method, endpoint, optional payload, expected
status) and invoke test_client.request(method, endpoint, json=payload,
headers=make_auth_headers(...)) so you replace the three observer_* tests with
one parametric test, likewise collapse the builtin-409 and nonexistent-404 cases
into parameterized tests to remove duplication while keeping existing test
coverage.
| @pytest.mark.unit | ||
| class TestUpdatePreset: | ||
| def test_update_custom_preset(self, test_client: TestClient[Any]) -> None: | ||
| # Create first | ||
| body = _make_valid_preset_body(name="updatable") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| # Update | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| update_body["openness"] = 0.1 | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/updatable", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 200 | ||
| assert resp.json()["data"]["openness"] == 0.1 | ||
|
|
||
| def test_update_builtin_returns_409(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body() | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/visionary_leader", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 409 | ||
|
|
||
| def test_update_nonexistent_returns_404(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body() | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/nonexistent_xyz", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 404 | ||
|
|
||
| def test_observer_cannot_update(self, test_client: TestClient[Any]) -> None: | ||
| body = _make_valid_preset_body(name="obs_update_test") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/obs_update_test", | ||
| json=update_body, | ||
| headers=make_auth_headers("observer"), | ||
| ) | ||
| assert resp.status_code == 403 | ||
|
|
||
| def test_update_with_invalid_config_returns_400( | ||
| self, test_client: TestClient[Any] | ||
| ) -> None: | ||
| body = _make_valid_preset_body(name="invalid_update") | ||
| create_resp = test_client.post( | ||
| "/api/v1/personalities/presets", | ||
| json=body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert create_resp.status_code == 201 | ||
| update_body = {k: v for k, v in body.items() if k != "name"} | ||
| update_body["openness"] = 2.0 | ||
| resp = test_client.put( | ||
| "/api/v1/personalities/presets/invalid_update", | ||
| json=update_body, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert resp.status_code == 400 | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest extracting update body construction helper.
The pattern update_body = {k: v for k, v in body.items() if k != "name"} is repeated in 5 tests (lines 214, 226, 236, 252, 270). Extract to a helper to improve maintainability.
♻️ Proposed refactor: Extract helper function
Add this helper after _make_valid_preset_body:
def _make_update_body(base_body: dict[str, Any], **overrides: Any) -> dict[str, Any]:
"""Build an update body by removing 'name' and applying overrides."""
update_body = {k: v for k, v in base_body.items() if k != "name"}
update_body.update(overrides)
return update_bodyThen replace the repeated pattern. For example, in test_update_custom_preset:
- update_body = {k: v for k, v in body.items() if k != "name"}
- update_body["openness"] = 0.1
+ update_body = _make_update_body(body, openness=0.1)Apply similar changes to the other four tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_personalities.py` around lines 202 - 278, The
tests repeat the pattern of constructing an update body by removing "name" from
a preset body; add a helper function named _make_update_body(base_body:
dict[str, Any], **overrides: Any) -> dict[str, Any] (place it after
_make_valid_preset_body) which returns {k: v for k, v in base_body.items() if k
!= "name"} updated with overrides, then replace the repeated comprehensions in
TestUpdatePreset methods (test_update_custom_preset,
test_update_builtin_returns_409, test_update_nonexistent_returns_404,
test_observer_cannot_update, test_update_with_invalid_config_returns_400) with
calls to _make_update_body(body, ...) (e.g., pass openness=0.1 or other
overrides) to centralize construction and keep intent clear.
Add REST endpoints for personality preset management under /api/v1/personalities/: Discovery (Issue #755): - GET /presets -- paginated list of all presets (builtin + custom) - GET /presets/{name} -- full preset detail with source tag - GET /schema -- PersonalityConfig JSON schema CRUD (Issue #756): - POST /presets -- create custom preset (validated, no builtin shadows) - PUT /presets/{name} -- update custom preset - DELETE /presets/{name} -- delete custom preset (builtins protected) Architecture: - PersonalityPresetRepository protocol + SQLite implementation - PersonalityPresetService merges builtin + custom sources - custom_presets table in SQLite schema - Observability event constants for preset operations - 54 new tests (repo, service, property-based, controller) Closes #755 Closes #756 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 4 agents, 14 findings addressed: - Add structured logging to PersonalityPresetService (all error paths + state transitions) - Fix empty-name sentinel bug in get() -- raise NotFoundError early - Use _normalize_preset_name() consistently in update/delete - Inline LIMIT constant to avoid f-string SQL pattern - Remove re-export from repositories.py (was over 800-line limit) - Import PersonalityPresetRepository directly in protocol.py - Remove unused logger from controller - Add service-level event constants (PRESET_CREATED, etc.) - Add 9 new tests: observer auth on update, duplicate create 409, invalid update config, QueryError propagation (5 tests), service update validation - Use hypothesis.assume() instead of bare return in property tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Source fixes: - Replace opaque positional tuples with PresetRow/PresetListRow NamedTuples in repository protocol and SQLite implementation - Guard json.loads on DB strings with JSONDecodeError handling to prevent single corrupt row from poisoning list_all - Redact Pydantic ValidationError detail from API responses (security) - Extract _PresetFieldsBase to deduplicate 13 fields across Create/Update DTOs - Add max_length=50 on traits tuple to prevent unbounded input - Remove unused PresetSchemaResponse dead code - Refactor _to_detail to spread config dict instead of 12 duplicated defaults - Remove redundant Response wrapper on create_preset (Litestar handles status) - Add logger = get_logger(__name__) to controller (only one without it) - Use PresetSource enum in PresetEntry instead of Literal duplicate - Extract _check_not_builtin and _parse_config_json helpers to bring create/update under 50-line limit - Fix count() to raise QueryError instead of returning 0 for impossible None - Use async with for cursor context managers in get/list_all/count - Remove LIMIT 10000 magic number from list_all SQL - Remove redundant aiosqlite.Error from except clauses (is sqlite3.Error) - Fix ValidationError message to not expose internal details Documentation fixes: - Add personality preset endpoints to API Surface table in operations.md - Add custom_presets to PersistenceBackend protocol listing in memory.md - Add custom presets row to Entities Persisted table in memory.md - Update event taxonomy count 53 -> 54 in operations.md - Update CLAUDE.md package structure: api/, persistence/, templates/ - Add Personality Presets section to organization.md design page Test fixes: - Add PersonalityPresetRepository isinstance conformance test - Fix upsert test to verify created_at preservation with different timestamps - Fix property test traits strategy to generate 0-5 elements (was always 1) - Remove hardcoded max_examples=50 to inherit Hypothesis profile settings - Fix fake repos to use NotBlankStr and NamedTuple return types - Add test for service.get with blank name - Fix >= to == assertion for builtin count - Assert first POST status in duplicate creation test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Assert create_resp.status_code == 201 for all setup POST calls in update and delete test classes, preventing confusing failures if the precondition create silently fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use async context manager for DELETE cursor in preset_repo.py, matching the pattern used in get/list_all/count - Add TOCTOU limitation comments on create() and update() explaining the non-atomic get-then-save pattern and why it is acceptable for the current MVP with low concurrency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
07e1f66 to
09d0de5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/synthorg/api/dto_personalities.py (1)
36-49: 🧹 Nitpick | 🔵 TrivialResponse DTOs use
strinstead ofNotBlankStrfor traits and communication_style.While the request DTOs (
_PresetFieldsBase) correctly useNotBlankStr, the response DTOs use plainstr. This is inconsistent with the prior review that was marked as addressed. Since response data comes from already-validated sources (service layer), this is a minor concern but worth noting for API contract consistency.As per coding guidelines: Use
NotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants.♻️ Align response DTOs with request validation
class PresetSummaryResponse(BaseModel): """Summary of a personality preset for list endpoints.""" model_config = ConfigDict(frozen=True, allow_inf_nan=False) name: NotBlankStr description: str = "" - traits: tuple[str, ...] = () + traits: tuple[NotBlankStr, ...] = () source: PresetSource class PresetDetailResponse(BaseModel): """Full personality preset definition.""" model_config = ConfigDict(frozen=True, allow_inf_nan=False) name: NotBlankStr source: PresetSource description: str = "" - traits: tuple[str, ...] = () - communication_style: str = "neutral" + traits: tuple[NotBlankStr, ...] = () + communication_style: NotBlankStr = "neutral"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/dto_personalities.py` around lines 36 - 49, The response DTO PresetDetailResponse currently declares traits: tuple[str, ...] and communication_style: str; update these to use the NotBlankStr type from core.types to match request DTOs and API contract consistency—i.e., change traits to tuple[NotBlankStr, ...] (or tuple[NotBlankStr, ...] = ()) and communication_style to NotBlankStr (or NotBlankStr = "neutral" if keeping a default) inside the PresetDetailResponse class so the response types align with _PresetFieldsBase and the NotBlankStr validation contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/organization.md`:
- Around line 294-297: Replace the informal term "builtin" with the preferred
"built-in" across the documentation fragment describing presets: update the
phrasing in the paragraph referencing templates/presets.py, the POST
/api/v1/personalities/presets endpoint, validation against PersonalityConfig,
and the response field `source: "builtin" | "custom"` so that it reads `source:
"built-in" | "custom"`, ensuring consistency with project terminology.
In `@src/synthorg/persistence/preset_repository.py`:
- Around line 21-29: The PresetListRow named tuple loosens the name type to
plain str; update its declaration to use NotBlankStr for the name field to match
the repository/protocol typing (change PresetListRow.name: str ->
PresetListRow.name: NotBlankStr) and add the appropriate import for NotBlankStr
from core.types; ensure any usages expecting PresetListRow continue to accept
the NotBlankStr-typed name.
In `@src/synthorg/persistence/sqlite/schema.sql`:
- Around line 299-304: The CREATE TABLE for custom_presets allows empty strings
for name and config_json; update the schema for the table (custom_presets) to
add DB-level CHECK constraints such as CHECK(length(name) > 0) and
CHECK(length(config_json) > 0) (or CHECK(name <> ''), CHECK(config_json <> ''))
so empty-string names or payloads are rejected at the database level; modify the
CREATE TABLE statement (and any migration that creates this table) to include
these checks while keeping existing NOT NULLs and defaults for other columns
(e.g., description, created_at, updated_at).
In `@tests/unit/api/fakes.py`:
- Around line 541-555: The fake save method currently overwrites created_at on
every call; change save (async def save) to preserve the original created_at
when self._presets already contains the given name: if name exists, read
existing = self._presets[name] and use existing.created_at when constructing or
updating the PresetRow, while still replacing config_json, description and
updated_at; if it does not exist, create the PresetRow with the provided
created_at as before. Ensure you reference the PresetRow constructor and the
_presets dict so tests keep SQLite upsert semantics.
In `@tests/unit/persistence/sqlite/test_preset_repo.py`:
- Around line 176-210: Consolidate the five nearly identical tests that assert a
QueryError by replacing them with a single parametrized test using
pytest.mark.parametrize that iterates over the repository method calls;
reference the existing test names (test_save_raises_query_error,
test_get_raises_query_error, test_list_all_raises_query_error,
test_delete_raises_query_error, test_count_raises_query_error) and the
unmigrated_repo fixture and QueryError exception to locate the code, and call
the appropriate repository method (e.g., unmigrated_repo.save(...),
unmigrated_repo.get("x"), unmigrated_repo.list_all(),
unmigrated_repo.delete("x"), unmigrated_repo.count()) inside the single test
body asserting pytest.raises(QueryError).
---
Duplicate comments:
In `@src/synthorg/api/dto_personalities.py`:
- Around line 36-49: The response DTO PresetDetailResponse currently declares
traits: tuple[str, ...] and communication_style: str; update these to use the
NotBlankStr type from core.types to match request DTOs and API contract
consistency—i.e., change traits to tuple[NotBlankStr, ...] (or
tuple[NotBlankStr, ...] = ()) and communication_style to NotBlankStr (or
NotBlankStr = "neutral" if keeping a default) inside the PresetDetailResponse
class so the response types align with _PresetFieldsBase and the NotBlankStr
validation contract.
🪄 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: 2dc4530d-bb0d-4db4-a75a-492bb85103eb
📒 Files selected for processing (22)
CLAUDE.mddocs/design/memory.mddocs/design/operations.mddocs/design/organization.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/api/dto_personalities.pysrc/synthorg/observability/events/preset.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/templates/preset_service.pytests/unit/api/controllers/test_personalities.pytests/unit/api/fakes.pytests/unit/observability/test_events.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/persistence/test_protocol.pytests/unit/templates/test_preset_service.pytests/unit/templates/test_preset_service_properties.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: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations
Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
PEP 758 except syntax: useexcept A, B:(no parentheses) -- ruff enforces this on Python 3.14
Line length: 88 characters (ruff enforced)
Files:
tests/unit/observability/test_events.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/protocol.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/templates/test_preset_service_properties.pytests/unit/api/fakes.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/observability/events/preset.pysrc/synthorg/persistence/preset_repository.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_migrations.pysrc/synthorg/api/controllers/personalities.pytests/unit/templates/test_preset_service.pytests/unit/api/controllers/test_personalities.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests:asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Parametrize: Prefer@pytest.mark.parametrizefor testing similar cases
Tests must usetest-provider,test-small-001, etc. instead of real vendor names
Property-based testing in Python: use Hypothesis (@given+@settings); run withHYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties(dev profile: 1000 examples)
Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)-- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/observability/test_events.pytests/unit/persistence/sqlite/test_preset_repo.pytests/unit/templates/test_preset_service_properties.pytests/unit/api/fakes.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/templates/test_preset_service.pytests/unit/api/controllers/test_personalities.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict)
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code)
Variable name for logger: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Structured kwargs for logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG logging for object creation, internal flow, entry/exit of key...
Files:
src/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/observability/events/preset.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/api/controllers/personalities.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
🧠 Learnings (64)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
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> import EVENT_CONSTANT`
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.md
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to 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:
tests/unit/observability/test_events.pydocs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
tests/unit/observability/test_events.pydocs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/persistence/protocol.pydocs/design/memory.mdtests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/preset_repository.pytests/unit/persistence/test_protocol.pysrc/synthorg/persistence/sqlite/preset_repo.pysrc/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/persistence/protocol.pydocs/design/memory.mdtests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/preset_repository.pytests/unit/persistence/test_protocol.pysrc/synthorg/persistence/sqlite/preset_repo.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/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
src/synthorg/persistence/protocol.pydocs/design/memory.mdtests/unit/persistence/test_protocol.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/persistence/protocol.pydocs/design/operations.mdsrc/synthorg/api/controllers/__init__.pytests/unit/persistence/test_protocol.pyCLAUDE.md
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/persistence/protocol.pysrc/synthorg/persistence/preset_repository.pyCLAUDE.mdsrc/synthorg/templates/preset_service.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/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
docs/design/memory.mdtests/unit/persistence/test_protocol.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:
docs/design/memory.md
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
docs/design/organization.mdtests/unit/templates/test_preset_service_properties.pysrc/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to tests/**/*.py : Property-based testing in Python: use Hypothesis (`given` + `settings`); run with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties` (dev profile: 1000 examples)
Applied to files:
tests/unit/templates/test_preset_service_properties.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: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/templates/test_preset_service_properties.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 tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/templates/test_preset_service_properties.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {docs/rest-api.md,docs/_generated/api-reference.html} : REST API reference in `docs/rest-api.md` + `docs/_generated/api-reference.html` (generated by `scripts/export_openapi.py`)
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
docs/design/operations.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/personalities.pyCLAUDE.mdsrc/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Settings: Runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge, ConfigResolver (typed composed reads for controllers), validation, registry, change notifications via message bus. Per-namespace setting definitions in definitions/ submodule (api, company, providers, memory, budget, security, coordination, observability, backup).
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
docs/design/operations.mdsrc/synthorg/observability/events/preset.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:
docs/design/operations.mdsrc/synthorg/observability/events/preset.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
src/synthorg/api/controllers/__init__.pyCLAUDE.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/api/controllers/test_personalities.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/api/controllers/test_personalities.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {docs/design/operations.md,src/synthorg/providers/presets.py,.claude/**/*.{md,yml,yaml}} : Vendor names may appear only in: (1) Operations design page, (2) `.claude/` skill/agent files, (3) third-party import paths, (4) provider presets (`src/synthorg/providers/presets.py`), (5) tests using `test-provider`, `test-small-001`, etc.
Applied to files:
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/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/api/**/*.py : REST API: Litestar framework, controllers with guards, channels for WebSocket, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint. RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST API and WebSocket API with JWT + API key + WS ticket authentication, RFC 9457 structured errors, and content negotiation.
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/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-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
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-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-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-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-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Web dashboard: see `web/CLAUDE.md` for commands, design system, and component inventory
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-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-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.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/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to src/synthorg/**/*.py : Errors: handle explicitly, never silently swallow
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-16T20:14:00.937Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:14:00.937Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization) for `dict`/`list` fields
Applied to files:
src/synthorg/templates/preset_service.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/templates/preset_service.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to src/synthorg/**/*.py : 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/api/dto_personalities.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Applied to files:
src/synthorg/api/dto_personalities.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/api/dto_personalities.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/api/dto_personalities.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `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/api/dto_personalities.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. Use `computed_field` for derived values instead of storing redundant fields. Use `NotBlankStr` for all identifier/name fields.
Applied to files:
src/synthorg/api/dto_personalities.py
🪛 LanguageTool
docs/design/organization.md
[grammar] ~294-~294: Ensure spelling is correct
Context: ...ality presets come in two flavors: - Builtin presets ship with the codebase (`temp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~297-~297: Ensure spelling is correct
Context: ...h ^[a-z][a-z0-9_]*$ and cannot shadow builtin names. All custom presets are validated...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (46)
tests/unit/persistence/sqlite/test_migrations.py (1)
32-32: Schema expectation update is correct.Adding
custom_presetsto_EXPECTED_TABLEScorrectly keeps this migration test aligned with the new SQLite schema.tests/unit/observability/test_events.py (1)
211-211: Domain discovery expectation is correctly updated.Including
"preset"in the expected module set keeps discovery coverage in sync with the new events domain.docs/design/memory.md (1)
432-434: Documentation alignment looks good.The protocol/entity docs now include
custom_presetsconsistently with the persistence and API changes.Also applies to: 494-494
src/synthorg/persistence/sqlite/backend.py (1)
49-51: Repository wiring is complete and consistent.
custom_presetsis correctly integrated across import, lifecycle state, repository creation, and guarded public access.Also applies to: 105-106, 126-127, 197-197, 452-459
src/synthorg/api/controllers/__init__.py (1)
21-23: Controller registration update is correct.Import, route registration tuple, and export list are all updated coherently for
PersonalityPresetController.Also applies to: 57-57, 81-81
src/synthorg/persistence/protocol.py (1)
15-17: Protocol extension is well-scoped and consistent.Adding
custom_presetstoPersistenceBackendcleanly exposes the new repository contract.Also applies to: 62-63, 187-190
CLAUDE.md (1)
89-105: Package structure docs update is consistent with the new preset feature set.The added module descriptions are clear and aligned with the API/persistence/template changes.
docs/design/operations.md (1)
1115-1115: Operations docs are correctly updated for personality presets and observability events.The API surface and event taxonomy sections are in sync with the feature scope.
Also applies to: 1343-1346
tests/unit/persistence/sqlite/test_preset_repo.py (1)
43-163: Great coverage of repository behavior and failure modes.This suite exercises core CRUD semantics and the unmigrated-table error contract well.
tests/unit/templates/test_preset_service_properties.py (1)
68-96: Property tests are well-targeted and add meaningful invariant coverage.The round-trip and normalization properties strengthen confidence beyond example-based tests.
tests/unit/persistence/test_protocol.py (1)
341-366: Protocol conformance coverage for custom preset persistence is correctly extended.This keeps the new
custom_presetscontract guarded in the same style as the other repositories.Also applies to: 453-455, 527-531
src/synthorg/observability/events/preset.py (1)
8-24: Event constants are well-scoped and consistently named.The new preset event domain cleanly matches the structured logging pattern already used across subsystems.
src/synthorg/persistence/preset_repository.py (1)
31-111: Protocol surface and docstrings are clear and complete.The CRUD contract is well-defined and easy to implement consistently across backends.
src/synthorg/api/controllers/personalities.py (5)
100-107: Schema endpoint still usesasyncunnecessarily.Per prior review,
get_schema()only calls the synchronousPersonalityPresetService.get_schema(). While Litestar handles this correctly, removingasyncwould be more accurate.
1-30: Well-organized controller setup.Imports are clean, logger properly initialized with
get_logger(__name__), and helper functions are appropriately scoped.
32-58: Helper functions are well-designed.The
_to_detailcorrectly excludesdescriptionfrom the config spread since it's passed as a separate field, preventing field duplication.
69-98: Discovery endpoints implemented correctly.Pagination and guards are properly applied. The service is correctly constructed per-request.
111-154: CRUD endpoints properly implemented.Correct HTTP status codes, appropriate guards, and clean delegation to the service layer.
tests/unit/templates/test_preset_service.py (6)
1-11: LGTM! Well-structured test module.Clean imports and helper setup. The test organization follows the service's method structure effectively.
45-81: Good coverage of list_all() behavior.Tests cover builtin presets, custom preset merging, sorting, and source tagging correctly.
83-124: Solid get() test coverage.Tests appropriately cover builtin/custom retrieval, case-insensitivity, and error cases.
126-179: Comprehensive create() test coverage.Properly validates success cases and all expected error conditions including name format validation and config validation.
181-223: Good update() test coverage.Particularly valuable: the test verifying
created_atpreservation whileupdated_atchanges.
225-254: LGTM! Complete delete and schema testing.The sync
test_returns_json_schemais appropriate sinceget_schema()is a static method.src/synthorg/persistence/sqlite/preset_repo.py (6)
1-38: Well-structured repository implementation.Proper imports, logging setup, and clear docstring explaining the class purpose. The dependency injection of
aiosqlite.Connectionis clean.
39-81: Upsert logic is correct.The
ON CONFLICT DO UPDATEcorrectly excludescreated_at, preserving the original creation timestamp on updates while updating other fields.
82-121: LGTM! Correct get() implementation.Proper use of async context manager, appropriate logging levels, and correct row-to-NamedTuple mapping.
122-147: LGTM! Clean list_all() implementation.Properly returns sorted results with all required fields mapped to
PresetListRow.
149-181: LGTM! Delete implementation follows prior review feedback.Correctly uses async context manager and properly checks
rowcountfor delete status.
183-203: Good defensive programming in count().The check for
NonefromCOUNT(*)(Lines 198-201) is a reasonable safeguard against driver anomalies, with appropriate error logging.tests/unit/api/controllers/test_personalities.py (6)
1-36: Well-designed test helper.The
_make_valid_preset_body()helper with**overridespattern enables clean test variations.
41-99: Comprehensive discovery endpoint tests.Good coverage of pagination, field validation, and RBAC for both list and get-by-name endpoints.
101-111: LGTM! Adequate schema endpoint test.
116-200: Excellent create endpoint test coverage.Prior review feedback about asserting setup POST results has been addressed. Tests thoroughly cover success, validation errors, conflicts, and authorization.
202-278: Good update endpoint test coverage.Tests properly verify CRUD semantics, authorization, and validation. The prior review's suggestion to extract the update body helper is a nice-to-have refactor but not blocking.
280-319: Complete delete endpoint test coverage.Good verification that deleted presets return 404 on subsequent GET, and proper RBAC/conflict handling.
src/synthorg/templates/preset_service.py (7)
1-40: Well-structured service module.Clean imports, proper logger setup, and the name pattern regex is appropriately defined at module level.
41-61: LGTM! PresetEntry model follows conventions.Frozen model with
allow_inf_nan=FalseandNotBlankStrfor the name field per coding guidelines.
63-177: Helper functions are well-implemented.Each function handles errors appropriately with logging before raising. The
_parse_config_jsonhelper (Lines 135-159) properly addresses prior review feedback about guarding JSON decode operations.
189-250: LGTM! list_all() and get() implement correct precedence.Custom presets properly override builtins with the same name, and the JSON parsing is safely guarded.
252-313: create() correctly documents TOCTOU limitation.The acknowledged race condition (Lines 274-278) is acceptable for the low-concurrency admin use case. The implementation properly validates, normalizes, and persists.
315-374: update() correctly preserves created_at.The implementation properly uses
existing.created_atwhen saving, ensuring the original creation timestamp is maintained.
376-409: LGTM! delete() and get_schema() are well-implemented.The
delete()method avoids TOCTOU by relying on the repository's atomic delete-and-return-affected-count pattern. Theget_schema()is correctly static.src/synthorg/api/dto_personalities.py (3)
19-24: LGTM! Clean PresetSource enum.
68-87: LGTM! Request base model is well-constrained.Properly uses
extra="forbid",NotBlankStrfor string identifiers, and appropriate field constraints.
89-104: LGTM! Request DTOs correctly handle name field semantics.
CreatePresetRequestincludesnameand excludes it into_config_dict(), whileUpdatePresetRequestgets the name from the path parameter and dumps all fields.
docs/design/organization.md
Outdated
| - **Builtin presets** ship with the codebase (`templates/presets.py`) and are read-only. | ||
| - **Custom presets** are user-defined via the REST API (`POST /api/v1/personalities/presets`), persisted to the database, and managed through full CRUD operations. | ||
|
|
||
| Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow builtin names. All custom presets are validated against `PersonalityConfig` before persistence. The API distinguishes origin via a `source: "builtin" | "custom"` field in responses. |
There was a problem hiding this comment.
Use “built-in” instead of “builtin” for documentation consistency.
Minor wording fix improves readability and addresses the grammar hint.
✏️ Suggested doc wording diff
-- **Builtin presets** ship with the codebase (`templates/presets.py`) and are read-only.
+- **Built-in presets** ship with the codebase (`templates/presets.py`) and are read-only.
...
-Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow builtin names.
+Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow built-in names.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Builtin presets** ship with the codebase (`templates/presets.py`) and are read-only. | |
| - **Custom presets** are user-defined via the REST API (`POST /api/v1/personalities/presets`), persisted to the database, and managed through full CRUD operations. | |
| Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow builtin names. All custom presets are validated against `PersonalityConfig` before persistence. The API distinguishes origin via a `source: "builtin" | "custom"` field in responses. | |
| - **Built-in presets** ship with the codebase (`templates/presets.py`) and are read-only. | |
| - **Custom presets** are user-defined via the REST API (`POST /api/v1/personalities/presets`), persisted to the database, and managed through full CRUD operations. | |
| Custom preset names must match `^[a-z][a-z0-9_]*$` and cannot shadow built-in names. All custom presets are validated against `PersonalityConfig` before persistence. The API distinguishes origin via a `source: "builtin" | "custom"` field in responses. |
🧰 Tools
🪛 LanguageTool
[grammar] ~294-~294: Ensure spelling is correct
Context: ...ality presets come in two flavors: - Builtin presets ship with the codebase (`temp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~297-~297: Ensure spelling is correct
Context: ...h ^[a-z][a-z0-9_]*$ and cannot shadow builtin names. All custom presets are validated...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/organization.md` around lines 294 - 297, Replace the informal
term "builtin" with the preferred "built-in" across the documentation fragment
describing presets: update the phrasing in the paragraph referencing
templates/presets.py, the POST /api/v1/personalities/presets endpoint,
validation against PersonalityConfig, and the response field `source: "builtin"
| "custom"` so that it reads `source: "built-in" | "custom"`, ensuring
consistency with project terminology.
- Use "built-in" in prose, keep "builtin" for API enum values (docs/design/organization.md) - Type PresetListRow.name as NotBlankStr to match protocol contract (persistence/preset_repository.py) - Add CHECK(length > 0) constraints on name and config_json columns (persistence/sqlite/schema.sql) - Preserve created_at on upsert in FakePersonalityPresetRepository to match SQLite ON CONFLICT semantics (tests/unit/api/fakes.py) - Parametrize 5 identical QueryError propagation tests into one (tests/unit/persistence/sqlite/test_preset_repo.py) - Use NotBlankStr for traits and communication_style in response DTOs to align with request DTO contract (api/dto_personalities.py) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 `@docs/design/organization.md`:
- Line 299: Update the discovery endpoints line to use the fully qualified paths
including the common prefix; replace "GET /presets", "GET /presets/{name}", "GET
/schema" with "GET /api/v1/personalities/presets", "GET
/api/v1/personalities/presets/{name}", and "GET /api/v1/personalities/schema" so
the doc consistently uses the /api/v1/personalities prefix and avoids ambiguity
for consumers.
🪄 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: 87c96477-9d43-4fd5-8c28-14b0a50ebad3
📒 Files selected for processing (6)
docs/design/organization.mdsrc/synthorg/api/dto_personalities.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/api/fakes.pytests/unit/persistence/sqlite/test_preset_repo.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations
Do NOT usefrom __future__ import annotations-- Python 3.14 has PEP 649
PEP 758 except syntax: useexcept A, B:(no parentheses) -- ruff enforces this on Python 3.14
Line length: 88 characters (ruff enforced)
Files:
tests/unit/api/fakes.pytests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests:asyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Parametrize: Prefer@pytest.mark.parametrizefor testing similar cases
Tests must usetest-provider,test-small-001, etc. instead of real vendor names
Property-based testing in Python: use Hypothesis (@given+@settings); run withHYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties(dev profile: 1000 examples)
Hypothesis profiles:ci(50 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var
For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)-- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/api/fakes.pytests/unit/persistence/sqlite/test_preset_repo.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict)
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap and handler-cleanup code)
Variable name for logger: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Structured kwargs for logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG logging for object creation, internal flow, entry/exit of key...
Files:
src/synthorg/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.py
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/templates/**/*.py : Templates: pre-built company templates, personality presets, and builder.
Applied to files:
docs/design/organization.mdsrc/synthorg/api/dto_personalities.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: Persistence backend: pluggable PersistenceBackend protocol in `src/synthorg/persistence/`, SQLite initial, SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/schema.sqltests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/preset_repository.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/persistence/sqlite/schema.sqltests/unit/persistence/sqlite/test_preset_repo.pysrc/synthorg/persistence/preset_repository.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/persistence/sqlite/test_preset_repo.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/persistence/sqlite/test_preset_repo.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/persistence/sqlite/test_preset_repo.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/persistence/sqlite/test_preset_repo.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/persistence/preset_repository.py
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/persistence/preset_repository.py
📚 Learning: 2026-03-31T14:40:41.736Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:40:41.736Z
Learning: Applies to src/synthorg/**/*.py : 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/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Applied to files:
src/synthorg/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `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/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.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/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.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/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. Use `computed_field` for derived values instead of storing redundant fields. Use `NotBlankStr` for all identifier/name fields.
Applied to files:
src/synthorg/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.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/persistence/preset_repository.pysrc/synthorg/api/dto_personalities.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/persistence/preset_repository.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/api/dto_personalities.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/api/dto_personalities.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/api/dto_personalities.py
🔇 Additional comments (6)
src/synthorg/persistence/preset_repository.py (1)
1-111: Well-structured repository protocol with correct typing.The protocol cleanly separates the
PresetRow(used byget) fromPresetListRow(used bylist_all), matching the SQLite implementation's return semantics. All identifier fields correctly useNotBlankStr, and the async method signatures align with the implementations in the relevant code snippets.tests/unit/api/fakes.py (2)
535-570: Fake repository correctly mirrors SQLite semantics.The
savemethod now preservescreated_aton upsert (lines 549-554), andlist_allorders results alphabetically by name viasorted(self._presets.items()), matching the SQLite implementation'sORDER BY name. ThePresetListRow(name, *row)unpacking is a nice pattern for reconstructing the list row from the storedPresetRow.
572-684: Backend integration looks correct.The
FakePersistenceBackendproperly initializes the_custom_presetsrepository and exposes it via thecustom_presetsproperty, matching the pattern used for other repositories in this file.src/synthorg/persistence/sqlite/schema.sql (1)
298-305: Schema addition looks good.Non-empty constraints for
nameandconfig_jsonimprove data-integrity defense for custom preset storage.tests/unit/persistence/sqlite/test_preset_repo.py (1)
176-201: Nice consolidation of error-path tests.The parametrized QueryError coverage is cleaner and easier to maintain while preserving behavior checks.
src/synthorg/api/dto_personalities.py (1)
49-49: The patternNotBlankStr("neutral")is not problematic.Annotatedtypes are callable at runtime (returning the annotated type itself), and this pattern is used consistently throughout the codebase (96+ instances) without issues. The learnings recommend usingNotBlankStrfor identifier/name fields with no prohibition on constructor-style calls. No changes needed.> Likely an incorrect or invalid review comment.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [0.5.4](v0.5.3...v0.5.4) (2026-04-01) ### Features * artifact and project management UI in web dashboard ([#954](#954)) ([00a0430](00a0430)) * embed MkDocs build output in React web dashboard at /docs ([#948](#948)) ([f229fc2](f229fc2)) * personality preset discovery API and user-defined preset CRUD ([#952](#952)) ([497848a](497848a)) * support multi-provider model resolution with budget-based selection ([#953](#953)) ([146b782](146b782)) * support per-agent memory retention overrides ([#209](#209)) ([#951](#951)) ([020c610](020c610)) ### Documentation * write user guides and tutorials ([#949](#949)) ([1367225](1367225)) --- 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
/api/v1/personalities/GET /presets(paginated list),GET /presets/{name}(full detail),GET /schema(PersonalityConfig JSON schema) -- all withsource: "builtin" | "custom"taggingPOST /presets(create with name collision prevention),PUT /presets/{name}(update),DELETE /presets/{name}(builtins protected) -- persisted via SQLitePersonalityPresetRepositoryprotocol + SQLite implementation,PersonalityPresetServicemerging builtin + custom sources, DTOs, controller with guardsTest plan
uv run ruff check src/ tests/-- cleanuv run mypy src/ tests/-- clean (1269 files, no issues)Closes #755
Closes #756
🤖 Generated with Claude Code