feat: Add conversation variable persistence layer

[tomerqodo]

Benchmark PR from agentic-review-benchmarks#4

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (4) 📎 Requirement gaps (0)

1. Session(db.engine) not context-managed 📘 Rule violation ⛯ Reliability

Description

• The new ConversationVariableUpdaterImpl.update() opens a SQLAlchemy session via
  Session(db.engine) without a context manager, and never closes it, risking connection leaks and
  inconsistent transaction cleanup.
• This violates the requirement that sessions be managed with context managers to ensure lifecycle
  correctness and reliability.

Code

api/services/conversation_variable_updater.py[R18-23]

+        session = Session(db.engine)
+        row = session.scalar(stmt)
+        if not row:
+            raise ConversationVariableNotFoundError("conversation variable not found in the database")
+        row.data = variable.model_dump_json()
+        session.commit()

Evidence

PR Compliance ID 11 requires SQLAlchemy sessions to be used with a context manager. The updated code
creates a session directly (session = Session(db.engine)) and performs work/commit without `with
... as session:` or explicit closing.

AGENTS.md
api/services/conversation_variable_updater.py[14-23]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ConversationVariableUpdaterImpl.update()` creates a SQLAlchemy session without a context manager, which can leak connections and skip proper cleanup.

## Issue Context
Compliance requires context-managed sessions (e.g., `with Session(db.engine, expire_on_commit=False) as session:`) for safe lifecycle management.

## Fix Focus Areas
- api/services/conversation_variable_updater.py[14-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Updater raises non-domain exception 📘 Rule violation ✓ Correctness

Description

• The updater introduces ConversationVariableNotFoundError as a local Exception type rather than
  using a domain-specific exception from the standard services/errors or core/errors locations.
• This makes consistent controller-level translation and error semantics harder to enforce and
  violates the domain-exception policy.

Code

api/services/conversation_variable_updater.py[R9-22]

+class ConversationVariableNotFoundError(Exception):
+    pass


class ConversationVariableUpdaterImpl:
-    def update(self, conversation_id: str, variable: Variable):
+    def update(self, conversation_id: str, variable: Variable) -> None:
        stmt = select(ConversationVariable).where(
            ConversationVariable.id == variable.id, ConversationVariable.conversation_id == conversation_id
        )
-        with Session(db.engine) as session:
-            row = session.scalar(stmt)
-            if not row:
-                raise VariableOperatorNodeError("conversation variable not found in the database")
-            row.data = variable.model_dump_json()
-            session.commit()
-
-    def flush(self):
+        session = Session(db.engine)
+        row = session.scalar(stmt)
+        if not row:
+            raise ConversationVariableNotFoundError("conversation variable not found in the database")
+        row.data = variable.model_dump_json()

Evidence

PR Compliance ID 20 requires services to raise domain-specific exceptions from the project’s
designated error modules. The code defines a new generic exception inline and raises it from service
logic.

AGENTS.md
api/services/conversation_variable_updater.py[9-22]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new inline generic exception (`ConversationVariableNotFoundError`) is introduced in a service module rather than using the repository’s domain-specific exception modules.

## Issue Context
Services should raise domain-specific exceptions from `services/errors` or `core/errors`, and controllers should translate them to HTTP responses.

## Fix Focus Areas
- api/services/conversation_variable_updater.py[9-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. __init__ missing -> None 📘 Rule violation ✓ Correctness

Description

• The newly added VariableAssignerNode.__init__ in v2 lacks an explicit return type annotation.
• This violates the requirement that all Python function definitions include full parameter and
  return type annotations using modern Python 3.12+ syntax.

Code

api/core/workflow/nodes/variable_assigner/v2/node.py[R56-62]

+    def __init__(
+        self,
+        id: str,
+        config: Mapping[str, Any],
+        graph_init_params: "GraphInitParams",
+        graph_runtime_state: "GraphRuntimeState",
+    ):

Evidence

PR Compliance ID 7 requires return type annotations on all functions. The added __init__
definition does not specify -> None.

AGENTS.md
api/core/workflow/nodes/variable_assigner/v2/node.py[56-62]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly added `__init__` method is missing a return type annotation.

## Issue Context
Type annotation compliance requires all function definitions to include return types.

## Fix Focus Areas
- api/core/workflow/nodes/variable_assigner/v2/node.py[56-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (1)

4. Tests lack return annotations 📘 Rule violation ✓ Correctness

Description

• Newly added pytest test functions (e.g.,
  test_persists_conversation_variables_from_assigner_output) do not include -> None return
  annotations.
• This violates the requirement that all Python function definitions include type annotations for
  parameters and return values.

Code

api/tests/unit_tests/core/app/layers/test_conversation_variable_persist_layer.py[63]

+def test_persists_conversation_variables_from_assigner_output():

Evidence

PR Compliance ID 7 requires explicit return type annotations on all function definitions. The added
test functions are defined without -> None.

AGENTS.md
api/tests/unit_tests/core/app/layers/test_conversation_variable_persist_layer.py[63-65]
api/tests/unit_tests/core/app/layers/test_conversation_variable_persist_layer.py[88-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New pytest test functions are missing return type annotations.

## Issue Context
Repository typing compliance requires all function definitions to have return type annotations.

## Fix Focus Areas
- api/tests/unit_tests/core/app/layers/test_conversation_variable_persist_layer.py[63-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. session(db.engine) not context-managed 📘 Rule violation ⛯ Reliability

• The new ConversationVariableUpdaterImpl.update() opens a SQLAlchemy session via
  Session(db.engine) without a context manager, and never closes it, risking connection leaks and
  inconsistent transaction cleanup.
• This violates the requirement that sessions be managed with context managers to ensure lifecycle
  correctness and reliability.

Agent prompt

## Issue description
`ConversationVariableUpdaterImpl.update()` creates a SQLAlchemy session without a context manager, which can leak connections and skip proper cleanup.

## Issue Context
Compliance requires context-managed sessions (e.g., `with Session(db.engine, expire_on_commit=False) as session:`) for safe lifecycle management.

## Fix Focus Areas
- api/services/conversation_variable_updater.py[14-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

2. Updater raises non-domain exception 📘 Rule violation ✓ Correctness

• The updater introduces ConversationVariableNotFoundError as a local Exception type rather than
  using a domain-specific exception from the standard services/errors or core/errors locations.
• This makes consistent controller-level translation and error semantics harder to enforce and
  violates the domain-exception policy.

Agent prompt

## Issue description
A new inline generic exception (`ConversationVariableNotFoundError`) is introduced in a service module rather than using the repository’s domain-specific exception modules.

## Issue Context
Services should raise domain-specific exceptions from `services/errors` or `core/errors`, and controllers should translate them to HTTP responses.

## Fix Focus Areas
- api/services/conversation_variable_updater.py[9-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

3. init missing -> none 📘 Rule violation ✓ Correctness

• The newly added VariableAssignerNode.__init__ in v2 lacks an explicit return type annotation.
• This violates the requirement that all Python function definitions include full parameter and
  return type annotations using modern Python 3.12+ syntax.

Agent prompt

## Issue description
The newly added `__init__` method is missing a return type annotation.

## Issue Context
Type annotation compliance requires all function definitions to include return types.

## Fix Focus Areas
- api/core/workflow/nodes/variable_assigner/v2/node.py[56-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

4. Tests lack return annotations 📘 Rule violation ✓ Correctness

• Newly added pytest test functions (e.g.,
  test_persists_conversation_variables_from_assigner_output) do not include -> None return
  annotations.
• This violates the requirement that all Python function definitions include type annotations for
  parameters and return values.

Agent prompt

## Issue description
New pytest test functions are missing return type annotations.

## Issue Context
Repository typing compliance requires all function definitions to have return type annotations.

## Fix Focus Areas
- api/tests/unit_tests/core/app/layers/test_conversation_variable_persist_layer.py[63-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools