Skip to content

feat: implement AgentStateRepository for runtime state persistence#459

Merged
Aureliolo merged 7 commits intomainfrom
feat/agent-state-repository
Mar 15, 2026
Merged

feat: implement AgentStateRepository for runtime state persistence#459
Aureliolo merged 7 commits intomainfrom
feat/agent-state-repository

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Summary

  • Add ExecutionStatus enum (idle/executing/paused) to core/enums.py
  • Add AgentRuntimeState frozen Pydantic model with status invariant validation, idle() and from_context() factories
  • Add AgentStateRepository protocol (save/get/get_active/delete) to persistence layer
  • Add SQLiteAgentStateRepository with v8 migration (agent_states table + status index)
  • Add 10 persistence event constants for structured logging
  • Wire through PersistenceBackend protocol, SQLitePersistenceBackend, and all re-exports
  • Update FakePersistenceBackend in API test fixtures with properly-filtered get_active()
  • Update CLAUDE.md package structure and logging event documentation

Test plan

  • 41 new tests (23 model + 15 repo + 3 review-driven additions)
  • Protocol compliance test (isinstance check)
  • Backend before-connect guard test
  • V8 migration table + index creation tests
  • Schema version constant updated in all test files
  • uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80 — 8042 passed, 94.52% coverage
  • uv run ruff check src/ tests/ — clean
  • uv run mypy src/ tests/ — clean

Review coverage

Pre-reviewed by 12 agents, 10 findings addressed:

  • Parameterized ExecutionStatus.IDLE.value in SQL instead of hardcoded string
  • ValidationError in test_frozen instead of bare Exception
  • FakeAgentStateRepository.get_active() properly filters by status
  • Parametrized idle-violation tests
  • Added idle(" "), from_context(cost_usd=0.0), and get_active corrupt-row tests
  • CLAUDE.md package structure + logging events updated

Closes #261

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 15, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Track per-agent runtime status (IDLE/EXECUTING/PAUSED) with timestamps, turn counts and costs; create/query idle and active states.
  • Persistence

    • Persist and expose agent runtime state for dashboards and graceful shutdown; SQLite-backed repository and migration added.
  • Observability

    • New agent-state persistence events and clarified logging guidance.
  • Documentation

    • Design docs describing agent execution status and runtime-state.
  • Tests

    • Extensive unit tests for model, persistence, migrations and backend behavior.

Walkthrough

Adds ExecutionStatus enum and a frozen AgentRuntimeState Pydantic model with validation and factories; defines AgentStateRepository protocol and a SQLiteAgentStateRepository implementation; integrates the repository into the SQLite backend; adds migration v8 to create agent_states table; surface observability events, tests, and in-memory test fakes.

Changes

Cohort / File(s) Summary
Core enum & runtime model
src/synthorg/core/enums.py, src/synthorg/engine/agent_state.py, src/synthorg/engine/__init__.py
Adds ExecutionStatus enum and frozen AgentRuntimeState (fields, validation, idle and from_context factories); exports AgentRuntimeState from engine package.
Persistence protocol & public API
src/synthorg/persistence/repositories.py, src/synthorg/persistence/protocol.py, src/synthorg/persistence/__init__.py
Introduces AgentStateRepository protocol with save/get/get_active/delete; adds agent_states property to PersistenceBackend protocol; exposes repository in persistence package exports.
SQLite implementation & backend wiring
src/synthorg/persistence/sqlite/agent_state_repo.py, src/synthorg/persistence/sqlite/backend.py, src/synthorg/persistence/sqlite/__init__.py
Implements SQLiteAgentStateRepository (aiosqlite-backed) with upsert/select/delete, row→model deserialization + validation, QueryError handling, and observability event logging; wires repository into SQLitePersistenceBackend and exposes it.
DB schema & migrations
src/synthorg/persistence/sqlite/migrations.py
Bumps SCHEMA_VERSION to 8 and adds migration v8 to create agent_states table with CHECK constraints and composite index idx_as_status_activity.
Observability events
src/synthorg/observability/events/persistence.py
Adds persistence event constants under persistence.agent_state.* for saved, save_failed, fetched, fetch_failed, not_found, active_queried, active_query_failed, deleted, delete_failed, deserialize_failed.
Tests — model, sqlite, migrations
tests/unit/engine/test_agent_state.py, tests/unit/persistence/sqlite/test_agent_state_repo.py, tests/unit/persistence/sqlite/test_backend.py, tests/unit/persistence/sqlite/test_migrations*.py, tests/unit/persistence/test_migrations_v2.py
Adds comprehensive unit tests for AgentRuntimeState invariants and factories, extensive SQLite repo tests (round-trip, upsert, ordering, deserialization errors), backend pre-connect behavior, and migration v8 assertions; updates schema-version tests to 8.
Test fakes & conftest
tests/unit/api/fakes.py, tests/unit/api/conftest.py, tests/unit/persistence/test_protocol.py
Introduces in-memory fake repositories including FakeAgentStateRepository and FakePersistenceBackend; centralizes test fakes and updates conftest to import them; adds protocol test ensuring _FakeAgentStateRepository conforms to AgentStateRepository.
Docs & misc
docs/design/agents.md, docs/design/engine.md, docs/design/memory.md, CLAUDE.md
Documents AgentRuntimeState, ExecutionStatus, persistence integration and logging guidance; updates design docs to reference the new runtime-state entity and persistence surface.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Repo as SQLiteAgentStateRepository
    participant DB as SQLite DB
    participant Logger

    Client->>Repo: save(state: AgentRuntimeState)
    Repo->>DB: INSERT OR REPLACE (json)
    alt DB OK
        DB-->>Repo: OK
        Repo->>Logger: log "persistence.agent_state.saved"
        Repo-->>Client: None
    else DB Error
        DB-->>Repo: Error
        Repo->>Logger: log "persistence.agent_state.save_failed"
        Repo-->>Client: raise QueryError
    end

    Client->>Repo: get(agent_id)
    Repo->>DB: SELECT by agent_id
    alt Found
        DB-->>Repo: row
        Repo->>Repo: _row_to_model()
        alt Deserialization OK
            Repo->>Logger: log "persistence.agent_state.fetched"
            Repo-->>Client: AgentRuntimeState
        else Validation Error
            Repo->>Logger: log "persistence.agent_state.deserialize_failed"
            Repo-->>Client: raise QueryError
        end
    else Not Found
        DB-->>Repo: empty
        Repo->>Logger: log "persistence.agent_state.not_found"
        Repo-->>Client: None
    end

    Client->>Repo: get_active()
    Repo->>DB: SELECT WHERE status != 'IDLE' ORDER BY last_activity_at DESC
    alt DB OK
        DB-->>Repo: rows[]
        Repo->>Logger: log "persistence.agent_state.active_queried"
        Repo-->>Client: tuple[AgentRuntimeState,...]
    else DB Error
        DB-->>Repo: Error
        Repo->>Logger: log "persistence.agent_state.active_query_failed"
        Repo-->>Client: raise QueryError
    end

    Client->>Repo: delete(agent_id)
    Repo->>DB: DELETE by agent_id
    alt DB OK
        DB-->>Repo: rows_affected
        alt Deleted
            Repo->>Logger: log "persistence.agent_state.deleted"
            Repo-->>Client: True
        else Not Found
            Repo->>Logger: log "persistence.agent_state.not_found"
            Repo-->>Client: False
        end
    else DB Error
        DB-->>Repo: Error
        Repo->>Logger: log "persistence.agent_state.delete_failed"
        Repo-->>Client: raise QueryError
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: implementing AgentStateRepository for runtime state persistence, which is the primary objective.
Description check ✅ Passed The description comprehensively covers the changeset, detailing ExecutionStatus enum, AgentRuntimeState model, repository protocol, SQLite implementation, migrations, event constants, test coverage, and validation results.
Linked Issues check ✅ Passed The PR meets all requirements from issue #261: AgentStateRepository protocol defined [#261], SQLite implementation with migration [#261], queries by agent_id and active agents [#261], integration with persistence layer [#261], and support for checkpoint recovery [#261].
Out of Scope Changes check ✅ Passed All changes are within scope of #261. The PR includes ExecutionStatus enum, AgentRuntimeState model, repository protocol, SQLite implementation, event constants, persistence layer integration, test fixtures updates, and documentation—all directly aligned with the AgentStateRepository feature objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/agent-state-repository
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/agent-state-repository
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a robust system for managing and persisting the runtime state of agents. By defining a clear data model for agent execution status and implementing a dedicated repository for its storage, the system gains enhanced capabilities for monitoring agent activity, facilitating graceful shutdowns, and improving recovery mechanisms. This foundational change ensures that agent states are reliably tracked and accessible across operations, contributing to the overall stability and observability of the agent ecosystem.

Highlights

  • Agent Runtime State Model: Introduced a new ExecutionStatus enum (idle, executing, paused) and a frozen Pydantic model AgentRuntimeState to capture an agent's current execution status, including associated identifiers, cost, and turn count. This model includes status-invariant validation and factory methods for creating idle states or states from an AgentContext.
  • Agent State Persistence: Implemented an AgentStateRepository protocol for CRUD operations on AgentRuntimeState objects. A concrete SQLiteAgentStateRepository was added, providing persistence for agent states in a SQLite database.
  • Database Migration: The SQLite schema version was incremented to 8, introducing a new agent_states table with a status index to store the AgentRuntimeState data.
  • Observability and Integration: Added 10 new persistence event constants for structured logging related to agent state operations. The PersistenceBackend protocol and SQLitePersistenceBackend were updated to include the new AgentStateRepository.
  • Testing and Documentation: Comprehensive unit tests were added for the AgentRuntimeState model and the SQLiteAgentStateRepository. The FakePersistenceBackend used in API test fixtures was updated to correctly filter active agent states, and the CLAUDE.md documentation was updated to reflect the new package structure and logging events.
