feat: implement artifact and project persistence#947
Merged
Conversation
WalkthroughAdds artifact and project persistence end-to-end: new 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 |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements the backend infrastructure for artifact and project management, replacing previous stubs with full CRUD functionality. It introduces SQLite-backed repositories, a pluggable artifact storage system with a filesystem implementation, and corresponding API controllers and DTOs. The changes also include new observability events and comprehensive unit tests. A performance issue was identified in the filesystem storage backend, where calculating the total storage size via directory iteration on every write operation could lead to significant latency as the number of artifacts grows.
| raise ArtifactTooLargeError(msg) | ||
|
|
||
| async with self._write_lock: | ||
| current_total = await self.total_size() |
Contributor
There was a problem hiding this comment.
The store method recalculates the total size of all artifacts on every call by iterating through the directory via self.total_size(). This is inefficient and will perform poorly with a large number of artifacts, as it involves iterating over all files and calling stat() on each one. This expensive operation is also performed inside a lock, which can become a serious bottleneck, blocking other concurrent writes.
A more performant approach would be to cache the total size in an instance variable. This variable could be initialized once at startup (or lazily on first access) by scanning the directory. Then, it should be updated incrementally on store and delete operations under the same lock to ensure consistency. This would change the total_size() call from an expensive I/O operation to a fast in-memory read, significantly improving write performance and scalability.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #947 +/- ##
==========================================
- Coverage 92.15% 91.98% -0.17%
==========================================
Files 600 605 +5
Lines 32052 32543 +491
Branches 3115 3141 +26
==========================================
+ Hits 29537 29935 +398
- Misses 1985 2071 +86
- Partials 530 537 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/synthorg/api/app.py (1)
500-527:⚠️ Potential issue | 🔴 CriticalArtifact uploads above 2MB are still rejected by the app-level body limit.
With artifact persistence/storage now wired, uploads are expected to support up to 50MB, but Line 725 keeps
request_max_body_sizeat 2MB, so requests fail before artifact endpoint validation.💡 Proposed fix
- request_max_body_size=2_097_152, # 2 MB + # Must be >= artifact API max payload (50 MB) so endpoint-level + # validation can enforce exact storage limits. + request_max_body_size=52_428_800, # 50 MBAlso applies to: 580-588
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/app.py` around lines 500 - 527, The Litestar app factory (the function that returns Litestar and has the "Create and configure the Litestar application." docstring) currently sets request_max_body_size to 2MB which causes artifact uploads to be rejected; update the request_max_body_size setting used when constructing the Litestar app (and the other occurrence referenced around the same factory) to allow 50MB (e.g. 50 * 1024 * 1024) or make it configurable via the existing config/settings service, ensuring the change is applied to both places that set request_max_body_size.tests/unit/api/controllers/test_artifacts.py (1)
1-112: 🧹 Nitpick | 🔵 TrivialConsider adding tests for storage limit error responses.
Based on the
FakeArtifactStorageimplementation (which never raisesArtifactTooLargeErrororArtifactStorageFullError), the HTTP 413 and 507 error response paths in the upload controller cannot be exercised in these unit tests. Consider adding integration tests with a realFileSystemArtifactStorageor extendingFakeArtifactStorageto support configurable error injection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/controllers/test_artifacts.py` around lines 1 - 112, Tests do not cover the upload error paths because FakeArtifactStorage never raises ArtifactTooLargeError or ArtifactStorageFullError; modify tests to exercise the 413 and 507 responses by either (a) adding integration tests that use FileSystemArtifactStorage against a small configured storage/quota and exercise the upload endpoint (upload handler/upload controller) to trigger 413/507, or (b) extend FakeArtifactStorage to support configurable error injection (e.g., a flag or param in FakeArtifactStorage used by the upload handler) so unit tests can simulate raising ArtifactTooLargeError and ArtifactStorageFullError and assert the controller returns HTTP 413 and 507 respectively. Ensure you reference FakeArtifactStorage, ArtifactTooLargeError, ArtifactStorageFullError, and the upload controller/endpoint in the new or updated tests.
🤖 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/artifacts.py`:
- Around line 224-229: The current rollback deletes on-disk content in the
except block after repo.save(updated) fails (see repo.save(updated) and
storage.delete(artifact_id)), which can permanently lose data; change the flow
to an atomic/staged write: write new bytes to a temporary key or path (e.g.,
storage.store(temp_id) or storage.store(artifact_id + ".tmp")), then call
repo.save(updated); on success replace/rename the temp to the real artifact (or
delete the old and move temp into place); on failure either restore the original
bytes from a saved backup or leave the original untouched and delete only the
temp, and ensure any storage.delete calls refer to the temp and do not silently
suppress restore errors—use explicit error handling rather than
contextlib.suppress to surface failures.
- Around line 54-59: The ArtifactController is missing a public DELETE route to
remove artifact metadata and bytes; add a delete handler (e.g., method
delete_artifact or delete) on ArtifactController registered at
"/artifacts/{artifact_id}" that validates/authenticates the caller, calls
repo.delete(artifact_id) to remove metadata and then calls
artifact_storage.delete(artifact_id) to remove stored bytes, handles and logs
errors from each call, and returns the appropriate HTTP status (204 on success,
404 if not found, 500 on storage/backend errors). Ensure you reference the
existing ArtifactController class and use the repository and artifact_storage
instances the controller already uses so deletion is reachable from the API.
- Around line 28-42: TaskIdFilter and CreatedByFilter currently allow plain
str|None; change their annotated types to use NotBlankStr so whitespace-only
values are rejected at the API boundary: update TaskIdFilter and CreatedByFilter
to Annotated[NotBlankStr | None, Parameter(...)] (or the equivalent NotBlankStr
| None inside Annotated), and add an import for NotBlankStr from core.types;
ensure you only change the type aliases (keep the Parameter metadata intact) so
identifier validation is applied consistently at the controller level.
In `@src/synthorg/api/controllers/projects.py`:
- Around line 32-38: Replace the loose string type for LeadFilter with the
stricter identifier type: change LeadFilter from "str | None" to "NotBlankStr |
None" and import NotBlankStr from core.types so the API performs boundary
validation (no whitespace-only values) consistent with the repository and other
DTOs; keep the Parameter metadata (required=False, description) unchanged and
ensure any uses of LeadFilter (e.g., in controller functions or calls to
repository methods) remain compatible with the repository signature that expects
NotBlankStr.
In `@src/synthorg/api/dto.py`:
- Around line 268-280: The validator _validate_deadline_format currently
suppresses the original parsing traceback by using "raise ValueError(...) from
None"; remove the "from None" and re-raise the ValueError so the original
datetime.fromisoformat ValueError context is preserved (or alternatively capture
the original exception and include its message in the new ValueError) when
validating the deadline field to aid debugging.
In `@src/synthorg/persistence/filesystem_artifact_storage.py`:
- Around line 238-241: The _write_file method currently writes directly with
file_path.write_bytes, risking truncated or missing data on overwrite; change it
to create the parent dir (keep self._artifacts_dir.mkdir(...)), write the
content to a uniquely named temporary file in the same directory (so same
filesystem), fsync the temp file (and its file descriptor) after writing, close
it, then atomically move it over the destination using Path.replace(file_path)
so the replace only happens after a successful full write; ensure any temp file
is removed on failure.
In `@src/synthorg/persistence/repositories.py`:
- Around line 827-846: The protocol must guarantee deterministic ordering for
paginated results: update the PersistenceBackend contract and the repository
method docs for list_artifacts and list_projects to state a specific stable
ordering (e.g., sort by artifact/project id ascending or by created_at then id)
and ensure all implementations honor it; change the docstrings for
list_artifacts(list_projects) to declare the chosen ordering as part of the
contract and update the SQLite repository implementation to apply that ordered
sort so pagination remains stable across backends.
- Around line 798-925: Move the ArtifactRepository and ProjectRepository
Protocol definitions out of repositories.py into a new module (e.g.,
synthorg.persistence.protocols or synthorg.persistence.repo_protocols) and
import/re-export them from repositories.py to preserve the public API;
specifically, create the new module containing the ArtifactRepository and
ProjectRepository classes (with their docstrings unchanged), update any local
imports to reference the new module, and add a re-export line in repositories.py
(e.g., from .repo_protocols import ArtifactRepository, ProjectRepository) so
callers of ArtifactRepository and ProjectRepository keep working and the
original file stays under the 800-line cap.
In `@src/synthorg/persistence/sqlite/project_repo.py`:
- Line 96: Replace the parenthesized multi-except clauses with the PEP 758
no-parentheses form for each occurrence in project_repo.py: change `except
(sqlite3.Error, aiosqlite.Error) as exc:` to `except sqlite3.Error,
aiosqlite.Error as exc:` and change `except (ValueError, ValidationError,
json.JSONDecodeError, KeyError) as exc:` to `except ValueError, ValidationError,
json.JSONDecodeError, KeyError as exc:`; update every place those patterns
appear (the handlers around the sqlite/aiosqlite errors and the validation/json
exception handlers) so the exception targets are listed without surrounding
parentheses while keeping the same exception variable name `exc`.
In `@tests/unit/api/controllers/test_projects.py`:
- Around line 49-64: The test test_list_projects_after_create is brittle because
it asserts pagination["total"] == 2 against shared state; change the test to
create projects with a unique name prefix (e.g., include a UUID or timestamp)
using the same POST calls, then GET /api/v1/projects and either filter
body["data"] by that prefix and assert you find exactly the two created items,
or assert body["pagination"]["total"] >= 2 but explicitly verify the created
project names appear in the returned data; apply the same fix pattern to
test_list_artifacts_after_create so both tests no longer rely on global
project/artifact counts.
In `@tests/unit/persistence/test_protocol.py`:
- Around line 304-314: The fake repository's method signatures use plain str
instead of the protocol's NotBlankStr alias, causing type mismatch; update the
type annotations for list_artifacts (parameters task_id and created_by) to use
NotBlankStr | None (keeping artifact_type as ArtifactType | None) so they match
the protocol's signatures, and apply the same change to
_FakeProjectRepository.list_projects' lead parameter to use NotBlankStr | None
as well; leave return types and implementations unchanged.
---
Outside diff comments:
In `@src/synthorg/api/app.py`:
- Around line 500-527: The Litestar app factory (the function that returns
Litestar and has the "Create and configure the Litestar application." docstring)
currently sets request_max_body_size to 2MB which causes artifact uploads to be
rejected; update the request_max_body_size setting used when constructing the
Litestar app (and the other occurrence referenced around the same factory) to
allow 50MB (e.g. 50 * 1024 * 1024) or make it configurable via the existing
config/settings service, ensuring the change is applied to both places that set
request_max_body_size.
In `@tests/unit/api/controllers/test_artifacts.py`:
- Around line 1-112: Tests do not cover the upload error paths because
FakeArtifactStorage never raises ArtifactTooLargeError or
ArtifactStorageFullError; modify tests to exercise the 413 and 507 responses by
either (a) adding integration tests that use FileSystemArtifactStorage against a
small configured storage/quota and exercise the upload endpoint (upload
handler/upload controller) to trigger 413/507, or (b) extend FakeArtifactStorage
to support configurable error injection (e.g., a flag or param in
FakeArtifactStorage used by the upload handler) so unit tests can simulate
raising ArtifactTooLargeError and ArtifactStorageFullError and assert the
controller returns HTTP 413 and 507 respectively. Ensure you reference
FakeArtifactStorage, ArtifactTooLargeError, ArtifactStorageFullError, and the
upload controller/endpoint in the new or updated tests.
🪄 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: 69f21004-46a1-41f3-a1e6-760f63fa2cc7
📒 Files selected for processing (32)
CLAUDE.mddocs/design/memory.mddocs/design/operations.mddocs/design/page-structure.mdsrc/synthorg/api/app.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/dto.pysrc/synthorg/api/state.pysrc/synthorg/core/artifact.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/artifact_storage.pysrc/synthorg/persistence/errors.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/api/conftest.pytests/unit/api/controllers/test_artifacts.pytests/unit/api/controllers/test_projects.pytests/unit/api/fakes.pytests/unit/core/test_artifact.pytests/unit/persistence/sqlite/test_artifact_repo.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/persistence/test_artifact_storage_protocol.pytests/unit/persistence/test_filesystem_artifact_storage.pytests/unit/persistence/test_protocol.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 (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotations-- Python 3.14 has native PEP 649 lazy annotations
Useexcept A, B:syntax (PEP 758 except syntax, no parentheses) for multiple exception handling in Python 3.14
Files:
src/synthorg/persistence/errors.pytests/unit/api/conftest.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/core/test_artifact.pytests/unit/persistence/test_artifact_storage_protocol.pysrc/synthorg/persistence/__init__.pytests/unit/api/controllers/test_projects.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pytests/unit/api/fakes.pysrc/synthorg/core/artifact.pysrc/synthorg/api/state.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/api/controllers/test_artifacts.pytests/unit/persistence/test_filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_storage.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/persistence/sqlite/test_artifact_repo.pysrc/synthorg/api/dto.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/persistence/filesystem_artifact_storage.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: All public functions and classes must have type hints. Type-check code with mypy in strict mode
Use Google style docstrings on all public classes and functions. This is enforced by ruff D rules
Create new objects instead of mutating existing ones. Usecopy.deepcopy()at construction for non-Pydantic internal collections andMappingProxyTypefor read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic models for config/identity and separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 withallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields, including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants, instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Maintain line length of 88 characters (enforced by ruff)
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate data at system boundaries (user input, external APIs, config files)
Library reference indocs/api/is auto-generated via mkdocstrings + Griffe (AST-based). Docstrings in source code are the source of truth
Files:
src/synthorg/persistence/errors.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pysrc/synthorg/core/artifact.pysrc/synthorg/api/state.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/artifact_storage.pysrc/synthorg/api/dto.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/persistence/filesystem_artifact_storage.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Variable name must always belogger
Use event name constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool). Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured logging:logger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
DEBUG logging is appropriate for object creation, internal flow, and entry/exit of key functions
All provider calls go throughBaseCompletionProviderwhich applies retry and rate limiting automatically. Never implement retry logic in driver subclasses or calling code
Retryable errors include:RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError. Non-retryable errors raise immediately without retry
Files:
src/synthorg/persistence/errors.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pysrc/synthorg/core/artifact.pysrc/synthorg/api/state.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/artifact_storage.pysrc/synthorg/api/dto.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/persistence/filesystem_artifact_storage.py
!(src/synthorg/observability/setup.py|src/synthorg/observability/sinks.py)
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
import logging,logging.getLogger(), orprint()in application code. Exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint(..., file=sys.stderr)for bootstrap/handler-cleanup code
Files:
src/synthorg/persistence/errors.pytests/unit/api/conftest.pytests/unit/persistence/sqlite/test_migrations.pydocs/design/operations.mdtests/unit/core/test_artifact.pytests/unit/persistence/test_artifact_storage_protocol.pysrc/synthorg/persistence/__init__.pytests/unit/api/controllers/test_projects.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pytests/unit/api/fakes.pydocs/design/memory.mdsrc/synthorg/core/artifact.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/api/state.pysrc/synthorg/persistence/sqlite/backend.pydocs/design/page-structure.mdtests/unit/api/controllers/test_artifacts.pytests/unit/persistence/test_filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_storage.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_project_repo.pyCLAUDE.mdtests/unit/persistence/sqlite/test_artifact_repo.pysrc/synthorg/api/dto.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/persistence/filesystem_artifact_storage.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers to categorize tests
Maintain 80% minimum test coverage (enforced in CI). Run coverage with:uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80
Useasyncio_mode = "auto"in pytest configuration -- no manual@pytest.mark.asynciomarkers needed
Global test timeout is 30 seconds per test (configured in pyproject.toml). Do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed
Prefer@pytest.mark.parametrizefor testing similar cases
NEVER skip, dismiss, or ignore flaky tests. Always fix them fully. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks blocking indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/api/conftest.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/core/test_artifact.pytests/unit/persistence/test_artifact_storage_protocol.pytests/unit/api/controllers/test_projects.pytests/unit/api/fakes.pytests/unit/api/controllers/test_artifacts.pytests/unit/persistence/test_filesystem_artifact_storage.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/persistence/sqlite/test_artifact_repo.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use Markdown for documentation in
docs/directory, built with Zensical. Config:mkdocs.yml. Do not edit.github/CHANGELOG.md(auto-generated) or release config files manually
Files:
docs/design/operations.mddocs/design/memory.mddocs/design/page-structure.md
🧠 Learnings (51)
📓 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).
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).
📚 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:
docs/design/operations.mdCLAUDE.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
docs/design/operations.mdCLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
docs/design/operations.mdsrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pysrc/synthorg/persistence/artifact_storage.pytests/unit/persistence/test_protocol.pyCLAUDE.mdsrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/filesystem_artifact_storage.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/__init__.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pydocs/design/memory.mdsrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/artifact_storage.pytests/unit/persistence/test_protocol.pytests/unit/persistence/sqlite/test_project_repo.pyCLAUDE.mdsrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/persistence/sqlite/artifact_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/__init__.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/app.pydocs/design/memory.mdsrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/artifact_storage.pyCLAUDE.mdsrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/persistence/sqlite/artifact_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/__init__.pydocs/design/memory.mdsrc/synthorg/persistence/artifact_storage.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:
src/synthorg/persistence/__init__.pydocs/design/memory.mdsrc/synthorg/persistence/artifact_storage.pyCLAUDE.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/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/protocol.pyCLAUDE.md
📚 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/app.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
tests/unit/api/fakes.pytests/unit/persistence/test_protocol.pysrc/synthorg/persistence/repositories.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/observability/**/*.py : Observability includes structured logging via `get_logger(__name__)`, correlation tracking, and log sinks.
Applied to files:
src/synthorg/api/state.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use import logging / logging.getLogger() / print() in application code.
Applied to files:
src/synthorg/api/state.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 : Every module with business logic must import `from synthorg.observability import get_logger` and define `logger = get_logger(__name__)`
Applied to files:
src/synthorg/api/state.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 : Every module with business logic MUST have `from synthorg.observability import get_logger` followed by `logger = get_logger(__name__)`.
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic must import logger via `from synthorg.observability import get_logger` and initialize with `logger = get_logger(__name__)`
Applied to files:
src/synthorg/api/state.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 : Every module with business logic must have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/synthorg/api/state.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/page-structure.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 docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-31T08:35:07.813Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T08:35:07.813Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue. Design specification is the starting point for architecture, data models, and behavior
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue — DESIGN_SPEC.md is a pointer file linking to 7 design pages (Agents, Organization, Communication, Engine, Memory, Operations)
Applied to files:
docs/design/page-structure.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:
docs/design/page-structure.md
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue. DESIGN_SPEC.md is a pointer file linking to the 7 design pages (index, agents, organization, communication, engine, memory, operations).
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-31T08:35:07.813Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T08:35:07.813Z
Learning: Applies to web/src/**/*.{ts,tsx} : See `web/CLAUDE.md` for web dashboard design system, component inventory, design token rules, and TypeScript/React conventions (TS6, Storybook 10 post-training references)
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-31T08:35:30.026Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-03-31T08:35:30.026Z
Learning: Applies to web/src/{components,pages}/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput)
Applied to files:
docs/design/page-structure.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:
tests/unit/persistence/test_protocol.pysrc/synthorg/persistence/repositories.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.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/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:
tests/unit/persistence/test_protocol.pysrc/synthorg/persistence/repositories.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/api/**/*.py : Authentication uses JWT + API key. Approval gate integration for high-risk operations.
Applied to files:
tests/unit/persistence/test_protocol.pysrc/synthorg/persistence/repositories.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:
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-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.mdsrc/synthorg/persistence/repositories.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.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/api/dto.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/api/dto.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.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.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/api/dto.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/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:
src/synthorg/persistence/repositories.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
src/synthorg/persistence/repositories.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-31T08:35:07.813Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T08:35:07.813Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/persistence.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/persistence.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/observability/events/persistence.py
🔇 Additional comments (30)
src/synthorg/observability/events/persistence.py (1)
203-243: Event coverage for artifact/project persistence looks complete and consistent.The added constants follow the established naming taxonomy and align with downstream repository/storage logging usage.
tests/unit/api/conftest.py (1)
40-43: Fixture wiring is correctly updated for artifact storage.Injecting
FakeArtifactStorageintocreate_appkeeps API unit tests aligned with the new app state dependencies.Also applies to: 278-279
docs/design/operations.md (1)
1108-1108: API surface documentation is accurately updated.The endpoint description now reflects artifact CRUD and binary content transfer behavior.
CLAUDE.md (1)
99-100: Persistence package documentation is aligned with the implementation.The updated summary correctly captures repository expansion and artifact storage pluggability.
src/synthorg/persistence/errors.py (1)
37-42: New artifact storage exceptions are well-scoped and correctly integrated into the hierarchy.This gives callers and controllers clearer failure semantics for size-limit and capacity-limit paths.
tests/unit/persistence/sqlite/test_migrations.py (1)
30-31: Migration expectations were correctly expanded for the new schema objects.The added table and index assertions align with the artifact/project DDL and protect against regression in schema application.
Also applies to: 67-71
src/synthorg/persistence/__init__.py (1)
10-11: Public persistence exports are cleanly updated for the new repositories and error types.This keeps the package import surface consistent for downstream consumers.
Also applies to: 23-23, 28-28, 34-37, 47-47
tests/unit/core/test_artifact.py (2)
87-114: Good coverage added for artifact content field defaults and size validation.These tests exercise both happy-path construction and rejection of invalid negative size values.
215-216: JSON round-trip coverage for new artifact fields is correctly extended.Including
content_typeandsize_bytesin serialization assertions closes a key regression gap.Also applies to: 223-224
tests/unit/api/fakes.py (2)
404-494: Good protocol-aligned test doubles for artifact/project persistence and storage.The in-memory repositories and storage backend are coherent, async-safe for unit usage, and structurally match the new persistence/storage interfaces.
521-523:FakePersistenceBackendwiring is clean and complete.Exposing
artifactsandprojectsvia dedicated properties keeps parity with the extended backend protocol and improves test realism.Also applies to: 562-568
src/synthorg/core/artifact.py (1)
64-72: Model extension is solid and persistence-aligned.
content_typeand non-negativesize_bytesare correctly modeled for artifact content metadata.tests/unit/persistence/test_artifact_storage_protocol.py (1)
13-22: Nice contract tests for runtime protocol conformance.These checks are focused and correctly verify both production and fake storage implementations against
ArtifactStorageBackend.src/synthorg/persistence/sqlite/schema.sql (1)
263-296: Schema additions are consistent with repository expectations.
artifacts/projectstables and indexes align with the new persistence layer and include key integrity checks for numeric bounds.src/synthorg/persistence/protocol.py (1)
18-25:PersistenceBackendextension is well-structured.Adding typed
artifactsandprojectsrepository properties cleanly expands the protocol surface for the new persistence domains.Also applies to: 57-58, 173-181
src/synthorg/api/app.py (1)
580-588: Auto-wiring artifact storage from the persistence data directory is a good default.This keeps metadata/content co-located and reduces bootstrap friction when
SYNTHORG_DB_PATHis used.Also applies to: 646-646
src/synthorg/api/state.py (1)
71-71:AppStateartifact storage integration is consistent and robust.The
has_*flag plus guarded accessor mirrors existing service patterns and preserves clear 503 behavior when unconfigured.Also applies to: 119-119, 125-125, 189-198
docs/design/page-structure.md (1)
12-12: Doc updates are clear and accurately reflect phased delivery.The backend-ready vs dashboard-deferred distinction is explicit and keeps the page map truthful.
Also applies to: 47-47, 368-369, 387-387
tests/unit/api/controllers/test_projects.py (2)
27-47: LGTM!Comprehensive test covering the create-and-retrieve flow with proper assertions on response status, success flag, ID format, and field values.
66-89: LGTM!Good coverage of both valid and invalid deadline scenarios, ensuring the DTO validation (ISO 8601 parsing) is exercised via the API.
src/synthorg/persistence/sqlite/backend.py (1)
428-444: LGTM!The new
artifactsandprojectsproperties follow the established pattern: they use_require_connectedfor safety, have proper Google-style docstrings, and are wired into the lifecycle via_clear_state()and_create_repositories().docs/design/memory.md (1)
482-483: LGTM!Documentation accurately reflects the new
ArtifactandProjectpersistence capabilities with correct source module paths, repository names, and key query patterns.tests/unit/api/controllers/test_artifacts.py (2)
27-49: LGTM!Solid test covering artifact creation and retrieval with proper assertions on status codes, ID format, and field preservation.
77-104: LGTM!Good coverage of the
task_idfilter functionality, verifying both the pagination count and the actual filtered data.tests/unit/persistence/test_protocol.py (1)
485-489: LGTM!Good addition of protocol compliance tests for the new artifact and project repositories.
src/synthorg/persistence/artifact_storage.py (1)
1-88: LGTM!Clean protocol definition with proper docstrings documenting the contract, including expected exceptions (
ArtifactTooLargeError,ArtifactStorageFullError,RecordNotFoundError). The stateless design (no lifecycle methods) is appropriate for a storage backend focused solely on content persistence.tests/unit/persistence/sqlite/test_project_repo.py (1)
41-175: LGTM!Excellent test coverage for
SQLiteProjectRepositoryincluding:
- Basic CRUD operations (save, get, delete)
- Upsert behavior verification
- Filter combinations (status, lead, combined)
- Field roundtrip preservation (JSON fields, nullable fields, floats)
- Edge cases (empty tuples, None values)
src/synthorg/api/dto.py (2)
222-241: LGTM!Well-structured DTO with appropriate field constraints (
max_lengthon path, description, content_type) and proper use ofNotBlankStrfor identifier fields.
582-610: LGTM!The new DTOs are properly exported in
__all__maintaining alphabetical order.src/synthorg/persistence/sqlite/artifact_repo.py (1)
100-100: Please provide the review comment you would like me to verify and rewrite. Include it within<review_comment>tags along with any relevant context about the code being reviewed (file paths, code snippets, etc.).
Comment on lines
+827
to
+846
| async def list_artifacts( | ||
| self, | ||
| *, | ||
| task_id: NotBlankStr | None = None, | ||
| created_by: NotBlankStr | None = None, | ||
| artifact_type: ArtifactType | None = None, | ||
| ) -> tuple[Artifact, ...]: | ||
| """List artifacts with optional filters. | ||
|
|
||
| Args: | ||
| task_id: Filter by originating task ID. | ||
| created_by: Filter by creator agent ID. | ||
| artifact_type: Filter by artifact type. | ||
|
|
||
| Returns: | ||
| Matching artifacts as a tuple. | ||
|
|
||
| Raises: | ||
| PersistenceError: If the operation fails. | ||
| """ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make list ordering part of the protocol contract.
The SQLite repos sort by id, but list_artifacts() / list_projects() do not promise any ordering here. Because the controllers paginate the returned tuples afterward, a future backend can legally return a different order and make pages unstable across requests unless the protocol requires a deterministic sort.
Based on learnings, "Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD)."
Also applies to: 893-910
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/persistence/repositories.py` around lines 827 - 846, The
protocol must guarantee deterministic ordering for paginated results: update the
PersistenceBackend contract and the repository method docs for list_artifacts
and list_projects to state a specific stable ordering (e.g., sort by
artifact/project id ascending or by created_at then id) and ensure all
implementations honor it; change the docstrings for
list_artifacts(list_projects) to declare the chosen ordering as part of the
contract and update the SQLite repository implementation to apply that ordered
sort so pagination remains stable across backends.
| ), | ||
| ) | ||
| await self._db.commit() | ||
| except (sqlite3.Error, aiosqlite.Error) as exc: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 225 src/synthorg/persistence/sqlite/project_repo.py | tail -n +90 | cat -nRepository: Aureliolo/synthorg
Length of output: 5859
🏁 Script executed:
sed -n '92,100p;119,127p;134,142p;181,189p;187,195p;211,219p' src/synthorg/persistence/sqlite/project_repo.pyRepository: Aureliolo/synthorg
Length of output: 2464
🏁 Script executed:
wc -l src/synthorg/persistence/sqlite/project_repo.pyRepository: Aureliolo/synthorg
Length of output: 113
🏁 Script executed:
sed -n '180,220p' src/synthorg/persistence/sqlite/project_repo.py | cat -nRepository: Aureliolo/synthorg
Length of output: 1942
Switch to PEP 758 multi-except syntax (except A, B: without parentheses).
These exception handlers use the parenthesized form, which violates the Python 3.14 style rule. Change lines 96, 123, 138, 185, 191, and 215 to use the no-parentheses form:
except (sqlite3.Error, aiosqlite.Error) as exc:→except sqlite3.Error, aiosqlite.Error as exc:except (ValueError, ValidationError, json.JSONDecodeError, KeyError) as exc:→except ValueError, ValidationError, json.JSONDecodeError, KeyError as exc:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/persistence/sqlite/project_repo.py` at line 96, Replace the
parenthesized multi-except clauses with the PEP 758 no-parentheses form for each
occurrence in project_repo.py: change `except (sqlite3.Error, aiosqlite.Error)
as exc:` to `except sqlite3.Error, aiosqlite.Error as exc:` and change `except
(ValueError, ValidationError, json.JSONDecodeError, KeyError) as exc:` to
`except ValueError, ValidationError, json.JSONDecodeError, KeyError as exc:`;
update every place those patterns appear (the handlers around the
sqlite/aiosqlite errors and the validation/json exception handlers) so the
exception targets are listed without surrounding parentheses while keeping the
same exception variable name `exc`.
Comment on lines
+49
to
+64
| def test_list_projects_after_create(self, test_client: TestClient[Any]) -> None: | ||
| test_client.post( | ||
| "/api/v1/projects", | ||
| json={"name": "P1"}, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| test_client.post( | ||
| "/api/v1/projects", | ||
| json={"name": "P2"}, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| resp = test_client.get("/api/v1/projects") | ||
| assert resp.status_code == 200 | ||
| body = resp.json() | ||
| assert body["pagination"]["total"] == 2 | ||
| assert len(body["data"]) == 2 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test may be brittle due to shared state across tests.
This test asserts pagination["total"] == 2, but if tests run in a different order or if the test database isn't reset between tests, the count could include projects created by other tests. Consider either:
- Filtering by a unique identifier (e.g., project name prefix)
- Asserting
>= 2and checking specific items exist - Ensuring test isolation via fixture cleanup
The same concern applies to test_list_artifacts_after_create in the artifacts test file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_projects.py` around lines 49 - 64, The test
test_list_projects_after_create is brittle because it asserts
pagination["total"] == 2 against shared state; change the test to create
projects with a unique name prefix (e.g., include a UUID or timestamp) using the
same POST calls, then GET /api/v1/projects and either filter body["data"] by
that prefix and assert you find exactly the two created items, or assert
body["pagination"]["total"] >= 2 but explicitly verify the created project names
appear in the returned data; apply the same fix pattern to
test_list_artifacts_after_create so both tests no longer rely on global
project/artifact counts.
Add persistence layer for artifacts and projects with pluggable artifact content storage backend. - Extend Artifact model with content_type and size_bytes fields - Add ArtifactRepository and ProjectRepository protocols - Implement SQLiteArtifactRepository and SQLiteProjectRepository - Add ArtifactStorageBackend protocol with FileSystemArtifactStorage - Flesh out ArtifactController and ProjectController with full CRUD - Add content upload/download endpoints for artifacts - Wire artifact storage into AppState and create_app - Add CreateArtifactRequest and CreateProjectRequest DTOs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 6 agents, 15 findings addressed: - Fix path traversal in FileSystemArtifactStorage (validate artifact_id) - Add asyncio.Lock to prevent TOCTOU race on storage capacity - Add rollback cleanup on two-phase write failure in upload_content - Validate enum filter values (400 instead of 500 for invalid type/status) - Normalize timestamps to UTC in artifact_repo (match user_repo pattern) - Add KeyError to deserialization catch blocks in both repos - Add OSError handling in retrieve (not just FileNotFoundError) - Use unlink(missing_ok-safe) pattern in delete to prevent TOCTOU - Add ORDER BY id to list_artifacts and list_projects for determinism - Re-export ArtifactTooLargeError/ArtifactStorageFullError from __init__ - Update docs: memory.md entities table, page-structure.md stub refs, operations.md artifact endpoint desc, CLAUDE.md package structure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Security:
- Increase request_max_body_size to 50 MB to match artifact storage limit
- Replace str(exc) with static error messages in 413/507 responses
- Add content_type allowlist + Content-Disposition: attachment on download
- Add max_length to filter query parameters
- Validate FileSystemArtifactStorage constructor limits
Data safety:
- Atomic file writes via temp file + Path.replace()
- Log rollback failures instead of silently suppressing
- Narrow rollback except clause to PersistenceError
- Add KeyError to list_artifacts deserialization catch
- Cache total_size for O(1) per-write instead of O(N) scan
- Add SQL LIMIT safety cap to list queries
API completeness:
- Add DELETE /artifacts/{id} endpoint
- Return JSON error body on download 404 (not empty bytes)
- Log content-missing case in download
Type safety:
- Use AwareDatetime for Artifact.created_at (NOT NULL in schema)
- Use NotBlankStr for ArtifactStorageBackend artifact_id params
- Use NotBlankStr + max_length for controller filter params
- Fix test_protocol.py fake type annotations
Code structure:
- Extract ArtifactRepository + ProjectRepository to artifact_project_repos.py
(repositories.py was 925 lines, now under 800)
- Split store() into store() + _check_capacity_and_write() (was 63 lines)
- Add deterministic ordering contract to protocol docstrings
Docs & conventions:
- Fix "CRUD" text in operations.md (no update/delete for projects)
- Update RecordNotFoundError docstring (no longer unused)
- Add team uniqueness + max_length validation to CreateProjectRequest
- Preserve exception context in deadline validator (from exc)
- Add new observability event constants
Tests:
- Add download_content tests (happy path, 404, content-missing)
- Add delete endpoint tests
- Add storage limit error tests (413/507 via FakeArtifactStorage)
- Add invalid type/status filter tests
- Add duplicate team validation test
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e4f80a0 to
7582f2b
Compare
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/artifacts.py`:
- Around line 72-80: TypeFilter currently uses plain str | None while
TaskIdFilter and CreatedByFilter use NotBlankStr | None; change the annotated
type for TypeFilter to use NotBlankStr from core.types (i.e., TypeFilter =
Annotated[NotBlankStr | None, Parameter(...)]), keeping the same Parameter
metadata (query="type", max_length=QUERY_MAX_LENGTH, description=...) so
optional whitespace-only values are rejected at the API boundary consistently
with TaskIdFilter and CreatedByFilter.
- Around line 216-230: Add INFO-level logging for the artifact deletion state
transition when repo.delete(artifact_id) returns True, e.g., log that the
artifact metadata was deleted along with the artifact_id and any relevant
context (use the existing logger available in scope or state.app_state.logger);
then wrap the call to state.app_state.artifact_storage.delete(artifact_id) in a
try/except and log a WARNING if storage.delete raises or returns a failure
(include artifact_id and error/exception details) so orphaned content is visible
for operators. Ensure logs reference the same unique identifiers used here
(artifact_id, repo.delete, storage.delete) and keep the existing response
behavior unchanged.
- Around line 292-307: The PersistenceError caught after await
repo.save(updated) should be logged with context before being re-raised: inside
the except PersistenceError block log an ERROR or WARNING via logger (including
artifact_id and the exception message) prior to attempting storage.delete and
before re-raising; keep the existing cleanup-exception handling that logs
PERSISTENCE_ARTIFACT_STORAGE_ROLLBACK_FAILED. Also add an INFO log on successful
completion just before returning the Response (e.g., log upload success with
artifact_id and any relevant metadata) so both error and success paths are
logged (referencing repo.save, PersistenceError, storage.delete, logger, and the
existing PERSISTENCE_ARTIFACT_STORAGE_ROLLBACK_FAILED).
- Around line 189-195: The artifact creation path in create handler (after
awaiting state.app_state.persistence.artifacts.save(artifact) and before
returning Response with ApiResponse[Artifact]) lacks an INFO-level log; add an
import for the appropriate event constant (e.g., ARTIFACT_CREATED_EVENT or
create one if missing) and log the state transition using the module logger
(e.g., logger.info or processLogger.info) including the event constant and
identifying artifact fields (id, name or similar) to record the creation
immediately after repo.save(artifact) and before returning the Response.
In `@src/synthorg/persistence/filesystem_artifact_storage.py`:
- Around line 286-301: The _write_file_atomic method currently only calls
f.flush(), which doesn't guarantee data is on disk; after writing and flush()
call os.fsync(f.fileno()) to force the file's data and metadata to disk before
replacing; after tmp_path.replace(file_path) also fsync the artifacts directory
(open self._artifacts_dir as a file descriptor and call os.fsync on it, then
close) to ensure the rename is durable; handle errors as currently done (unlink
tmp_path on exception) and keep using tmp_path and file_path variables as in the
function.
In `@src/synthorg/persistence/sqlite/artifact_repo.py`:
- Around line 107-114: Replace all parenthesized multi-except clauses that use
"except (A, B) as exc:" with the PEP 758 form "except A, B as exc:" for the
sqlite3/Error and aiosqlite/Error cases in this file; specifically change
occurrences that catch "sqlite3.Error" and "aiosqlite.Error" (e.g., the block
that logs PERSISTENCE_ARTIFACT_SAVE_FAILED using logger.exception and raises
QueryError for artifact.id) as well as the other occurrences referenced in the
review so they use the no-parentheses syntax.
In `@tests/unit/api/controllers/test_artifacts.py`:
- Around line 170-172: The test uses the deprecated
asyncio.get_event_loop().run_until_complete(...) pattern around calling
storage.store(artifact_id, payload); replace that call with
asyncio.run(storage.store(artifact_id, payload)) or, alternatively, convert the
test to an async test (e.g., mark the test coroutine with pytest.mark.asyncio
and await storage.store(...)) so you remove get_event_loop().run_until_complete
and use asyncio.run or direct await on storage.store.
In `@tests/unit/api/fakes.py`:
- Around line 413-432: Update the type annotations on FakeArtifactRepository to
match the ArtifactRepository protocol: change parameter and return types in get,
list_artifacts (task_id and created_by), and delete to use NotBlankStr | None
(and NotBlankStr for IDs where appropriate) instead of plain str so signatures
align with ArtifactRepository; ensure imports/reference to NotBlankStr are
present and apply the same pattern to FakeProjectRepository methods (get,
list_projects lead param, delete) for consistency with their protocol.
In `@tests/unit/persistence/sqlite/test_artifact_repo.py`:
- Around line 154-174: The test test_created_at_always_aware contains a
redundant local import of datetime and UTC; remove the inner "from datetime
import UTC, datetime" line in that test so it uses the module-level imports,
leaving the rest of the test (Artifact construction, repo.save, repo.get, and
assertions on created_at) unchanged.
In `@tests/unit/persistence/test_filesystem_artifact_storage.py`:
- Around line 123-145: Add tests that assert absolute paths are rejected
similarly to the existing relative path traversal tests: for the
FileSystemArtifactStorage API methods store, retrieve, delete, and exists (same
test pattern as test_path_traversal_rejected_store/retrieve/delete/exists).
Create new async tests (e.g., test_absolute_path_rejected_store,
test_absolute_path_rejected_retrieve, etc.) that call
storage.store("/etc/passwd", ...), storage.retrieve("/etc/passwd"),
storage.delete("/etc/passwd"), and storage.exists("/etc/passwd") respectively,
and assert they raise ValueError with message matching "Invalid artifact_id";
reuse the same pytest.raises(match=...) pattern so behavior is consistent with
existing tests.
🪄 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: e3f7ca29-766a-49eb-8b52-2582b2e3adbb
📒 Files selected for processing (33)
CLAUDE.mddocs/design/memory.mddocs/design/operations.mddocs/design/page-structure.mdsrc/synthorg/api/app.pysrc/synthorg/api/controllers/artifacts.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/dto.pysrc/synthorg/api/state.pysrc/synthorg/core/artifact.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/artifact_project_repos.pysrc/synthorg/persistence/artifact_storage.pysrc/synthorg/persistence/errors.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/persistence/sqlite/schema.sqltests/unit/api/conftest.pytests/unit/api/controllers/test_artifacts.pytests/unit/api/controllers/test_projects.pytests/unit/api/fakes.pytests/unit/core/test_artifact.pytests/unit/persistence/sqlite/test_artifact_repo.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/persistence/test_artifact_storage_protocol.pytests/unit/persistence/test_filesystem_artifact_storage.pytests/unit/persistence/test_protocol.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue
Files:
tests/unit/api/conftest.pytests/unit/core/test_artifact.pytests/unit/persistence/test_artifact_storage_protocol.pysrc/synthorg/api/app.pytests/unit/persistence/sqlite/test_migrations.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/errors.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/state.pytests/unit/persistence/test_protocol.pytests/unit/persistence/test_filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_storage.pytests/unit/api/controllers/test_projects.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/api/fakes.pysrc/synthorg/api/dto.pytests/unit/api/controllers/test_artifacts.pysrc/synthorg/core/artifact.pysrc/synthorg/api/controllers/projects.pytests/unit/persistence/sqlite/test_artifact_repo.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_project_repos.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/api/controllers/artifacts.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with appropriate markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain minimum 80% test coverage (enforced in CI)
Always include-n autowhen running pytest for parallel test execution with pytest-xdist; never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) — use generic names:example-provider,example-large-001,example-medium-001,example-small-001, ortest-provider,test-small-001for tests
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests mocktime.monotonic()andasyncio.sleep()to make them deterministic
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)— it is cancellation-safe and carries no timing assumptions
Use Hypothesis for property-based testing (@given+@settings); useHYPOTHESIS_PROFILE=devenv var to run 1000 examples in dev profile (default is 50 examples for CI)
Files:
tests/unit/api/conftest.pytests/unit/core/test_artifact.pytests/unit/persistence/test_artifact_storage_protocol.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/test_protocol.pytests/unit/persistence/test_filesystem_artifact_storage.pytests/unit/api/controllers/test_projects.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/api/fakes.pytests/unit/api/controllers/test_artifacts.pytests/unit/persistence/sqlite/test_artifact_repo.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for exception handling — PEP 758 except syntax enforced by ruff
Include type hints for all public functions and use mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones; usecopy.deepcopy()for non-Pydantic internal collections andMappingProxyTypefor read-only enforcement
Use frozen Pydantic models for config/identity and separate mutable-via-copy models for runtime state that evolves; never mix static config fields with mutable runtime fields
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code; prefer structured concurrency over barecreate_task
Keep functions to under 50 lines and files to under 800 lines
Use 88-character line length (enforced by ruff)
Handle errors explicitly and never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic must includefrom synthorg.observability import get_loggerandlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint()for bootstrap)
Use the variable namelogger(not_logger, notlog) for log instances
Always use constants from domain-specific modules undersynthorg.observability.eventsfor event names (import directly...
Files:
src/synthorg/api/app.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/errors.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/state.pysrc/synthorg/persistence/artifact_storage.pysrc/synthorg/api/dto.pysrc/synthorg/core/artifact.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_project_repos.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_repo.pysrc/synthorg/api/controllers/artifacts.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
All provider calls go through
BaseCompletionProviderwhich applies retry and rate limiting automatically; never implement retry logic in driver subclasses or calling code
Files:
src/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/api/dto.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.py
🧠 Learnings (64)
📓 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).
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).
📚 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:
CLAUDE.mdsrc/synthorg/api/app.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/protocol.pydocs/design/memory.mdsrc/synthorg/persistence/artifact_storage.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_project_repos.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/sqlite/project_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/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
CLAUDE.mdsrc/synthorg/api/app.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/protocol.pytests/unit/persistence/test_protocol.pysrc/synthorg/persistence/artifact_storage.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_project_repos.pysrc/synthorg/persistence/repositories.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-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:
CLAUDE.mdsrc/synthorg/api/app.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/__init__.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/artifact_storage.pytests/unit/persistence/sqlite/test_project_repo.pytests/unit/persistence/sqlite/test_artifact_repo.pysrc/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/persistence/artifact_project_repos.pysrc/synthorg/persistence/sqlite/artifact_repo.pysrc/synthorg/persistence/repositories.pysrc/synthorg/persistence/sqlite/project_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: 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:
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-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:
CLAUDE.mdsrc/synthorg/persistence/__init__.pydocs/design/memory.mdsrc/synthorg/persistence/artifact_storage.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:
CLAUDE.mdsrc/synthorg/persistence/protocol.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to 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.mdsrc/synthorg/persistence/repositories.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: 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-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/app.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/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/app.pytests/unit/persistence/test_protocol.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/persistence/repositories.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/synthorg/observability/setup.py : May use stdlib `logging` and `print(..., file=sys.stderr)` for bootstrap and handler-cleanup code that runs before or during logging system configuration
Applied to files:
src/synthorg/api/app.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/persistence.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/__init__.pydocs/design/memory.mdsrc/synthorg/persistence/artifact_storage.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/observability/**/*.py : Observability includes structured logging via `get_logger(__name__)`, correlation tracking, and log sinks.
Applied to files:
src/synthorg/api/state.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use import logging / logging.getLogger() / print() in application code.
Applied to files:
src/synthorg/api/state.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 : Every module with business logic must import `from synthorg.observability import get_logger` and define `logger = get_logger(__name__)`
Applied to files:
src/synthorg/api/state.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 : Every module with business logic MUST have `from synthorg.observability import get_logger` followed by `logger = get_logger(__name__)`.
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic must import logger via `from synthorg.observability import get_logger` and initialize with `logger = get_logger(__name__)`
Applied to files:
src/synthorg/api/state.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 : Every module with business logic must have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/synthorg/api/state.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 : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/persistence/test_protocol.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:
tests/unit/persistence/test_protocol.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Applied to files:
tests/unit/persistence/test_protocol.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.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:
tests/unit/persistence/test_protocol.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
tests/unit/persistence/test_protocol.pysrc/synthorg/api/dto.pysrc/synthorg/core/artifact.pysrc/synthorg/api/controllers/projects.pysrc/synthorg/api/controllers/artifacts.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/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:
tests/unit/persistence/test_protocol.pysrc/synthorg/persistence/repositories.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/api/**/*.py : Authentication uses JWT + API key. Approval gate integration for high-risk operations.
Applied to files:
tests/unit/persistence/test_protocol.pysrc/synthorg/persistence/repositories.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
tests/unit/persistence/test_protocol.pytests/unit/api/fakes.pysrc/synthorg/persistence/repositories.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Update the relevant `docs/design/` page to reflect new reality when approved deviations occur
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/page-structure.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 docs/design/**/*.md : Design specification pages in `docs/design/` must be consulted before implementing features (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue — DESIGN_SPEC.md is a pointer file linking to 7 design pages (Agents, Organization, Communication, Engine, Memory, Operations)
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue. DESIGN_SPEC.md is a pointer file linking to the 7 design pages (index, agents, organization, communication, engine, memory, operations).
Applied to files:
docs/design/page-structure.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:
docs/design/page-structure.md
📚 Learning: 2026-03-31T12:08:03.565Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-03-31T12:08:03.565Z
Learning: Applies to web/src/{components,pages}/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, StaggerItem, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput)
Applied to files:
docs/design/page-structure.md
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/api/dto.pysrc/synthorg/core/artifact.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/api/dto.pysrc/synthorg/core/artifact.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.pysrc/synthorg/api/controllers/artifacts.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/api/dto.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/persistence.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/persistence.pysrc/synthorg/persistence/repositories.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/persistence.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/**/*.py : Keep functions to under 50 lines and files to under 800 lines
Applied to files:
src/synthorg/persistence/repositories.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/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:
src/synthorg/persistence/repositories.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
src/synthorg/persistence/repositories.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
src/synthorg/persistence/repositories.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 : Retryable errors (`is_retryable=True`): `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`. Non-retryable errors raise immediately without retry.
Applied to files:
src/synthorg/persistence/repositories.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-31T12:07:37.364Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.364Z
Learning: Applies to src/**/*.py : Use `except A, B:` (no parentheses) for exception handling — PEP 758 except syntax enforced by ruff
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/**/*.py : Handle errors explicitly and never silently swallow exceptions
Applied to files:
src/synthorg/persistence/sqlite/project_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: 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/persistence/sqlite/project_repo.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/persistence/sqlite/project_repo.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/controllers/artifacts.py
Comment on lines
+107
to
+114
| except (sqlite3.Error, aiosqlite.Error) as exc: | ||
| msg = f"Failed to save artifact {artifact.id!r}" | ||
| logger.exception( | ||
| PERSISTENCE_ARTIFACT_SAVE_FAILED, | ||
| artifact_id=artifact.id, | ||
| error=str(exc), | ||
| ) | ||
| raise QueryError(msg) from exc |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Switch to PEP 758 multi-except syntax (except A, B: without parentheses).
The exception handlers use the parenthesized form, which violates the Python 3.14 style rule enforced by ruff. Apply to all occurrences in this file (lines 107, 134, 149, 201, 207, 231).
As per coding guidelines: "Use except A, B: (no parentheses) for exception handling — PEP 758 except syntax enforced by ruff".
♻️ Proposed fix for line 107
- except (sqlite3.Error, aiosqlite.Error) as exc:
+ except sqlite3.Error, aiosqlite.Error as exc:♻️ Proposed fix for lines 134, 201, 231
- except (sqlite3.Error, aiosqlite.Error) as exc:
+ except sqlite3.Error, aiosqlite.Error as exc:♻️ Proposed fix for lines 149, 207
- except (ValueError, ValidationError, KeyError) as exc:
+ except ValueError, ValidationError, KeyError as exc:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| except (sqlite3.Error, aiosqlite.Error) as exc: | |
| msg = f"Failed to save artifact {artifact.id!r}" | |
| logger.exception( | |
| PERSISTENCE_ARTIFACT_SAVE_FAILED, | |
| artifact_id=artifact.id, | |
| error=str(exc), | |
| ) | |
| raise QueryError(msg) from exc | |
| except sqlite3.Error, aiosqlite.Error as exc: | |
| msg = f"Failed to save artifact {artifact.id!r}" | |
| logger.exception( | |
| PERSISTENCE_ARTIFACT_SAVE_FAILED, | |
| artifact_id=artifact.id, | |
| error=str(exc), | |
| ) | |
| raise QueryError(msg) from exc |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/persistence/sqlite/artifact_repo.py` around lines 107 - 114,
Replace all parenthesized multi-except clauses that use "except (A, B) as exc:"
with the PEP 758 form "except A, B as exc:" for the sqlite3/Error and
aiosqlite/Error cases in this file; specifically change occurrences that catch
"sqlite3.Error" and "aiosqlite.Error" (e.g., the block that logs
PERSISTENCE_ARTIFACT_SAVE_FAILED using logger.exception and raises QueryError
for artifact.id) as well as the other occurrences referenced in the review so
they use the no-parentheses syntax.
- TypeFilter uses NotBlankStr for consistency with other filter params - Add INFO logging on create, delete, and upload success paths - Log PersistenceError before rollback in upload_content - Wrap storage.delete in try/except in delete_artifact handler - Add os.fsync after write + directory fsync (Linux) for durability - Fix test: replace deprecated asyncio.get_event_loop().run_until_complete - Fix test: remove redundant local datetime import - Update FakeArtifactRepository/FakeProjectRepository to use NotBlankStr - Add 4 absolute path traversal rejection tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/artifacts.py`:
- Around line 305-313: The except block handling PersistenceError should not
emit PERSISTENCE_ARTIFACT_STORED; update the logger.warning call in the
PersistenceError handler (the block around repo.save(updated), except
PersistenceError as exc) to use a failure-specific event constant imported from
synthorg.observability.events (e.g. PERSISTENCE_ARTIFACT_STORE_FAILED or the
project’s defined failure constant) instead of PERSISTENCE_ARTIFACT_STORED,
preserve the same context fields (artifact_id, error, note), log at WARNING, and
then re-raise or propagate the exception as before so success remains logged
only with PERSISTENCE_ARTIFACT_STORED.
- Around line 221-233: The handler currently deletes the DB record via
repo.delete(artifact_id) before resolving state.app_state.artifact_storage,
which can raise a 503 and leave metadata removed but blob present; fix by
resolving and invoking the storage delete first: obtain storage =
state.app_state.artifact_storage and call await storage.delete(artifact_id)
(handling storage errors and returning an appropriate error response) before
calling repo.delete(artifact_id); only after storage.delete succeeds should you
call repo.delete and then log with PERSISTENCE_ARTIFACT_DELETED. Ensure you
reference the existing symbols repo.delete, state.app_state.artifact_storage,
storage.delete, and artifact_id when making this reorder and error handling.
In `@src/synthorg/persistence/filesystem_artifact_storage.py`:
- Around line 160-184: store() currently reads and then later assigns
self._cached_total without synchronization, so concurrent calls to store(),
delete(), or the cold-cache path in total_size() can leave _cached_total
inconsistent; introduce an asyncio.Lock (e.g., self._total_lock initialized in
__init__) and acquire it around the capacity check and the final cached update
inside store() (wrap the sequence using current_total = await self.total_size(),
existing_size = ..., new_total check, the write, and self._cached_total = ... in
one locked critical section), and likewise acquire the same lock when any other
method mutates or rewrites _cached_total (notably delete() and the cold-cache
branch in total_size()) so all updates to _cached_total are serialized and
consistent with _max_total_bytes checks.
In `@tests/unit/api/controllers/test_artifacts.py`:
- Around line 166-169: The test seeds the fake storage by writing directly to
storage._store which bypasses FakeArtifactStorage's API and any bookkeeping;
change the test to call the public method (e.g., storage.store(artifact_id,
payload) or the actual public store method on the FakeArtifactStorage instance)
to seed the artifact, remove the direct _store assignment, and adjust for async
if the store method is awaitable so the test uses the storage.store call before
calling test_client.get.
🪄 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: e5eac933-47a2-45a9-86f3-cc2e1e804652
📒 Files selected for processing (6)
src/synthorg/api/controllers/artifacts.pysrc/synthorg/persistence/filesystem_artifact_storage.pytests/unit/api/controllers/test_artifacts.pytests/unit/api/fakes.pytests/unit/persistence/sqlite/test_artifact_repo.pytests/unit/persistence/test_filesystem_artifact_storage.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue
Files:
tests/unit/persistence/test_filesystem_artifact_storage.pytests/unit/persistence/sqlite/test_artifact_repo.pytests/unit/api/controllers/test_artifacts.pysrc/synthorg/persistence/filesystem_artifact_storage.pytests/unit/api/fakes.pysrc/synthorg/api/controllers/artifacts.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with appropriate markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain minimum 80% test coverage (enforced in CI)
Always include-n autowhen running pytest for parallel test execution with pytest-xdist; never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) — use generic names:example-provider,example-large-001,example-medium-001,example-small-001, ortest-provider,test-small-001for tests
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests mocktime.monotonic()andasyncio.sleep()to make them deterministic
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)— it is cancellation-safe and carries no timing assumptions
Use Hypothesis for property-based testing (@given+@settings); useHYPOTHESIS_PROFILE=devenv var to run 1000 examples in dev profile (default is 50 examples for CI)
Files:
tests/unit/persistence/test_filesystem_artifact_storage.pytests/unit/persistence/sqlite/test_artifact_repo.pytests/unit/api/controllers/test_artifacts.pytests/unit/api/fakes.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for exception handling — PEP 758 except syntax enforced by ruff
Include type hints for all public functions and use mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones; usecopy.deepcopy()for non-Pydantic internal collections andMappingProxyTypefor read-only enforcement
Use frozen Pydantic models for config/identity and separate mutable-via-copy models for runtime state that evolves; never mix static config fields with mutable runtime fields
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code; prefer structured concurrency over barecreate_task
Keep functions to under 50 lines and files to under 800 lines
Use 88-character line length (enforced by ruff)
Handle errors explicitly and never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic must includefrom synthorg.observability import get_loggerandlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint()for bootstrap)
Use the variable namelogger(not_logger, notlog) for log instances
Always use constants from domain-specific modules undersynthorg.observability.eventsfor event names (import directly...
Files:
src/synthorg/persistence/filesystem_artifact_storage.pysrc/synthorg/api/controllers/artifacts.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
All provider calls go through
BaseCompletionProviderwhich applies retry and rate limiting automatically; never implement retry logic in driver subclasses or calling code
Files:
src/synthorg/api/controllers/artifacts.py
🧠 Learnings (28)
📓 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).
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).
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to tests/**/*.py : For tasks that must block indefinitely until cancelled, use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)` — it is cancellation-safe and carries no timing assumptions
Applied to files:
tests/unit/api/controllers/test_artifacts.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/filesystem_artifact_storage.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/filesystem_artifact_storage.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:
src/synthorg/persistence/filesystem_artifact_storage.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:
tests/unit/api/fakes.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Applied to files:
tests/unit/api/fakes.pysrc/synthorg/api/controllers/artifacts.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:
tests/unit/api/fakes.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
tests/unit/api/fakes.pysrc/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
tests/unit/api/fakes.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/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/artifacts.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/controllers/artifacts.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/api/controllers/artifacts.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/controllers/artifacts.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/artifacts.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 : DEBUG level logging for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-16T06:30:54.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:30:54.142Z
Learning: Applies to src/synthorg/**/*.py : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log object creation/internal flow/entry-exit at DEBUG
Applied to files:
src/synthorg/api/controllers/artifacts.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 DEBUG logging for object creation, internal flow, and entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/artifacts.py
🔇 Additional comments (3)
tests/unit/api/fakes.py (1)
405-585: Nice contract-faithful test doubles.The new artifact/project repositories plus
FakeArtifactStorageand theFakePersistenceBackend.artifacts/projectsaccessors mirror the production persistence surface closely, which should keep the controller tests exercising the right seams.tests/unit/persistence/sqlite/test_artifact_repo.py (1)
131-172: Good round-trip coverage for the new artifact fields.These checks pin the easy regressions in this mapper:
content_type,size_bytes, and timezone-awarecreated_atsurviving the SQLite round-trip unchanged.tests/unit/persistence/test_filesystem_artifact_storage.py (1)
123-169: Nice hardening coverage for path validation.Covering both traversal and absolute-path inputs across
store(),retrieve(),delete(), andexists()makes the_safe_path()contract much harder to regress.
Comment on lines
+160
to
+184
| current_total = await self.total_size() | ||
| existing_size = await asyncio.to_thread(self._stat_or_zero, file_path) | ||
| new_total = current_total - existing_size + size | ||
| if new_total > self._max_total_bytes: | ||
| msg = ( | ||
| f"Storing artifact {artifact_id!r} ({size} bytes) " | ||
| f"would exceed total limit of " | ||
| f"{self._max_total_bytes} bytes " | ||
| f"(current usage: {current_total} bytes)" | ||
| ) | ||
| logger.warning(PERSISTENCE_ARTIFACT_STORE_FAILED, error=msg) | ||
| raise ArtifactStorageFullError(msg) | ||
|
|
||
| try: | ||
| await asyncio.to_thread(self._write_file_atomic, file_path, content) | ||
| except OSError as exc: | ||
| msg = f"Failed to store artifact {artifact_id!r}" | ||
| logger.exception( | ||
| PERSISTENCE_ARTIFACT_STORE_FAILED, | ||
| artifact_id=artifact_id, | ||
| error=str(exc), | ||
| ) | ||
| raise | ||
| # Update cached total incrementally. | ||
| self._cached_total = current_total - existing_size + size |
There was a problem hiding this comment.
Protect _cached_total with one critical section.
store() assumes the cached total stays stable between the capacity check and the final assignment, but delete() and the cold-cache path in total_size() still rewrite _cached_total outside that lock. A concurrent store() plus delete()/total_size() can leave the cache stale, and the next max_total_bytes check will over- or under-count usage.
🔒 Suggested shape
class FileSystemArtifactStorage:
+ async def _get_total_size_locked(self) -> int:
+ if self._cached_total is None:
+ self._cached_total = await asyncio.to_thread(self._compute_total_size)
+ return self._cached_total
+
async def _check_capacity_and_write(
self,
file_path: Path,
artifact_id: str,
content: bytes,
size: int,
) -> None:
- current_total = await self.total_size()
+ current_total = await self._get_total_size_locked()
existing_size = await asyncio.to_thread(self._stat_or_zero, file_path)
new_total = current_total - existing_size + size
...
async def delete(self, artifact_id: str) -> bool:
file_path = self._safe_path(artifact_id)
- try:
- size, deleted = await asyncio.to_thread(
- self._delete_file_with_size,
- file_path,
- )
- except OSError as exc:
- ...
- raise
- if deleted and self._cached_total is not None:
- self._cached_total = max(0, self._cached_total - size)
+ async with self._write_lock:
+ try:
+ size, deleted = await asyncio.to_thread(
+ self._delete_file_with_size,
+ file_path,
+ )
+ except OSError as exc:
+ ...
+ raise
+ if deleted and self._cached_total is not None:
+ self._cached_total = max(0, self._cached_total - size)
...
async def total_size(self) -> int:
- if self._cached_total is not None:
- return self._cached_total
- total = await asyncio.to_thread(self._compute_total_size)
- self._cached_total = total
- return total
+ async with self._write_lock:
+ return await self._get_total_size_locked()Also applies to: 225-284
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/persistence/filesystem_artifact_storage.py` around lines 160 -
184, store() currently reads and then later assigns self._cached_total without
synchronization, so concurrent calls to store(), delete(), or the cold-cache
path in total_size() can leave _cached_total inconsistent; introduce an
asyncio.Lock (e.g., self._total_lock initialized in __init__) and acquire it
around the capacity check and the final cached update inside store() (wrap the
sequence using current_total = await self.total_size(), existing_size = ...,
new_total check, the write, and self._cached_total = ... in one locked critical
section), and likewise acquire the same lock when any other method mutates or
rewrites _cached_total (notably delete() and the cold-cache branch in
total_size()) so all updates to _cached_total are serialized and consistent with
_max_total_bytes checks.
- Use PERSISTENCE_ARTIFACT_SAVE_FAILED (not _STORED) in PersistenceError handler -- success event must not be emitted on failure - Delete storage content before repo metadata so inconsistency is detectable (metadata present + no blob) rather than invisible (no metadata + orphaned blob) - Test: use asyncio.run(storage.store(...)) instead of internal dict Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/artifacts.py`:
- Around line 310-327: The current flow (storage.store(...), repo.save(updated),
storage.delete(...)) can corrupt previous content on overwrite; change to stage
new content under a temporary key (e.g., call storage.store(temp_key, ...)
instead of storage.store(artifact_id, ...)), then call repo.save(updated) and
only after repo.save succeeds atomically move/rename the temp key to artifact_id
(or copy then delete temp) and clean up; if the storage backend lacks
rename/copy semantics, alternatively read and persist the previous content to a
backup key before writing the new one so you can restore it in the
except-handler (use storage.store/ storage.delete/ storage.get as needed);
update the rollback code around repo.save, storage.delete, and artifact_id to
remove temp keys and/or restore the backup on failure and ensure all cleanup
paths log errors.
In `@tests/unit/api/controllers/test_artifacts.py`:
- Around line 151-198: Add three unit tests in
tests/unit/api/controllers/test_artifacts.py to exercise the PUT
/api/v1/artifacts/{artifact_id}/content endpoint: (1)
test_upload_content_success — create an artifact via test_client.post (same
pattern as test_download_content), then PUT binary payload to
f"/api/v1/artifacts/{artifact_id}/content" with make_auth_headers("ceo"), assert
status_code == 200 and response.data["size_bytes"] equals len(payload) and
storage.store has the bytes; (2) test_upload_content_too_large — create
artifact, set test_client.app.state.app_state.artifact_storage.raise_too_large =
True before the PUT, assert response status_code == 413 and error indicates too
large; (3) test_upload_content_storage_full — create artifact, set
.raise_storage_full = True, PUT and assert status_code == 507 and error
indicates storage full. Use the existing FakeArtifactStorage flags
raise_too_large and raise_storage_full and reset them after each test; reference
functions/objects: test_client, make_auth_headers, FakeArtifactStorage via
test_client.app.state.app_state.artifact_storage, and the PUT endpoint path
"/api/v1/artifacts/{artifact_id}/content".
🪄 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: 8973d4d7-26db-4e92-8c2c-6f73df5c1fb0
📒 Files selected for processing (2)
src/synthorg/api/controllers/artifacts.pytests/unit/api/controllers/test_artifacts.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: Deploy Preview
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue
Files:
tests/unit/api/controllers/test_artifacts.pysrc/synthorg/api/controllers/artifacts.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with appropriate markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain minimum 80% test coverage (enforced in CI)
Always include-n autowhen running pytest for parallel test execution with pytest-xdist; never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) — use generic names:example-provider,example-large-001,example-medium-001,example-small-001, ortest-provider,test-small-001for tests
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests mocktime.monotonic()andasyncio.sleep()to make them deterministic
For tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)— it is cancellation-safe and carries no timing assumptions
Use Hypothesis for property-based testing (@given+@settings); useHYPOTHESIS_PROFILE=devenv var to run 1000 examples in dev profile (default is 50 examples for CI)
Files:
tests/unit/api/controllers/test_artifacts.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for exception handling — PEP 758 except syntax enforced by ruff
Include type hints for all public functions and use mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones; usecopy.deepcopy()for non-Pydantic internal collections andMappingProxyTypefor read-only enforcement
Use frozen Pydantic models for config/identity and separate mutable-via-copy models for runtime state that evolves; never mix static config fields with mutable runtime fields
Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code; prefer structured concurrency over barecreate_task
Keep functions to under 50 lines and files to under 800 lines
Use 88-character line length (enforced by ruff)
Handle errors explicitly and never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic must includefrom synthorg.observability import get_loggerandlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.pyandobservability/sinks.pymay use stdlibloggingandprint()for bootstrap)
Use the variable namelogger(not_logger, notlog) for log instances
Always use constants from domain-specific modules undersynthorg.observability.eventsfor event names (import directly...
Files:
src/synthorg/api/controllers/artifacts.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
All provider calls go through
BaseCompletionProviderwhich applies retry and rate limiting automatically; never implement retry logic in driver subclasses or calling code
Files:
src/synthorg/api/controllers/artifacts.py
🧠 Learnings (23)
📓 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).
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).
📚 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/artifacts.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/controllers/artifacts.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/**/*.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/controllers/artifacts.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/controllers/artifacts.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/controllers/artifacts.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/controllers/artifacts.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/api/controllers/artifacts.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/controllers/artifacts.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/api/controllers/artifacts.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 : DEBUG level logging for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-16T06:30:54.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:30:54.142Z
Learning: Applies to src/synthorg/**/*.py : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log object creation/internal flow/entry-exit at DEBUG
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-31T12:07:37.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T12:07:37.367Z
Learning: Applies to src/**/*.py : Log all state transitions at INFO level
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/controllers/artifacts.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/api/controllers/artifacts.py
🔇 Additional comments (13)
src/synthorg/api/controllers/artifacts.py (7)
1-36: LGTM!Imports are well-organized, proper logger setup with
get_logger(__name__), and event constants are correctly imported from the domain-specific observability module.
38-85: LGTM!The
_SAFE_CONTENT_TYPESallowlist is a good security practice for content downloads. All three filter type aliases correctly useNotBlankStr | Nonefor identifier validation at the API boundary.
94-139: LGTM!The
list_artifactsmethod correctly:
- Uses
require_read_accessguard for the GET endpoint- Validates the
typefilter and returns 400 with a descriptive error for invalid values- Applies pagination to the results
141-168: LGTM!The
get_artifactmethod correctly handles the not-found case with a 404 response and uses appropriate guards.
170-201: LGTM!The
create_artifactmethod:
- Uses
require_write_accessguard- Generates unique IDs with
uuid.uuid4().hex[:12]- Uses UTC-normalized timestamps
- Logs the state transition at INFO level as required
203-248: LGTM!The
delete_artifactmethod correctly:
- Checks artifact existence before attempting deletion
- Deletes storage content first (lines 234-242), so metadata remains if storage fails
- Logs WARNING on storage delete failure
- Logs INFO on successful metadata deletion
- The comment on lines 231-233 accurately documents the ordering rationale
338-389: LGTM!The
download_contentmethod demonstrates good security practices:
- Content-Type allowlist (
_SAFE_CONTENT_TYPES) with fallback toapplication/octet-streamContent-Disposition: attachmentheader prevents inline rendering- Proper JSON error responses with explicit
media_typeoverride- WARNING log when content is missing from storage
tests/unit/api/controllers/test_artifacts.py (6)
11-18: LGTM!The test class is properly marked with
@pytest.mark.unit. Thetest_list_artifacts_emptytest correctly verifies the empty state response structure.
20-49: LGTM!Tests properly verify:
- 404 response for nonexistent artifacts
- Create returns 201 with auto-generated
artifact-prefixed ID- Created artifact is retrievable via GET
51-113: LGTM!Good coverage of listing and filtering:
- Verifies pagination total after multiple creates
- Verifies task_id filter returns correct results
- Verifies invalid type filter returns 400 with descriptive error
115-149: LGTM!Good coverage of deletion and parameter validation:
- Verifies oversized artifact ID is rejected with 400
- Verifies delete returns 200 and artifact is no longer retrievable
- Verifies delete of nonexistent artifact returns 404
151-173: LGTM!The
test_download_contenttest correctly:
- Uses
asyncio.run()instead of deprecatedget_event_loop().run_until_complete()- Seeds storage through the public
storage.store()API- Verifies both content bytes and attachment header
175-198: LGTM!Good edge case coverage:
test_download_content_not_found: Artifact metadata doesn't existtest_download_content_missing_bytes: Metadata exists but storage has no bytes (orphan scenario)Both correctly expect 404 with appropriate error messages.
Comment on lines
+310
to
+327
| try: | ||
| await repo.save(updated) | ||
| except PersistenceError as exc: | ||
| logger.warning( | ||
| PERSISTENCE_ARTIFACT_SAVE_FAILED, | ||
| artifact_id=artifact_id, | ||
| error=str(exc), | ||
| note="metadata save failed, rolling back content", | ||
| ) | ||
| try: | ||
| await storage.delete(artifact_id) | ||
| except Exception as cleanup_exc: | ||
| logger.warning( | ||
| PERSISTENCE_ARTIFACT_STORAGE_ROLLBACK_FAILED, | ||
| artifact_id=artifact_id, | ||
| error=str(cleanup_exc), | ||
| ) | ||
| raise |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Rollback may leave artifact in inconsistent state on overwrite.
If the artifact previously had content, storage.store() (line 288) overwrites it. If repo.save(updated) then fails and the rollback storage.delete() succeeds, the artifact loses its original content. The current approach is pragmatic but leaves a gap for overwrites.
Consider staging new content to a temporary key and renaming on success, or documenting this limitation as accepted behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/artifacts.py` around lines 310 - 327, The
current flow (storage.store(...), repo.save(updated), storage.delete(...)) can
corrupt previous content on overwrite; change to stage new content under a
temporary key (e.g., call storage.store(temp_key, ...) instead of
storage.store(artifact_id, ...)), then call repo.save(updated) and only after
repo.save succeeds atomically move/rename the temp key to artifact_id (or copy
then delete temp) and clean up; if the storage backend lacks rename/copy
semantics, alternatively read and persist the previous content to a backup key
before writing the new one so you can restore it in the except-handler (use
storage.store/ storage.delete/ storage.get as needed); update the rollback code
around repo.save, storage.delete, and artifact_id to remove temp keys and/or
restore the backup on failure and ensure all cleanup paths log errors.
Comment on lines
+151
to
+198
| def test_download_content(self, test_client: TestClient[Any]) -> None: | ||
| """Pre-populate storage via public API, then test download.""" | ||
| import asyncio | ||
|
|
||
| create_resp = test_client.post( | ||
| "/api/v1/artifacts", | ||
| json={ | ||
| "type": "code", | ||
| "path": "src/dl.py", | ||
| "task_id": "task-dl", | ||
| "created_by": "agent-1", | ||
| "content_type": "text/plain", | ||
| }, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| artifact_id = create_resp.json()["data"]["id"] | ||
| payload = b"hello world" | ||
| storage = test_client.app.state.app_state.artifact_storage | ||
| asyncio.run(storage.store(artifact_id, payload)) | ||
| dl_resp = test_client.get(f"/api/v1/artifacts/{artifact_id}/content") | ||
| assert dl_resp.status_code == 200 | ||
| assert dl_resp.content == payload | ||
| assert "attachment" in dl_resp.headers.get("content-disposition", "") | ||
|
|
||
| def test_download_content_not_found(self, test_client: TestClient[Any]) -> None: | ||
| resp = test_client.get("/api/v1/artifacts/nonexistent/content") | ||
| assert resp.status_code == 404 | ||
| body = resp.json() | ||
| assert body["success"] is False | ||
|
|
||
| def test_download_content_missing_bytes(self, test_client: TestClient[Any]) -> None: | ||
| """Artifact exists in DB but content not in storage.""" | ||
| create_resp = test_client.post( | ||
| "/api/v1/artifacts", | ||
| json={ | ||
| "type": "code", | ||
| "path": "src/ghost.py", | ||
| "task_id": "task-ghost", | ||
| "created_by": "agent-1", | ||
| }, | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| artifact_id = create_resp.json()["data"]["id"] | ||
| # Do not upload content -- storage has no bytes. | ||
| resp = test_client.get(f"/api/v1/artifacts/{artifact_id}/content") | ||
| assert resp.status_code == 404 | ||
| body = resp.json() | ||
| assert "content not found" in body["error"].lower() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if upload_content tests exist elsewhere
rg -n "upload_content|PUT.*content" --type=py tests/Repository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Find the artifacts controller/endpoint implementation
fd -t f "artifact" --type f --path "*/api/*" | grep -E "(controller|handler|route)" | head -20Repository: Aureliolo/synthorg
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Search for upload_content or PUT endpoint handling in the codebase
rg -l "upload_content" --type=py src/Repository: Aureliolo/synthorg
Length of output: 103
🏁 Script executed:
#!/bin/bash
# Look for PUT endpoint definition in artifacts-related files
rg -n "def.*PUT|router\.put|@.*\.put" --type=py src/ | grep -i artifactRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check FakeArtifactStorage in fakes.py
fd -t f "fakes.py" tests/ | xargs cat -n 2>/dev/null | head -200Repository: Aureliolo/synthorg
Length of output: 7884
🏁 Script executed:
#!/bin/bash
# Search for FakeArtifactStorage definition
rg -n "class FakeArtifactStorage" --type=py tests/Repository: Aureliolo/synthorg
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Check the artifacts controller to see if upload_content endpoint exists
cat -n src/synthorg/api/controllers/artifacts.py | head -150Repository: Aureliolo/synthorg
Length of output: 5628
🏁 Script executed:
#!/bin/bash
# Search for all PUT endpoints in the artifacts controller
rg -n "def.*put|@.*\.put|router\.put" --type=py src/synthorg/api/controllers/artifacts.pyRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Look for more of the artifacts controller, especially PUT endpoints
cat -n src/synthorg/api/controllers/artifacts.py | sed -n '140,250p'Repository: Aureliolo/synthorg
Length of output: 4324
🏁 Script executed:
#!/antml:bin/bash
# Search for upload or content-related methods
rg -n "upload|content" --type=py src/synthorg/api/controllers/artifacts.pyRepository: Aureliolo/synthorg
Length of output: 1707
🏁 Script executed:
#!/bin/bash
# Check FakeArtifactStorage for the flags mentioned in review
sed -n '466,550p' tests/unit/api/fakes.pyRepository: Aureliolo/synthorg
Length of output: 3223
🏁 Script executed:
#!/bin/bash
# Check the full test_artifacts.py file to see all tests
wc -l tests/unit/api/controllers/test_artifacts.pyRepository: Aureliolo/synthorg
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Search for upload_content tests in the test file
rg -n "test_upload|def test_" tests/unit/api/controllers/test_artifacts.py | head -30Repository: Aureliolo/synthorg
Length of output: 1027
🏁 Script executed:
#!/bin/bash
# Get the full upload_content endpoint implementation to understand what it does
sed -n '250,340p' src/synthorg/api/controllers/artifacts.pyRepository: Aureliolo/synthorg
Length of output: 2912
🏁 Script executed:
#!/bin/bash
# Double-check: search entire test directory for any upload_content or PUT content tests
rg -n "upload_content" tests/ --type=pyRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Get a list of all test methods in test_artifacts.py to confirm coverage
grep -n "def test_" tests/unit/api/controllers/test_artifacts.pyRepository: Aureliolo/synthorg
Length of output: 1027
🏁 Script executed:
#!/bin/bash
# Check the end of the test file to see if there's anything after line 198
tail -20 tests/unit/api/controllers/test_artifacts.pyRepository: Aureliolo/synthorg
Length of output: 885
Add tests for PUT /{artifact_id}/content endpoint to cover upload scenarios.
The upload_content endpoint exists but has no test coverage for:
- Successful upload (200 with updated
size_bytes) - 413 response when
ArtifactTooLargeErroris raised - 507 response when
ArtifactStorageFullErroris raised
FakeArtifactStorage provides raise_too_large and raise_storage_full flags to inject these errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/api/controllers/test_artifacts.py` around lines 151 - 198, Add
three unit tests in tests/unit/api/controllers/test_artifacts.py to exercise the
PUT /api/v1/artifacts/{artifact_id}/content endpoint: (1)
test_upload_content_success — create an artifact via test_client.post (same
pattern as test_download_content), then PUT binary payload to
f"/api/v1/artifacts/{artifact_id}/content" with make_auth_headers("ceo"), assert
status_code == 200 and response.data["size_bytes"] equals len(payload) and
storage.store has the bytes; (2) test_upload_content_too_large — create
artifact, set test_client.app.state.app_state.artifact_storage.raise_too_large =
True before the PUT, assert response status_code == 413 and error indicates too
large; (3) test_upload_content_storage_full — create artifact, set
.raise_storage_full = True, PUT and assert status_code == 507 and error
indicates storage full. Use the existing FakeArtifactStorage flags
raise_too_large and raise_storage_full and reset them after each test; reference
functions/objects: test_client, make_auth_headers, FakeArtifactStorage via
test_client.app.state.app_state.artifact_storage, and the PUT endpoint path
"/api/v1/artifacts/{artifact_id}/content".
Aureliolo
added a commit
that referenced
this pull request
Mar 31, 2026
🤖 I have created a release *beep* *boop* --- ## [0.5.3](v0.5.2...v0.5.3) (2026-03-31) ### Features * implement artifact and project persistence ([#947](#947)) ([6dea87a](6dea87a)) ### Maintenance * add allow_inf_nan=False to all remaining ConfigDict declarations ([#943](#943)) ([cd7bbca](cd7bbca)) * audit full web dashboard for hardcoded design token violations ([#944](#944)) ([a1322cd](a1322cd)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ArtifactStorageBackendprotocol withFileSystemArtifactStorage(stores content as files under<data_dir>/artifacts/<id>, 50 MB per artifact / 1 GB total limits)ArtifactControllerandProjectControllerwith full CRUD endpoints (list/get/create + content upload/download for artifacts)Artifactmodel withcontent_typeandsize_bytesfields for content storage metadataAppStateandcreate_app(auto-wired fromSYNTHORG_DB_PATHparent directory)CreateArtifactRequestandCreateProjectRequestDTOs with validationReview coverage
Pre-reviewed by 6 agents (docs-consistency, conventions-enforcer, persistence-reviewer, issue-resolution-verifier, silent-failure-hunter, type-design-analyzer). 15 findings addressed in a dedicated fix commit:
FileSystemArtifactStorage(validates artifact_id stays within bounds)asyncio.Lockon storage capacity checkspersistence/__init__.pyTest plan
uv run python -m pytest tests/unit/persistence/sqlite/test_artifact_repo.py -n auto-- 14 tests (CRUD, filters, roundtrips)uv run python -m pytest tests/unit/persistence/sqlite/test_project_repo.py -n auto-- 12 tests (CRUD, filters, JSON tuple roundtrips)uv run python -m pytest tests/unit/persistence/test_filesystem_artifact_storage.py -n auto-- 18 tests (store/retrieve/delete, size limits, path traversal)uv run python -m pytest tests/unit/api/controllers/test_artifacts.py tests/unit/api/controllers/test_projects.py -n auto-- 13 tests (API integration)uv run python -m pytest tests/unit/persistence/test_protocol.py -n auto-- protocol complianceuv run python -m pytest tests/ -n auto-- full suite passes (11,317 tests)uv run mypy src/ tests/-- zero issuesuv run ruff check src/ tests/-- zero issuesCloses #612
🤖 Generated with Claude Code