Changelog
  • CLAUDE.md
    • Updated the description for the engine/ directory to include agent runtime state.
    • Expanded the logging events documentation to include new persistence-related constants for agent states.
  • src/synthorg/core/enums.py
    • Added ExecutionStatus enum with IDLE, EXECUTING, and PAUSED members.
  • src/synthorg/engine/init.py
    • Imported and re-exported AgentRuntimeState.
  • src/synthorg/engine/agent_state.py
    • Added AgentRuntimeState Pydantic model for lightweight agent execution state.
    • Implemented _idle_violations and _validate_status_invariants for status-dependent validation.
    • Provided idle() and from_context() class methods for state creation.
  • src/synthorg/observability/events/persistence.py
    • Added 10 new PERSISTENCE_AGENT_STATE event constants for logging agent state operations.
  • src/synthorg/persistence/init.py
    • Imported and re-exported AgentStateRepository.
  • src/synthorg/persistence/protocol.py
    • Imported AgentStateRepository.
    • Added agent_states property to the PersistenceBackend protocol.
  • src/synthorg/persistence/repositories.py
    • Imported AgentRuntimeState.
    • Added AgentStateRepository protocol defining save, get, get_active, and delete methods.
    • Included AgentStateRepository in the module's __all__ export list.
  • src/synthorg/persistence/sqlite/init.py
    • Imported and re-exported SQLiteAgentStateRepository.
  • src/synthorg/persistence/sqlite/agent_state_repo.py
    • Added SQLiteAgentStateRepository for persisting AgentRuntimeState objects to SQLite.
    • Implemented save, get, get_active, and delete methods with error handling and logging.
    • Included a private helper _row_to_model for deserializing database rows to AgentRuntimeState.
  • src/synthorg/persistence/sqlite/backend.py
    • Imported SQLiteAgentStateRepository.
    • Initialized and cleared _agent_states repository in __init__ and _clear_state.
    • Instantiated SQLiteAgentStateRepository in _create_repositories.
    • Added agent_states property to SQLitePersistenceBackend.
  • src/synthorg/persistence/sqlite/migrations.py
    • Incremented SCHEMA_VERSION to 8.
    • Defined _V8_STATEMENTS to create the agent_states table and idx_as_status index.
    • Added _apply_v8 function and included it in the _MIGRATIONS list.
  • tests/unit/api/conftest.py
    • Imported AgentRuntimeState.
    • Added FakeAgentStateRepository for in-memory testing of agent state persistence.
    • Integrated FakeAgentStateRepository into FakePersistenceBackend.
  • tests/unit/engine/test_agent_state.py
    • Added unit tests for AgentRuntimeState model, covering factory methods, validation, and immutability.
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
    • Added unit tests for SQLiteAgentStateRepository, verifying CRUD operations, active state retrieval, and error handling.
  • tests/unit/persistence/sqlite/test_backend.py
    • Added a test to ensure agent_states property raises PersistenceConnectionError when the backend is not connected.
  • tests/unit/persistence/sqlite/test_migrations.py
    • Added tests for the v8 migration, confirming the creation of the agent_states table and its index.
  • tests/unit/persistence/sqlite/test_migrations_v6.py
    • Updated the schema version assertion to 8.
  • tests/unit/persistence/test_migrations_v2.py
    • Updated the schema version assertion to 8.
    • Updated the get_user_version assertion to 8 after migrations.
  • tests/unit/persistence/test_protocol.py
    • Imported AgentStateRepository and AgentRuntimeState.
    • Added _FakeAgentStateRepository and integrated it into _FakeBackend.
    • Added a test to confirm _FakeAgentStateRepository implements AgentStateRepository protocol.
Activity
  • The pull request includes 41 new tests: 23 for the AgentRuntimeState model, 15 for the SQLiteAgentStateRepository, and 3 additional tests driven by review feedback.
  • Protocol compliance for the new AgentStateRepository has been verified with an isinstance check.
  • A test was added to ensure the persistence backend correctly guards against repository access before connection.
  • Tests confirm the successful creation of the agent_states table and its index during the v8 migration.
  • The schema version constant has been updated across all relevant test files.
  • All unit tests passed with uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80, achieving 94.52% coverage.
  • Code quality checks passed with uv run ruff check src/ tests/, indicating a clean codebase.
  • Type checks passed with uv run mypy src/ tests/, ensuring type safety.
  • The changes underwent pre-review by 12 agents, leading to 10 addressed findings, including parameterizing ExecutionStatus.IDLE.value in SQL, using ValidationError in tests, correctly filtering FakeAgentStateRepository.get_active(), adding parametrized idle-violation tests, and updating CLAUDE.md documentation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 18:34 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a robust mechanism for persisting agent runtime state, a key feature for observability and recovery. The changes are well-structured, including a new AgentRuntimeState model, a repository protocol with a corresponding SQLite implementation, and the necessary database migration. The new components are cleanly integrated into the existing persistence layer and are supported by a comprehensive suite of unit tests. I have one suggestion to enhance the resilience of the get_active method against potential data corruption, ensuring that a single invalid record doesn't disrupt the entire query.

Comment on lines +122 to +127
states = tuple(self._row_to_model(dict(row)) for row in rows)
logger.debug(
PERSISTENCE_AGENT_STATE_ACTIVE_QUERIED,
count=len(states),
)
return states
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of get_active will fail completely if any single row from the database fails to deserialize into an AgentRuntimeState model. While this is a 'fail-fast' approach, it could reduce the system's resilience, especially for features like dashboards that rely on this data. A single corrupt row could prevent all active agent states from being displayed.

To improve robustness, I suggest handling deserialization errors on a per-row basis. By catching the QueryError from _row_to_model within the loop, you can log the issue for the specific corrupt row and continue processing the rest, ensuring that valid data is still returned.

        states: list[AgentRuntimeState] = []
        for row in rows:
            try:
                states.append(self._row_to_model(dict(row)))
            except QueryError:
                # _row_to_model logs the deserialization error. Skip corrupt row.
                continue
        logger.debug(
            PERSISTENCE_AGENT_STATE_ACTIVE_QUERIED,
            count=len(states),
        )
        return tuple(states)

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/engine/agent_state.py`:
- Line 104: The idle() classmethod currently types its agent_id param as str;
change its signature to use NotBlankStr from core.types (i.e., def idle(cls,
agent_id: NotBlankStr) -> AgentRuntimeState) so the public identifier follows
the project-wide identifier typing standard; update the import if missing
(import NotBlankStr from core.types) and adjust any callers/tests that relied on
a plain str as needed.

In `@src/synthorg/persistence/sqlite/agent_state_repo.py`:
- Around line 63-67: The log calls for state transition events
PERSISTENCE_AGENT_STATE_SAVED and the corresponding delete event are currently
using logger.debug; change them to logger.info so all agent state transitions
are logged at INFO level. Locate the calls in agent_state_repo.py that invoke
logger.debug(...) with PERSISTENCE_AGENT_STATE_SAVED (and the delete variant)
and replace logger.debug with logger.info while preserving the same message keys
and structured fields (e.g., agent_id=state.agent_id, status=state.status.value
or agent_id=agent_id) so the events remain identical except for the log level.
- Line 55: Replace the legacy parenthesized multi-exception handlers in this
file: change occurrences of "except (sqlite3.Error, aiosqlite.Error) as exc"
(found at the four locations noted) to the new PEP 758 comma-separated form
"except sqlite3.Error, aiosqlite.Error as exc" so ruff/Python 3.14 accepts it;
update all four instances (the handlers around the DB access/error-logging code)
to use the comma-separated exception list and keep the same exception variable
name and error handling logic.

In `@src/synthorg/persistence/sqlite/migrations.py`:
- Around line 410-419: The CREATE TABLE for agent_states permits invalid runtime
values; update the DDL in migrations.py where the CREATE TABLE IF NOT EXISTS
agent_states string is defined to add CHECK constraints: ensure turn_count >= 0
and accumulated_cost_usd >= 0.0, and restrict status to the canonical set used
by AgentRuntimeState (replace the status CHECK with status IN (...same enum
values used by AgentRuntimeState/AgentRuntimeState.status enum/constant) so
DB-level valid values match the app), keeping last_activity_at NOT NULL; apply
these CHECK clauses on agent_id/existing columns in that CREATE TABLE statement.

In `@tests/unit/api/conftest.py`:
- Around line 410-415: get_active currently returns active AgentRuntimeState
items in dict insertion order; change it to filter out ExecutionStatus.IDLE then
sort the resulting sequence by each state's last_activity_at in descending order
before converting to a tuple so it matches the repository contract; update the
body of get_active (function name: get_active) to perform filtering by s.status
!= ExecutionStatus.IDLE and then sort by s.last_activity_at (or equivalent
timestamp attribute) reverse=True prior to returning the tuple.

In `@tests/unit/persistence/sqlite/test_agent_state_repo.py`:
- Around line 201-237: Consolidate the four repetitive tests into one
parametrized pytest that iterates over the operation names and expected error
messages; create a single test (e.g.,
test_repo_methods_raise_query_error_on_db_error) decorated with
`@pytest.mark.parametrize` supplying tuples like ("save", ("state",), "Failed to
save"), ("get", ("agent-001",), "Failed to fetch"), ("get_active", (), "Failed
to query"), ("delete", ("agent-001",), "Failed to delete"); inside the test
instantiate SQLiteAgentStateRepository(memory_db), import QueryError,
dynamically call the repository method by name (or use conditional branching)
with the provided args, and assert with pytest.raises(QueryError,
match=expected_message); reference SQLiteAgentStateRepository, QueryError, and
memory_db to locate code to change.

In `@tests/unit/persistence/test_migrations_v2.py`:
- Line 125: Replace the hardcoded schema version in the assertion with the
shared SCHEMA_VERSION constant so the test adapts to future schema bumps; update
the assertion in tests/unit/persistence/test_migrations_v2.py to compare
get_user_version(memory_db) against SCHEMA_VERSION (import SCHEMA_VERSION from
the module that defines it) instead of the literal 8, keeping the use of
get_user_version and memory_db intact.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b03a9c07-29d0-4c3e-bed8-7d7d5b1603a2

📥 Commits

Reviewing files that changed from the base of the PR and between 2feed09 and 757904b.

📒 Files selected for processing (20)
  • CLAUDE.md
  • src/synthorg/core/enums.py
  • src/synthorg/engine/__init__.py
  • src/synthorg/engine/agent_state.py
  • src/synthorg/observability/events/persistence.py
  • src/synthorg/persistence/__init__.py
  • src/synthorg/persistence/protocol.py
  • src/synthorg/persistence/repositories.py
  • src/synthorg/persistence/sqlite/__init__.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/persistence/sqlite/backend.py
  • src/synthorg/persistence/sqlite/migrations.py
  • tests/unit/api/conftest.py
  • tests/unit/engine/test_agent_state.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
  • tests/unit/persistence/sqlite/test_backend.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/persistence/sqlite/test_migrations_v6.py
  • tests/unit/persistence/test_migrations_v2.py
  • tests/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). (3)
  • GitHub Check: Build Backend
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations
Use except A, B: syntax (no parentheses) instead of except (A, B): — PEP 758 syntax enforced by ruff on Python 3.14
All public functions must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Use NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Use @computed_field in Pydantic models for derived values instead of storing and validating redundant fields
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) instead of bare create_task
Line length: 88 characters (enforced by ruff)
Functions must be < 50 lines; files must be < 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate input at system boundaries (user input, external APIs, config files)
Create new objects instead of mutating existing ones (immutability principle)

Files:

  • src/synthorg/persistence/sqlite/migrations.py
  • tests/unit/engine/test_agent_state.py
  • src/synthorg/persistence/__init__.py
  • src/synthorg/observability/events/persistence.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • src/synthorg/persistence/sqlite/__init__.py
  • src/synthorg/persistence/repositories.py
  • src/synthorg/persistence/protocol.py
  • tests/unit/persistence/sqlite/test_backend.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/engine/__init__.py
  • src/synthorg/persistence/sqlite/backend.py
  • tests/unit/persistence/sqlite/test_migrations_v6.py
  • tests/unit/persistence/test_migrations_v2.py
  • src/synthorg/engine/agent_state.py
  • src/synthorg/core/enums.py
  • tests/unit/persistence/test_protocol.py
  • tests/unit/api/conftest.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every module with business logic must import from synthorg.observability import get_logger and define logger = get_logger(__name__)
Never use import logging, logging.getLogger(), or print() in application code — use the synthorg logger instead
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
Always log with structured kwargs: logger.info(EVENT, key=value) — never use old-style formatting logger.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
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Never implement retry logic in driver subclasses or calling code — all provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically
Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig; retryable errors are RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError
Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state
For dict/list fields in frozen Pydantic models, use copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use Pydantic v2 conventions: BaseModel, model_validator, computed_field, ConfigDict

Files:

  • src/synthorg/persistence/sqlite/migrations.py
  • src/synthorg/persistence/__init__.py
  • src/synthorg/observability/events/persistence.py
  • src/synthorg/persistence/sqlite/__init__.py
  • src/synthorg/persistence/repositories.py
  • src/synthorg/persistence/protocol.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/engine/__init__.py
  • src/synthorg/persistence/sqlite/backend.py
  • src/synthorg/engine/agent_state.py
  • src/synthorg/core/enums.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, @pytest.mark.slow to categorize tests
Maintain 80% minimum code coverage (enforced in CI)
Each test must complete within 30 seconds (timeout enforcement)
Always include -n auto when running pytest via uv run python -m pytest — never run tests sequentially (pytest-xdist parallelism)
Prefer @pytest.mark.parametrize for testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like example-provider, example-large-001, test-provider, test-small-001, or size aliases (large/medium/small)
Use Hypothesis for property-based testing with @given + @settings decorators; control profiles via HYPOTHESIS_PROFILE env var (ci for 200 examples, dev for 1000 examples)

Files:

  • tests/unit/engine/test_agent_state.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/persistence/sqlite/test_backend.py
  • tests/unit/persistence/sqlite/test_migrations_v6.py
  • tests/unit/persistence/test_migrations_v2.py
  • tests/unit/persistence/test_protocol.py
  • tests/unit/api/conftest.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
🧠 Learnings (14)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.

Applied to files:

  • tests/unit/engine/test_agent_state.py
  • src/synthorg/engine/agent_state.py
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state

Applied to files:

  • tests/unit/engine/test_agent_state.py
  • src/synthorg/engine/agent_state.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
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
  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
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
  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
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:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Always log with structured kwargs: `logger.info(EVENT, key=value)` — never use old-style formatting `logger.info("msg %s", val)`

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
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:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging`, `logging.getLogger()`, or `print()` in application code — use the synthorg logger instead

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).

Applied to files:

  • tests/unit/persistence/sqlite/test_migrations_v6.py
🧬 Code graph analysis (12)
tests/unit/engine/test_agent_state.py (2)
src/synthorg/core/enums.py (2)
  • ExecutionStatus (505-515)
  • TaskType (227-235)
src/synthorg/engine/agent_state.py (3)
  • AgentRuntimeState (19-150)
  • idle (104-117)
  • from_context (120-150)
src/synthorg/persistence/__init__.py (1)
src/synthorg/persistence/repositories.py (1)
  • AgentStateRepository (601-660)
src/synthorg/persistence/sqlite/__init__.py (1)
src/synthorg/persistence/sqlite/agent_state_repo.py (1)
  • SQLiteAgentStateRepository (29-168)
src/synthorg/persistence/repositories.py (2)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/persistence/sqlite/agent_state_repo.py (4)
  • save (39-67)
  • get (69-101)
  • get_active (103-127)
  • delete (129-151)
src/synthorg/persistence/protocol.py (3)
src/synthorg/persistence/repositories.py (1)
  • AgentStateRepository (601-660)
src/synthorg/persistence/sqlite/backend.py (1)
  • agent_states (397-403)
tests/unit/persistence/test_protocol.py (1)
  • agent_states (333-334)
src/synthorg/persistence/sqlite/agent_state_repo.py (2)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/engine/__init__.py (1)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/persistence/sqlite/backend.py (3)
src/synthorg/persistence/sqlite/agent_state_repo.py (1)
  • SQLiteAgentStateRepository (29-168)
src/synthorg/persistence/protocol.py (1)
  • agent_states (158-160)
tests/unit/persistence/test_protocol.py (1)
  • agent_states (333-334)
tests/unit/persistence/sqlite/test_migrations_v6.py (1)
tests/unit/persistence/test_migrations_v2.py (1)
  • test_schema_version_is_eight (32-33)
tests/unit/persistence/test_migrations_v2.py (3)
tests/unit/persistence/sqlite/test_migrations_v6.py (1)
  • test_schema_version_is_eight (20-21)
src/synthorg/persistence/sqlite/migrations.py (1)
  • get_user_version (283-287)
tests/unit/persistence/sqlite/conftest.py (1)
  • memory_db (15-22)
tests/unit/persistence/test_protocol.py (3)
src/synthorg/persistence/repositories.py (16)
  • AgentStateRepository (601-660)
  • save (50-59)
  • save (116-125)
  • get (61-73)
  • get (220-232)
  • get (348-360)
  • get (428-440)
  • get (553-565)
  • get (620-632)
  • get_active (634-646)
  • delete (97-109)
  • delete (262-274)
  • delete (398-410)
  • delete (470-482)
  • delete (585-597)
  • delete (648-660)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/persistence/sqlite/backend.py (1)
  • agent_states (397-403)
tests/unit/api/conftest.py (2)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
🔇 Additional comments (19)
tests/unit/persistence/test_migrations_v2.py (1)

32-33: Schema-version bump assertion is correct.

This aligns with the v8 migration rollout and keeps the test in sync.

src/synthorg/core/enums.py (1)

505-515: ExecutionStatus enum addition looks solid.

Clear values, clear semantics, and appropriate placement in the core enums module.

src/synthorg/persistence/sqlite/migrations.py (1)

424-428: Migration v8 wiring is correct.

_apply_v8 plus _MIGRATIONS registration is clean and consistent with the schema version bump.

Also applies to: 440-440

tests/unit/persistence/sqlite/test_migrations_v6.py (1)

20-21: Version assertion update is correct.

The test now correctly tracks the migrated schema target.

tests/unit/persistence/sqlite/test_migrations.py (1)

176-198: Good v8 migration coverage additions.

These tests validate both the new table and the status index, which closes the key migration assertions for this feature.

src/synthorg/observability/events/persistence.py (1)

178-198: Agent-state event constant set is consistent and complete.

This extends the persistence event surface cleanly for structured logging.

src/synthorg/engine/agent_state.py (1)

119-150: from_context() factory logic is well-structured.

The IDLE guard and context-to-state mapping are consistent with the model invariants.

src/synthorg/engine/__init__.py (1)

9-9: Public re-export of AgentRuntimeState is correctly wired.

Import and __all__ are in sync.

Also applies to: 212-212

CLAUDE.md (1)

122-122: Documentation updates are aligned with the new persistence surface.

The engine scope and logging guidance now reflect the introduced agent runtime state and persistence events clearly.

Also applies to: 190-190

tests/unit/engine/test_agent_state.py (1)

113-285: Strong invariant and factory coverage for AgentRuntimeState.

The test suite exercises idle/from_context factories, status invariants, immutability, and serialization paths thoroughly.

src/synthorg/persistence/repositories.py (1)

600-660: AgentStateRepository protocol surface is clean and complete.

The added CRUD/query contract is consistent with the runtime state model and persistence integration points.

src/synthorg/persistence/protocol.py (1)

157-160: Backend protocol wiring for agent_states looks correct.

The new property cleanly extends the backend contract without ambiguity.

src/synthorg/persistence/sqlite/__init__.py (1)

3-5: Public re-export of SQLiteAgentStateRepository is correctly wired.

This keeps the SQLite package API consistent with other repository exports.

Also applies to: 28-28

src/synthorg/persistence/__init__.py (1)

20-20: Root persistence API export update looks good.

AgentStateRepository is now exposed consistently via synthorg.persistence.

Also applies to: 29-29

src/synthorg/persistence/sqlite/backend.py (1)

93-93: agent_states backend lifecycle wiring is complete and safe.

Initialization, teardown, repository creation, and not-connected guarding are all implemented consistently.

Also applies to: 110-110, 177-177, 396-403

tests/unit/persistence/sqlite/test_backend.py (1)

92-95: Good pre-connection guard coverage for agent_states.

This mirrors the existing repository guard tests and correctly asserts PersistenceConnectionError before connect().

tests/unit/api/conftest.py (1)

437-437: Agent-state repository wiring in fake backend is clean and complete.

The fixture now exposes agent_states consistently through FakePersistenceBackend.

Also applies to: 509-511

tests/unit/persistence/sqlite/test_agent_state_repo.py (1)

63-195: Excellent coverage for agent-state repository behavior and corruption paths.

The suite validates contract-critical flows (upsert, active-state filtering/order, delete, and deserialization failures) well.

Also applies to: 238-296

tests/unit/persistence/test_protocol.py (1)

15-15: Protocol compliance coverage for AgentStateRepository looks correct.

The fake repo shape, backend exposure, and isinstance assertion are aligned with the new persistence interface.

Also applies to: 35-35, 249-260, 332-334, 394-395

data,
)
await self._db.commit()
except (sqlite3.Error, aiosqlite.Error) as exc:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify remaining tuple-style multi-exception handlers in this file.
rg -nP --type=py 'except\s*\([^)]*,[^)]*\)\s+as\s+\w+:' src/synthorg/persistence/sqlite/agent_state_repo.py

Repository: Aureliolo/synthorg

Length of output: 299


Replace parenthesized multi-exception syntax with PEP 758 style.

Lines 55, 79, 114, and 138 use the legacy parenthesized tuple syntax for multiple exception handlers. Replace with comma-separated format per PEP 758:

Refactor details
-        except (sqlite3.Error, aiosqlite.Error) as exc:
+        except sqlite3.Error, aiosqlite.Error as exc:

Apply this change to all four occurrences (lines 55, 79, 114, 138).

This is enforced by ruff on Python 3.14 per the coding guidelines.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/persistence/sqlite/agent_state_repo.py` at line 55, Replace the
legacy parenthesized multi-exception handlers in this file: change occurrences
of "except (sqlite3.Error, aiosqlite.Error) as exc" (found at the four locations
noted) to the new PEP 758 comma-separated form "except sqlite3.Error,
aiosqlite.Error as exc" so ruff/Python 3.14 accepts it; update all four
instances (the handlers around the DB access/error-logging code) to use the
comma-separated exception list and keep the same exception variable name and
error handling logic.

Comment on lines +410 to +415
async def get_active(self) -> tuple[AgentRuntimeState, ...]:
from synthorg.core.enums import ExecutionStatus

return tuple(
s for s in self._states.values() if s.status != ExecutionStatus.IDLE
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

get_active() in fake repo should preserve repository ordering contract.

Line 410 currently returns active states in dict insertion order, not last_activity_at descending. That diverges from the repository contract and can mask ordering bugs in API tests.

Proposed fix
     async def get_active(self) -> tuple[AgentRuntimeState, ...]:
         from synthorg.core.enums import ExecutionStatus

-        return tuple(
-            s for s in self._states.values() if s.status != ExecutionStatus.IDLE
-        )
+        active = (
+            s for s in self._states.values() if s.status != ExecutionStatus.IDLE
+        )
+        return tuple(
+            sorted(active, key=lambda s: s.last_activity_at, reverse=True)
+        )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/api/conftest.py` around lines 410 - 415, get_active currently
returns active AgentRuntimeState items in dict insertion order; change it to
filter out ExecutionStatus.IDLE then sort the resulting sequence by each state's
last_activity_at in descending order before converting to a tuple so it matches
the repository contract; update the body of get_active (function name:
get_active) to perform filtering by s.status != ExecutionStatus.IDLE and then
sort by s.last_activity_at (or equivalent timestamp attribute) reverse=True
prior to returning the tuple.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.76%. Comparing base (8e9f34f) to head (02703e3).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
+ Coverage   93.72%   93.76%   +0.04%     
==========================================
  Files         474      476       +2     
  Lines       22471    22616     +145     
  Branches     2166     2180      +14     
==========================================
+ Hits        21062    21207     +145     
  Misses       1098     1098              
  Partials      311      311              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/synthorg/persistence/sqlite/agent_state_repo.py (1)

55-55: 🛠️ Refactor suggestion | 🟠 Major

Replace parenthesized exception syntax with PEP 758 style.

Lines 55, 79, 114, and 138 use the legacy parenthesized tuple syntax except (sqlite3.Error, aiosqlite.Error) as exc:. Per coding guidelines, use the PEP 758 comma-separated format.

♻️ Proposed fix
-        except (sqlite3.Error, aiosqlite.Error) as exc:
+        except sqlite3.Error, aiosqlite.Error as exc:

Apply this change to all four occurrences (lines 55, 79, 114, 138).

As per coding guidelines: Use except A, B: syntax (no parentheses) in exception handlers—PEP 758 except syntax for Python 3.14. Ruff enforces this.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/persistence/sqlite/agent_state_repo.py` at line 55, Four
exception handlers in agent_state_repo.py use the parenthesized tuple form;
replace each occurrence of "except (sqlite3.Error, aiosqlite.Error) as exc:"
with the PEP 758 comma-separated syntax "except sqlite3.Error, aiosqlite.Error
as exc:" in all four places (the handlers around the code blocks at the four
occurrences). Update each try/except block that currently references both
sqlite3.Error and aiosqlite.Error so they use the comma-separated form to
satisfy the new exception syntax enforced by Ruff.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/unit/api/fakes.py`:
- Around line 511-533: Replace silent no-ops in FakeMessageBus by failing fast:
change publish, send_direct, unsubscribe (and subscribe if it should not be
used) to raise a clear test-time error (e.g., RuntimeError/AssertionError with a
message like "FakeMessageBus.<method> not implemented in test") so tests fail
when they exercise unexpected message-bus behavior; in receive avoid real-time
delays by removing the min(timeout, 0.01) sleep and instead yield to the event
loop deterministically (e.g., await asyncio.sleep(0) when you need to yield) or
return immediately, keeping the signature and timeout semantics for callers and
making tests deterministic.

---

Duplicate comments:
In `@src/synthorg/persistence/sqlite/agent_state_repo.py`:
- Line 55: Four exception handlers in agent_state_repo.py use the parenthesized
tuple form; replace each occurrence of "except (sqlite3.Error, aiosqlite.Error)
as exc:" with the PEP 758 comma-separated syntax "except sqlite3.Error,
aiosqlite.Error as exc:" in all four places (the handlers around the code blocks
at the four occurrences). Update each try/except block that currently references
both sqlite3.Error and aiosqlite.Error so they use the comma-separated form to
satisfy the new exception syntax enforced by Ruff.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5993e968-bde9-425e-8bc9-e104547fb93e

📥 Commits

Reviewing files that changed from the base of the PR and between 757904b and f23bc6b.

📒 Files selected for processing (11)
  • docs/design/agents.md
  • docs/design/engine.md
  • docs/design/memory.md
  • src/synthorg/engine/agent_state.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/persistence/sqlite/migrations.py
  • tests/unit/api/conftest.py
  • tests/unit/api/fakes.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/persistence/test_migrations_v2.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Markdown documentation files must follow Zensical build conventions with mkdocs.yml at repo root.

Files:

  • docs/design/agents.md
  • docs/design/memory.md
  • docs/design/engine.md
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Do not use from __future__ import annotations in Python code—Python 3.14 has PEP 649 native lazy annotations.
Use except A, B: syntax (no parentheses) in exception handlers—PEP 758 except syntax for Python 3.14. Ruff enforces this.
Add type hints to all public functions and classes. Use mypy strict mode.
Use Google-style docstrings on all public classes and functions. Ruff D rules enforce this.
Prefer immutability—create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement.
Handle errors explicitly—never silently swallow exceptions.
Use Line length of 88 characters (ruff enforced).

Files:

  • src/synthorg/persistence/sqlite/migrations.py
  • tests/unit/persistence/test_migrations_v2.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/api/fakes.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • tests/unit/api/conftest.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
  • src/synthorg/engine/agent_state.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 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.
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Use structured concurrency over bare create_task.
Keep functions under 50 lines and files under 800 lines.
Validate at system boundaries (user input, external APIs, config files).
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.
Always use logger as the variable name for the module logger (not _logger, not log).
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. import EVENT_CONSTANT.
Log structured data with logger.info(EVENT, key=value)—never use logger.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 level logging is for object creation, internal flow, and entry/exit of key functions.
Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig.
Retryable errors are RateLimitError, P...

Files:

  • src/synthorg/persistence/sqlite/migrations.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/engine/agent_state.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use pytest markers: @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, @pytest.mark.slow for test organization.
Use asyncio_mode = "auto" in pytest configuration—no manual @pytest.mark.asyncio needed on async tests.
Prefer @pytest.mark.parametrize for testing similar cases.
Use Hypothesis for property-based testing with @given + @settings. Run dev profile with HYPOTHESIS_PROFILE=dev for 1000 examples.
Never skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic.
Do NOT use vendor names (Anthropic, OpenAI, Claude, GPT) in tests. Use test-provider, test-small-001, etc.

Files:

  • tests/unit/persistence/test_migrations_v2.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/api/fakes.py
  • tests/unit/api/conftest.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
src/synthorg/{engine,providers}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

RetryExhaustedError signals that all retries failed—the engine layer catches this to trigger fallback chains.

Files:

  • src/synthorg/engine/agent_state.py
🧠 Learnings (12)
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) in exception handlers—PEP 758 except syntax for Python 3.14. Ruff enforces this.

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
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/agent_state_repo.py
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level.

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to tests/**/*.py : Never skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic.

Applied to files:

  • tests/unit/api/conftest.py
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to src/synthorg/api/**/*.py : Authentication uses JWT + API key. Approval gate integration for high-risk operations.

Applied to files:

  • tests/unit/api/conftest.py
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: 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/engine.md
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to tests/**/*.py : Prefer pytest.mark.parametrize for testing similar cases.

Applied to files:

  • tests/unit/persistence/sqlite/test_agent_state_repo.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.

Applied to files:

  • tests/unit/persistence/sqlite/test_agent_state_repo.py
📚 Learning: 2026-03-15T19:03:01.705Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:03:01.705Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.

Applied to files:

  • src/synthorg/engine/agent_state.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.

Applied to files:

  • src/synthorg/engine/agent_state.py
🧬 Code graph analysis (5)
tests/unit/persistence/test_migrations_v2.py (3)
tests/unit/persistence/sqlite/test_migrations_v6.py (1)
  • test_schema_version_is_eight (20-21)
src/synthorg/persistence/sqlite/migrations.py (1)
  • get_user_version (283-287)
tests/unit/persistence/sqlite/conftest.py (1)
  • memory_db (15-22)
tests/unit/persistence/sqlite/test_migrations.py (3)
tests/unit/persistence/test_migrations_v2.py (1)
  • memory_db (23-28)
tests/unit/persistence/sqlite/conftest.py (1)
  • memory_db (15-22)
src/synthorg/persistence/sqlite/migrations.py (1)
  • run_migrations (459-527)
src/synthorg/persistence/sqlite/agent_state_repo.py (2)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
tests/unit/persistence/sqlite/test_agent_state_repo.py (3)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
src/synthorg/engine/agent_state.py (2)
  • AgentRuntimeState (19-150)
  • idle (104-117)
src/synthorg/persistence/sqlite/agent_state_repo.py (5)
  • SQLiteAgentStateRepository (29-173)
  • save (39-67)
  • get (69-101)
  • get_active (103-127)
  • delete (129-156)
src/synthorg/engine/agent_state.py (2)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
tests/unit/engine/conftest.py (1)
  • engine (449-460)
🔇 Additional comments (23)
src/synthorg/persistence/sqlite/migrations.py (2)

407-436: LGTM! DDL constraints properly enforce AgentRuntimeState invariants.

The agent_states table now includes:

  • Column-level CHECKs for status, turn_count >= 0, and accumulated_cost_usd >= 0.0
  • Cross-field CHECK enforcing idle invariants (execution_id/task_id/started_at NULL, counters zero) and active invariants (execution_id/started_at NOT NULL)

This mirrors the Pydantic model validation in AgentRuntimeState._validate_status_invariants(), providing defense-in-depth at the database layer.


439-456: LGTM!

The _apply_v8 function follows the established migration pattern, and registration in _MIGRATIONS is correct.

src/synthorg/engine/agent_state.py (3)

19-57: LGTM! Well-designed frozen Pydantic model with proper validation.

The AgentRuntimeState model:

  • Uses ConfigDict(frozen=True) for immutability
  • Employs NotBlankStr for all identifier fields per coding guidelines
  • Includes proper ge=0 constraints on turn_count and accumulated_cost_usd
  • Uses AwareDatetime for timezone-aware timestamps

59-101: LGTM! Clear invariant validation with descriptive error messages.

The validator logic properly enforces:

  • IDLE: execution_id, task_id, started_at must be None; counters must be zero
  • EXECUTING/PAUSED: execution_id and started_at are required

The violation collection approach provides actionable error messages.


103-150: LGTM! Factory methods properly implemented.

  • idle() correctly uses NotBlankStr for the agent_id parameter (previous review addressed)
  • from_context() properly rejects IDLE status with a clear error message guiding users to idle() instead
  • Both factories set last_activity_at to current UTC time
src/synthorg/persistence/sqlite/agent_state_repo.py (5)

39-67: LGTM! Save method properly implemented.

  • Uses INSERT OR REPLACE for upsert semantics
  • Commits transaction after write
  • State transition logged at INFO level (previous review addressed)
  • Error handling wraps DB errors in QueryError with context

69-101: LGTM!

The get method correctly handles the not-found case by returning None and logging at DEBUG level (appropriate since this is not a state transition).


103-127: LGTM!

The get_active query:

  • Uses parameterized ExecutionStatus.IDLE.value instead of hardcoded string
  • Orders by last_activity_at DESC matching the composite index
  • Returns a tuple for immutability

129-156: LGTM!

The delete method:

  • Logs successful deletion at INFO level (previous review addressed)
  • Logs not-found at DEBUG level (symmetric with get)
  • Returns boolean indicating whether a row was actually deleted

158-173: LGTM!

The _row_to_model helper properly handles deserialization failures by logging and wrapping ValidationError in QueryError, maintaining the repository's error contract.

docs/design/agents.md (1)

147-149: LGTM!

The documentation accurately describes AgentRuntimeState as a lightweight snapshot for dashboard queries and graceful-shutdown discovery, correctly noting its independence from the checkpoint system.

docs/design/memory.md (2)

393-394: LGTM!

The PersistenceBackend protocol documentation now correctly lists agent_states among the repository properties.


451-451: LGTM!

The Entities Persisted table now includes AgentRuntimeState with the correct source module (engine/agent_state.py), repository (AgentStateRepository), and key queries (by agent_id, active agents).

docs/design/engine.md (1)

267-283: LGTM!

The new "Agent Execution Status" section:

  • Clearly documents the three ExecutionStatus states with their meanings
  • Explains the relationship to AgentRuntimeState and AgentStateRepository
  • Provides appropriate cross-reference to the Agents design page
tests/unit/persistence/test_migrations_v2.py (2)

32-33: LGTM!

Test name correctly updated to test_schema_version_is_eight to reflect the new schema version.


125-125: LGTM!

The assertion now uses SCHEMA_VERSION constant instead of a hardcoded value, making the test future-proof (previous review addressed).

tests/unit/persistence/sqlite/test_migrations.py (1)

176-216: LGTM! Comprehensive v8 migration tests.

The three new tests provide good coverage:

  • test_v8_creates_agent_states_table: Verifies table existence
  • test_v8_creates_agent_states_columns: Verifies exact column set using set equality
  • test_v8_creates_agent_states_composite_index: Verifies the composite index exists

This aligns with the migration implementation and ensures schema correctness.

tests/unit/persistence/sqlite/test_agent_state_repo.py (4)

29-55: LGTM — well-designed helper respects model invariants.

The _make_state helper correctly handles IDLE status by returning a minimal state that satisfies the AgentRuntimeState model's invariants (no execution_id, task_id, started_at; zero turn_count and cost). This prevents test failures from invariant violations.


249-270: Nice consolidation of error-path tests.

The parametrized approach addresses prior feedback and cleanly tests all four CRUD operations in a single test method. Using memory_db (unmigrated) to trigger SQL errors is an effective pattern.

One minor observation: _make_state() in line 252 is evaluated at module import time rather than test execution time. Since AgentRuntimeState is frozen (immutable), this is safe, but using pytest.param with a callable or a fixture would be more robust against future changes.


63-243: Comprehensive functional test coverage.

The test suite covers the key behaviors well:

  • Round-trip persistence for both active and idle states
  • Upsert semantics with field updates
  • Not-found returns None
  • get_active() filtering and ordering
  • Delete isolation between agents
  • Full lifecycle transitions

Good use of pytest.approx() for floating-point comparison (line 111).


272-332: Good coverage of deserialization error paths.

Testing corrupt rows that bypass SQL CHECK constraints but fail Pydantic validation is valuable for ensuring QueryError is raised consistently. Both get() and get_active() error paths are covered.

tests/unit/api/fakes.py (1)

387-389: get_active() ordering/filtering is correct.

Filtering out ExecutionStatus.IDLE and sorting by last_activity_at descending matches the expected repository contract.

tests/unit/api/conftest.py (1)

27-33: Good refactor: centralized fake imports and exports.

Moving to tests.unit.api.fakes improves fixture-module cohesion and keeps conftest.py focused on fixture wiring.

Comment on lines +511 to +533
async def publish(self, message: Message) -> None:
pass

async def send_direct(self, message: Message, *, recipient: str) -> None:
pass

async def subscribe(self, channel_name: str, subscriber_id: str) -> Any:
return None

async def unsubscribe(self, channel_name: str, subscriber_id: str) -> None:
pass

async def receive(
self,
channel_name: str,
subscriber_id: str,
*,
timeout: float | None = None, # noqa: ASYNC109
) -> Any:
# Simulate waiting for a message (yields to event loop)
if timeout is not None:
await asyncio.sleep(min(timeout, 0.01))
return None
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fail fast in FakeMessageBus methods; avoid silent no-ops and real sleeps.

Line 512, Line 515, and Line 521 currently swallow operations (pass), and Line 532 introduces real timing delay in unit tests. This can hide message-flow regressions and add flaky latency.

Proposed fix
 class FakeMessageBus:
@@
     def __init__(self) -> None:
         self._running = False
         self._channels: list[Channel] = []
+        self._published: list[Message] = []
+
+    def _require_running(self) -> None:
+        if not self._running:
+            msg = "Message bus is not running"
+            raise RuntimeError(msg)
@@
     async def publish(self, message: Message) -> None:
-        pass
+        self._require_running()
+        self._published.append(message)
@@
     async def send_direct(self, message: Message, *, recipient: str) -> None:
-        pass
+        self._require_running()
+        self._published.append(message)
@@
     async def unsubscribe(self, channel_name: str, subscriber_id: str) -> None:
-        pass
+        self._require_running()
@@
     async def receive(
@@
-        if timeout is not None:
-            await asyncio.sleep(min(timeout, 0.01))
+        if timeout is not None:
+            await asyncio.sleep(0)
         return None

As per coding guidelines "Handle errors explicitly—never silently swallow exceptions." and "For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/api/fakes.py` around lines 511 - 533, Replace silent no-ops in
FakeMessageBus by failing fast: change publish, send_direct, unsubscribe (and
subscribe if it should not be used) to raise a clear test-time error (e.g.,
RuntimeError/AssertionError with a message like "FakeMessageBus.<method> not
implemented in test") so tests fail when they exercise unexpected message-bus
behavior; in receive avoid real-time delays by removing the min(timeout, 0.01)
sleep and instead yield to the event loop deterministically (e.g., await
asyncio.sleep(0) when you need to yield) or return immediately, keeping the
signature and timeout semantics for callers and making tests deterministic.

Implements lightweight per-agent runtime state tracking (#261):
- ExecutionStatus enum (idle/executing/paused) in core/enums.py
- AgentRuntimeState frozen model with status invariant validation,
  idle() and from_context() factories
- AgentStateRepository protocol (save/get/get_active/delete)
- SQLiteAgentStateRepository with v8 migration (agent_states table)
- 10 persistence event constants for structured logging
- Full wiring: PersistenceBackend protocol, SQLitePersistenceBackend,
  re-exports in persistence/ and engine/ packages
- 38 new tests (23 model + 15 repo) plus protocol compliance,
  backend before-connect, and migration tests

Closes #261
- Add FakeAgentStateRepository and agent_states property to
  FakePersistenceBackend in tests/unit/api/conftest.py
- Use TaskType.DEVELOPMENT instead of string literal in test
Pre-reviewed by 12 agents, 10 findings addressed:
- Replace hardcoded 'idle' string with ExecutionStatus.IDLE.value param
- Use ValidationError in test_frozen instead of bare Exception
- Implement FakeAgentStateRepository.get_active() with proper filtering
- Add CLAUDE.md package structure + logging event documentation
- Parametrize idle-violation tests
- Add idle("  ") blank agent_id test
- Add from_context zero-cost test
- Add get_active deserialization failure test for corrupt rows
…eRabbit

- Update design spec: remove "(planned)" from agent state entity,
  add AgentRuntimeState to agents.md runtime state section,
  document ExecutionStatus enum in engine.md, update PersistenceBackend
  code block comment with agent_states
- Use NotBlankStr in idle() factory parameter signature
- Promote state transition logs (saved/deleted) from DEBUG to INFO
- Add delete() not-found logging for symmetry with get()
- Add CHECK constraints to agent_states DDL (status enum values,
  turn_count >= 0, cost >= 0, cross-field invariant)
- Replace single-column index with composite (status, last_activity_at)
- Fix FakeAgentStateRepository.get_active() ordering to match protocol
- Rename _heartbeats_repo/_agent_states_repo to _heartbeats/_agent_states
- Move ExecutionStatus import to top level in conftest
- Extract 15 Fake* classes from conftest.py to fakes.py (846→~290 lines)
- Add lifecycle transition test (idle→executing→idle roundtrip)
- Add V8 column schema test (PRAGMA table_info)
- Add empty-table get_active test
- Parametrize 4 QueryError error tests into 1
- Add __cause__ assertions to error chain tests
- Use SCHEMA_VERSION constant instead of hardcoded 8
- Update corrupt-row tests for CHECK constraint compatibility
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (2)
tests/unit/api/fakes.py (1)

511-521: ⚠️ Potential issue | 🟠 Major

Don't let the remaining message-bus stubs silently succeed.

receive() is deterministic now, but publish(), send_direct(), subscribe(), and unsubscribe() still do nothing (or return None). That lets API tests pass after unexpectedly exercising message-bus paths.

♻️ Proposed fix
     async def publish(self, message: Message) -> None:
-        pass
+        raise AssertionError("FakeMessageBus.publish() was called unexpectedly")

     async def send_direct(self, message: Message, *, recipient: str) -> None:
-        pass
+        raise AssertionError("FakeMessageBus.send_direct() was called unexpectedly")

     async def subscribe(self, channel_name: str, subscriber_id: str) -> Any:
-        return None
+        raise AssertionError("FakeMessageBus.subscribe() was called unexpectedly")

     async def unsubscribe(self, channel_name: str, subscriber_id: str) -> None:
-        pass
+        raise AssertionError("FakeMessageBus.unsubscribe() was called unexpectedly")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/api/fakes.py` around lines 511 - 521, The message-bus stub methods
publish, send_direct, subscribe, and unsubscribe currently do nothing (or return
None) and should instead surface errors so tests fail when these paths are
exercised; update the async methods publish(message: Message),
send_direct(message: Message, *, recipient: str), subscribe(channel_name: str,
subscriber_id: str), and unsubscribe(channel_name: str, subscriber_id: str) in
the test fake to raise a clear exception (e.g., NotImplementedError or a custom
RuntimeError with an explanatory message) so any unexpected use of the message
bus in tests fails loudly and points to the offending method.
src/synthorg/persistence/sqlite/agent_state_repo.py (1)

55-55: ⚠️ Potential issue | 🟠 Major

Switch these handlers to PEP 758 exception syntax.

These four except clauses still use the parenthesized multi-exception form, which is no longer the repo standard on Python 3.14. Replace each except (sqlite3.Error, aiosqlite.Error) as exc: with the comma form.

♻️ Proposed fix
-        except (sqlite3.Error, aiosqlite.Error) as exc:
+        except sqlite3.Error, aiosqlite.Error as exc:

Run this to verify the remaining occurrences. Expected result: 4 matches before the fix, 0 after.

#!/bin/bash
rg -nP --type=py 'except\s*\([^)]*,[^)]*\)\s+as\s+\w+:' src/synthorg/persistence/sqlite/agent_state_repo.py

As per coding guidelines: Use PEP 758 except syntax: use except A, B: (no parentheses) — ruff enforces this on Python 3.14.

Also applies to: 79-79, 114-114, 138-138

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/persistence/sqlite/agent_state_repo.py` at line 55, Replace the
parenthesized multi-exception handlers like "except (sqlite3.Error,
aiosqlite.Error) as exc:" in agent_state_repo.py with the PEP 758 comma form
(e.g. "except sqlite3.Error, aiosqlite.Error as exc:") for all occurrences;
search for the string "except (sqlite3.Error, aiosqlite.Error) as exc:" and
update each occurrence so it uses the comma-separated exception list instead of
parentheses to comply with the repo's Python 3.14 exception syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/engine/agent_state.py`:
- Around line 10-17: Add module-level observability by importing get_logger from
synthorg.observability and creating logger = get_logger(__name__) at top of the
module, then instrument the error paths in _validate_status_invariants and
from_context to emit structured logs (use logger.warning or logger.error as
appropriate) with the relevant event constant and contextual data immediately
before raising; ensure the invalid-IDLE branch in from_context and any
invariant-failure branches log the error and include identifying fields (e.g.,
status, context.id) so the raised exceptions are accompanied by structured
observability.

In `@src/synthorg/persistence/sqlite/migrations.py`:
- Around line 419-435: The schema stores last_activity_at as TEXT and
orders/indexes it (idx_as_status_activity) which misorders mixed-offset
AwareDatetime values; update persistence to normalize datetimes to UTC or,
better, add a numeric epoch column (e.g., last_activity_at_ts REAL/INTEGER) in
the migration and index/order by that instead, modify agent_state_repo.py
(functions that persist and get_active()) to write the normalized UTC epoch or
populate the new epoch column on save, update get_active() to sort by
last_activity_at_ts (and fall back to last_activity_at if needed), and ensure
agent_state.py produces/returns UTC-aware datetimes or the epoch for storage so
ordering is consistent across offsets.

In `@tests/unit/api/fakes.py`:
- Around line 229-260: The fake query implementation in query currently accepts
an invalid time window; add a guard that raises QueryError when both since and
until are provided and until < since so the fake matches AuditRepository.query
behavior. Locate the async def query(...) function, validate the since/until
parameters after the limit check (and before filtering), and raise QueryError
with a clear message (e.g., "until must be >= since") when the window is
impossible; keep existing filtering and return logic unchanged. Ensure you
reference QueryError and the since/until parameters in the check so tests
exercising invalid time windows receive the same error as the real repo.
- Around line 102-114: The fake repository accepts duplicate IDs and returns
messages in insertion order; update save(self, message: Message) to detect
existing Message.id in self._messages and surface a failure consistent with the
real contract (e.g., raise the same DuplicateMessageError or ValueError used by
the production repo) instead of silently appending, and change get_history(self,
channel: str, *, limit: int | None = None) to order messages by
Message.created_at descending (newest-first) before applying limit (treat limit
<= 0 as no limit) and return the sliced results as a tuple so tests exercise the
same duplicate-handling and ordering behavior as the real repository.

In `@tests/unit/persistence/sqlite/test_migrations.py`:
- Around line 176-216: Tests for v8 only check presence of agent_states, its
column names, and index name but don't lock down CHECK constraints or index
ordering; update the tests (e.g., in test_v8_creates_agent_states_columns and
test_v8_creates_agent_states_composite_index) to (1) assert the actual DDL by
selecting sqlite_master.sql for name='agent_states' and checking that required
CHECK clauses are present, (2) use PRAGMA index_xinfo('idx_as_status_activity')
to assert the index covers columns (status, last_activity_at) and that
last_activity_at is DESC, and (3) add a negative-insert test that attempts to
insert a row violating the CHECK constraint into agent_states and asserts that
the insert fails, thereby pinning the schema contract.

---

Duplicate comments:
In `@src/synthorg/persistence/sqlite/agent_state_repo.py`:
- Line 55: Replace the parenthesized multi-exception handlers like "except
(sqlite3.Error, aiosqlite.Error) as exc:" in agent_state_repo.py with the PEP
758 comma form (e.g. "except sqlite3.Error, aiosqlite.Error as exc:") for all
occurrences; search for the string "except (sqlite3.Error, aiosqlite.Error) as
exc:" and update each occurrence so it uses the comma-separated exception list
instead of parentheses to comply with the repo's Python 3.14 exception syntax.

In `@tests/unit/api/fakes.py`:
- Around line 511-521: The message-bus stub methods publish, send_direct,
subscribe, and unsubscribe currently do nothing (or return None) and should
instead surface errors so tests fail when these paths are exercised; update the
async methods publish(message: Message), send_direct(message: Message, *,
recipient: str), subscribe(channel_name: str, subscriber_id: str), and
unsubscribe(channel_name: str, subscriber_id: str) in the test fake to raise a
clear exception (e.g., NotImplementedError or a custom RuntimeError with an
explanatory message) so any unexpected use of the message bus in tests fails
loudly and points to the offending method.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d9c10f0d-8552-4b09-8ee2-04d6da4bf459

📥 Commits

Reviewing files that changed from the base of the PR and between f23bc6b and dfd9434.

📒 Files selected for processing (24)
  • CLAUDE.md
  • docs/design/agents.md
  • docs/design/engine.md
  • docs/design/memory.md
  • src/synthorg/core/enums.py
  • src/synthorg/engine/__init__.py
  • src/synthorg/engine/agent_state.py
  • src/synthorg/observability/events/persistence.py
  • src/synthorg/persistence/__init__.py
  • src/synthorg/persistence/protocol.py
  • src/synthorg/persistence/repositories.py
  • src/synthorg/persistence/sqlite/__init__.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/persistence/sqlite/backend.py
  • src/synthorg/persistence/sqlite/migrations.py
  • tests/unit/api/conftest.py
  • tests/unit/api/fakes.py
  • tests/unit/engine/test_agent_state.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
  • tests/unit/persistence/sqlite/test_backend.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/persistence/sqlite/test_migrations_v6.py
  • tests/unit/persistence/test_migrations_v2.py
  • tests/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 Web
  • GitHub Check: Build Backend
  • GitHub Check: Build Sandbox
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Do NOT use from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: use except A, B: (no parentheses) — ruff enforces this on Python 3.14
Include type hints on all public functions; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Line length: 88 characters (ruff default)

Files:

  • src/synthorg/engine/__init__.py
  • tests/unit/persistence/test_protocol.py
  • src/synthorg/persistence/__init__.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/persistence/sqlite/test_backend.py
  • src/synthorg/persistence/repositories.py
  • src/synthorg/persistence/sqlite/__init__.py
  • src/synthorg/core/enums.py
  • tests/unit/persistence/sqlite/test_migrations_v6.py
  • src/synthorg/engine/agent_state.py
  • src/synthorg/persistence/sqlite/backend.py
  • src/synthorg/persistence/sqlite/migrations.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/persistence/protocol.py
  • tests/unit/engine/test_agent_state.py
  • tests/unit/persistence/test_migrations_v2.py
  • tests/unit/api/conftest.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
  • src/synthorg/observability/events/persistence.py
  • tests/unit/api/fakes.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Create new objects for immutability; never mutate existing ones. For non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Separate config and runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves (e.g. agent execution state, task progress)
Use Pydantic v2 conventions: 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
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Keep functions under 50 lines; keep files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
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; always use the synthorg observability logger
Always use logger as the variable name (not _logger, not log)
Always use event name constants from the domain-specific module under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from `even...

Files:

  • src/synthorg/engine/__init__.py
  • src/synthorg/persistence/__init__.py
  • src/synthorg/persistence/repositories.py
  • src/synthorg/persistence/sqlite/__init__.py
  • src/synthorg/core/enums.py
  • src/synthorg/engine/agent_state.py
  • src/synthorg/persistence/sqlite/backend.py
  • src/synthorg/persistence/sqlite/migrations.py
  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • src/synthorg/persistence/protocol.py
  • src/synthorg/observability/events/persistence.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use test markers: @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, @pytest.mark.slow
Use asyncio_mode = "auto" — no manual @pytest.mark.asyncio needed
Set 30 second timeout per test
Prefer @pytest.mark.parametrize for testing similar cases
Tests must use test-provider, test-small-001, etc. instead of real vendor names
Use Python Hypothesis for property-based testing: @given + @settings. Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic

Files:

  • tests/unit/persistence/test_protocol.py
  • tests/unit/persistence/sqlite/test_migrations.py
  • tests/unit/persistence/sqlite/test_backend.py
  • tests/unit/persistence/sqlite/test_migrations_v6.py
  • tests/unit/engine/test_agent_state.py
  • tests/unit/persistence/test_migrations_v2.py
  • tests/unit/api/conftest.py
  • tests/unit/persistence/sqlite/test_agent_state_repo.py
  • tests/unit/api/fakes.py
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Markdown documentation files must follow Zensical-compatible format (no unsupported Markdown extensions)

Files:

  • docs/design/agents.md
  • docs/design/engine.md
  • docs/design/memory.md
docs/design/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Design specification pages in docs/design/ must be updated when approved deviations occur

Files:

  • docs/design/agents.md
  • docs/design/engine.md
  • docs/design/memory.md
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Separate config and runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state that evolves (e.g. agent execution state, task progress)
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Separate config and runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state that evolves (e.g. agent execution state, task progress)

Applied to files:

  • src/synthorg/engine/__init__.py
  • src/synthorg/engine/agent_state.py
  • tests/unit/engine/test_agent_state.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).

Applied to files:

  • tests/unit/persistence/sqlite/test_migrations_v6.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.

Applied to files:

  • src/synthorg/engine/agent_state.py
  • tests/unit/engine/test_agent_state.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators

Applied to files:

  • src/synthorg/engine/agent_state.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
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/agent_state_repo.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
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/agent_state_repo.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly; never silently swallow exceptions

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
  • CLAUDE.md
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Use DEBUG logging for object creation, internal flow, entry/exit of key functions

Applied to files:

  • src/synthorg/persistence/sqlite/agent_state_repo.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)

Applied to files:

  • tests/unit/engine/test_agent_state.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.

Applied to files:

  • tests/unit/persistence/sqlite/test_agent_state_repo.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases

Applied to files:

  • tests/unit/persistence/sqlite/test_agent_state_repo.py
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, etc.)

Applied to files:

  • src/synthorg/observability/events/persistence.py
  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
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
  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
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:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Use structured logging with `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)`

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
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:

  • CLAUDE.md
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging` / `logging.getLogger()` / `print()` in application code; always use the synthorg observability logger

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to src/synthorg/**/*.py : Always use `logger` as the variable name (not `_logger`, not `log`)

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-03-15T19:25:26.476Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:25:26.476Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic

Applied to files:

  • tests/unit/api/fakes.py
🧬 Code graph analysis (15)
src/synthorg/engine/__init__.py (1)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
tests/unit/persistence/test_protocol.py (4)
src/synthorg/persistence/repositories.py (24)
  • AgentStateRepository (601-660)
  • save (50-59)
  • save (116-125)
  • save (172-182)
  • save (209-218)
  • save (286-296)
  • save (337-346)
  • save (417-426)
  • save (489-498)
  • save (542-551)
  • save (609-618)
  • get (61-73)
  • get (220-232)
  • get (348-360)
  • get (428-440)
  • get (553-565)
  • get (620-632)
  • get_active (634-646)
  • delete (97-109)
  • delete (262-274)
  • delete (398-410)
  • delete (470-482)
  • delete (585-597)
  • delete (648-660)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/persistence/sqlite/agent_state_repo.py (4)
  • save (39-67)
  • get (69-101)
  • get_active (103-127)
  • delete (129-156)
src/synthorg/persistence/protocol.py (1)
  • agent_states (158-160)
src/synthorg/persistence/__init__.py (1)
src/synthorg/persistence/repositories.py (1)
  • AgentStateRepository (601-660)
tests/unit/persistence/sqlite/test_migrations.py (3)
tests/unit/persistence/test_migrations_v2.py (1)
  • memory_db (23-28)
tests/unit/persistence/sqlite/conftest.py (1)
  • memory_db (15-22)
src/synthorg/persistence/sqlite/migrations.py (1)
  • run_migrations (459-527)
tests/unit/persistence/sqlite/test_backend.py (2)
src/synthorg/persistence/sqlite/backend.py (1)
  • agent_states (397-403)
src/synthorg/persistence/protocol.py (1)
  • agent_states (158-160)
src/synthorg/persistence/repositories.py (2)
src/synthorg/engine/agent_state.py (1)
  • AgentRuntimeState (19-150)
src/synthorg/persistence/sqlite/agent_state_repo.py (4)
  • save (39-67)
  • get (69-101)
  • get_active (103-127)
  • delete (129-156)
src/synthorg/persistence/sqlite/__init__.py (1)
src/synthorg/persistence/sqlite/agent_state_repo.py (1)
  • SQLiteAgentStateRepository (29-173)
tests/unit/persistence/sqlite/test_migrations_v6.py (1)
tests/unit/persistence/test_migrations_v2.py (1)
  • test_schema_version_is_eight (32-33)
src/synthorg/engine/agent_state.py (2)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
tests/unit/engine/conftest.py (1)
  • engine (449-460)
src/synthorg/persistence/sqlite/backend.py (2)
src/synthorg/persistence/sqlite/agent_state_repo.py (1)
  • SQLiteAgentStateRepository (29-173)
src/synthorg/persistence/protocol.py (1)
  • agent_states (158-160)
src/synthorg/persistence/protocol.py (3)
src/synthorg/persistence/repositories.py (1)
  • AgentStateRepository (601-660)
src/synthorg/persistence/sqlite/backend.py (1)
  • agent_states (397-403)
tests/unit/persistence/test_protocol.py (1)
  • agent_states (333-334)
tests/unit/engine/test_agent_state.py (2)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
src/synthorg/engine/agent_state.py (3)
  • AgentRuntimeState (19-150)
  • idle (104-117)
  • from_context (120-150)
tests/unit/persistence/test_migrations_v2.py (3)
tests/unit/persistence/sqlite/test_migrations_v6.py (1)
  • test_schema_version_is_eight (20-21)
src/synthorg/persistence/sqlite/migrations.py (1)
  • get_user_version (283-287)
tests/unit/persistence/sqlite/conftest.py (1)
  • memory_db (15-22)
tests/unit/api/conftest.py (1)
tests/unit/api/fakes.py (2)
  • FakeMessageBus (494-554)
  • FakePersistenceBackend (395-491)
tests/unit/persistence/sqlite/test_agent_state_repo.py (3)
src/synthorg/core/enums.py (1)
  • ExecutionStatus (505-515)
src/synthorg/engine/agent_state.py (2)
  • AgentRuntimeState (19-150)
  • idle (104-117)
src/synthorg/persistence/sqlite/agent_state_repo.py (5)
  • SQLiteAgentStateRepository (29-173)
  • save (39-67)
  • get (69-101)
  • get_active (103-127)
  • delete (129-156)
🔇 Additional comments (8)
tests/unit/persistence/test_migrations_v2.py (1)

32-33: LGTM!

The schema version assertions are correctly updated to reflect SCHEMA_VERSION == 8, and line 125 now uses the SCHEMA_VERSION constant instead of a hardcoded value, making the test future-proof for schema bumps.

Also applies to: 125-125

src/synthorg/persistence/sqlite/__init__.py (1)

3-5: LGTM!

The new SQLiteAgentStateRepository import and export follow the established pattern for other SQLite repository classes in this module, and the alphabetical ordering in __all__ is maintained.

Also applies to: 28-28

docs/design/agents.md (1)

147-149: LGTM!

The documentation accurately describes the AgentRuntimeState concept and correctly notes its independence from the checkpoint system, which is an important architectural distinction for understanding the persistence model.

src/synthorg/persistence/__init__.py (1)

19-26: LGTM!

The AgentStateRepository re-export follows the established pattern for other repository protocols in this package, and alphabetical ordering in __all__ is preserved.

Also applies to: 28-29

tests/unit/persistence/sqlite/test_backend.py (1)

92-95: LGTM!

The test correctly follows the established pattern for verifying pre-connection error behavior, matching the existing tests for tasks, cost_records, messages, and audit_entries properties.

src/synthorg/core/enums.py (1)

505-516: LGTM!

The ExecutionStatus enum is well-defined with a clear docstring explaining its purpose and relationship to AgentRuntimeState. The enum follows the established StrEnum pattern used throughout the file.

tests/unit/persistence/sqlite/test_migrations_v6.py (1)

20-21: LGTM!

The schema version assertion is correctly updated to reflect the new SCHEMA_VERSION == 8, consistent with the v8 migration that introduces the agent_states table.

docs/design/engine.md (1)

267-283: LGTM!

The new "Agent Execution Status" section provides clear documentation of the ExecutionStatus enum with accurate descriptions for each state. The cross-references to AgentRuntimeState and the agents design page properly connect the concepts across the documentation.

Comment on lines +10 to +17
from datetime import UTC, datetime

from pydantic import AwareDatetime, BaseModel, ConfigDict, Field, model_validator

from synthorg.core.enums import ExecutionStatus
from synthorg.core.types import NotBlankStr # noqa: TC001
from synthorg.engine.context import AgentContext # noqa: TC001

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add module-level observability; invalid state creation is currently silent.

This new business-logic module never initializes logger, so invariant failures in _validate_status_invariants() and the invalid-IDLE path in from_context() raise with no structured signal. Please wire get_logger(__name__) here and log those error paths with event constants before raising.

As per coding guidelines: Every module with business logic MUST have: from synthorg.observability import get_logger then logger = get_logger(__name__); all error paths must log at WARNING or ERROR with context before raising.

Also applies to: 74-101, 119-149

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/engine/agent_state.py` around lines 10 - 17, Add module-level
observability by importing get_logger from synthorg.observability and creating
logger = get_logger(__name__) at top of the module, then instrument the error
paths in _validate_status_invariants and from_context to emit structured logs
(use logger.warning or logger.error as appropriate) with the relevant event
constant and contextual data immediately before raising; ensure the invalid-IDLE
branch in from_context and any invariant-failure branches log the error and
include identifying fields (e.g., status, context.id) so the raised exceptions
are accompanied by structured observability.

Comment on lines +419 to +435
last_activity_at TEXT NOT NULL,
started_at TEXT,
CHECK (
(status = 'idle'
AND execution_id IS NULL
AND task_id IS NULL
AND started_at IS NULL
AND turn_count = 0
AND accumulated_cost_usd = 0.0)
OR
(status IN ('executing', 'paused')
AND execution_id IS NOT NULL
AND started_at IS NOT NULL)
)
)""",
"CREATE INDEX IF NOT EXISTS idx_as_status_activity "
"ON agent_states(status, last_activity_at DESC)",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not rely on raw timestamp text for active-agent ordering.

Line 419 stores last_activity_at as TEXT, and Lines 434-435 index/order that text. That only stays chronologically correct if every persisted value is normalized to the same offset. src/synthorg/engine/agent_state.py (lines 18-149) accepts arbitrary AwareDatetime values, and src/synthorg/persistence/sqlite/agent_state_repo.py (lines 28-172) persists then sorts the raw serialized string in get_active(). Mixed offsets will return active agents in the wrong order. Normalize to UTC before persistence, or store/order by a numeric epoch column instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/persistence/sqlite/migrations.py` around lines 419 - 435, The
schema stores last_activity_at as TEXT and orders/indexes it
(idx_as_status_activity) which misorders mixed-offset AwareDatetime values;
update persistence to normalize datetimes to UTC or, better, add a numeric epoch
column (e.g., last_activity_at_ts REAL/INTEGER) in the migration and index/order
by that instead, modify agent_state_repo.py (functions that persist and
get_active()) to write the normalized UTC epoch or populate the new epoch column
on save, update get_active() to sort by last_activity_at_ts (and fall back to
last_activity_at if needed), and ensure agent_state.py produces/returns
UTC-aware datetimes or the epoch for storage so ordering is consistent across
offsets.

Comment on lines +102 to +114
async def save(self, message: Message) -> None:
self._messages.append(message)

async def get_history(
self,
channel: str,
*,
limit: int | None = None,
) -> tuple[Message, ...]:
result = [m for m in self._messages if m.channel == channel]
if limit is not None and limit > 0:
result = result[-limit:]
return tuple(result)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the fake message repository aligned with the real contract.

This fake now backs API tests, but save() silently accepts duplicate message IDs and get_history() returns insertion order / tail slices instead of newest-first history. That can hide duplicate-handling and ordering bugs that production repositories are supposed to surface.

♻️ Proposed fix
     async def save(self, message: Message) -> None:
+        if any(existing.id == message.id for existing in self._messages):
+            msg = f"Duplicate message {message.id!r}"
+            raise DuplicateRecordError(msg)
         self._messages.append(message)

     async def get_history(
         self,
         channel: str,
         *,
         limit: int | None = None,
     ) -> tuple[Message, ...]:
-        result = [m for m in self._messages if m.channel == channel]
-        if limit is not None and limit > 0:
-            result = result[-limit:]
+        result = sorted(
+            (m for m in self._messages if m.channel == channel),
+            key=lambda m: m.timestamp,
+            reverse=True,
+        )
+        if limit is not None and limit > 0:
+            result = result[:limit]
         return tuple(result)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/api/fakes.py` around lines 102 - 114, The fake repository accepts
duplicate IDs and returns messages in insertion order; update save(self,
message: Message) to detect existing Message.id in self._messages and surface a
failure consistent with the real contract (e.g., raise the same
DuplicateMessageError or ValueError used by the production repo) instead of
silently appending, and change get_history(self, channel: str, *, limit: int |
None = None) to order messages by Message.created_at descending (newest-first)
before applying limit (treat limit <= 0 as no limit) and return the sliced
results as a tuple so tests exercise the same duplicate-handling and ordering
behavior as the real repository.

- FakeMessageRepository: add duplicate ID detection (DuplicateRecordError),
  timestamp DESC ordering, and limit validation to match SQLiteMessageRepository
- FakeAuditRepository: add until < since guard to match real query contract
- V8 migration tests: verify DDL CHECK constraints in sqlite_master.sql,
  add negative-insert test for invalid status, verify composite index
  covers (status ASC, last_activity_at DESC) via PRAGMA index_xinfo
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 19:50 — with GitHub Actions Inactive
@Aureliolo Aureliolo merged commit 5009da7 into main Mar 15, 2026
30 of 31 checks passed
@Aureliolo Aureliolo deleted the feat/agent-state-repository branch March 15, 2026 19:52
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 19:52 — with GitHub Actions Inactive
Aureliolo added a commit that referenced this pull request Mar 15, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.2.6](v0.2.5...v0.2.6)
(2026-03-15)


### Features

* add intra-loop stagnation detector
([#415](#415))
([#458](#458))
([8e9f34f](8e9f34f))
* add RFC 9457 structured error responses (Phase 1)
([#457](#457))
([6612a99](6612a99)),
closes [#419](#419)
* implement AgentStateRepository for runtime state persistence
([#459](#459))
([5009da7](5009da7))
* **site:** add SEO essentials, contact form, early-access banner
([#467](#467))
([11b645e](11b645e)),
closes [#466](#466)


### Bug Fixes

* CLI improvements — config show, completion install, enhanced doctor,
Sigstore verification
([#465](#465))
([9e08cec](9e08cec))
* **site:** add reCAPTCHA v3, main landmark, and docs sitemap
([#469](#469))
([fa6d35c](fa6d35c))
* use force-tag-creation instead of manual tag creation hack
([#462](#462))
([2338004](2338004))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: implement AgentStateRepository for runtime state persistence

1 participant