FIX: bound attack conversation IDs before indexing (#2317)

Co-authored-by: Behnam Ousat <behnamousat@microsoft.com>

Author: behnam-o

.github/copilot-instructions.md

@@ -29,6 +29,7 @@ Aim for fewer, higher-signal comments. A review with 2-3 important comments is b
 
 BEFORE editing or code-reviewing any file, you MUST read the `.github/instructions/` files whose `applyTo` patterns match the files you are about to edit. For example:
 - Editing/code-reviewing `pyrit/**/*.py` → read `style-guide.instructions.md` and `user-custom.instructions.md`
+- Editing/code-reviewing database models, Alembic migrations, or memory migration tests → also read `database.instructions.md`
 - Editing/code-reviewing `pyrit/scenario/**` → also read `scenarios.instructions.md`
 - Editing/code-reviewing `pyrit/setup/initializers/techniques/**` → also read `setup-techniques.instructions.md`
 - Editing/code-reviewing `pyrit/converter/**` → also read `converters.instructions.md`

.github/instructions/database.instructions.md

@@ -0,0 +1,14 @@
+---
+description: "Use when editing database models, SQLAlchemy schemas, Alembic migrations, indexes, keys, constraints, or database migration tests for SQLite and SQL Server."
+applyTo: "pyrit/memory/**/*.py, tests/**/memory/**/*.py"
+---
+
+# Database Portability Guidelines
+
+- Test schema and migration changes against both SQLite and SQL Server semantics. SQLite accepts
+  text columns as index keys, while SQL Server rejects `VARCHAR(MAX)` / `NVARCHAR(MAX)` key columns.
+- Give every `String` / `Unicode` column used in a primary key, foreign key, index, or unique
+  constraint an explicit, appropriately bounded length in both ORM models and Alembic migrations.
+- When correcting an existing unbounded indexed column, alter it to the bounded type before
+  creating the index. Keep the operation dialect-aware when a backend such as SQLite does not
+  support the same `ALTER COLUMN` operation.

pyrit/memory/alembic/versions/d7e9f1a3b5c6_index_attack_result_recency.py

@@ -9,7 +9,9 @@
 composite ``(timestamp, id)`` index (serves the recency ORDER BY + keyset seek). Backfills
 ``timestamp`` from the legacy ``attack_metadata.updated_at``/``created_at`` JSON keys so
 manually-edited conversations keep their current History order, and drops the now-redundant
-``updated_at`` key from the JSON metadata.
+``updated_at`` key from the JSON metadata. Narrows ``conversation_id`` to ``VARCHAR(36)`` on
+all backends so the persisted schema matches the ORM model; this is required on SQL Server
+because ``VARCHAR(MAX)`` columns cannot be index keys.
 
 Revision ID: d7e9f1a3b5c6
 Revises: 3f6e8a0c2d4b
@@ -40,6 +42,7 @@
 # executemany per batch keeps the number of statements (and, on SQL Server, database roundtrips)
 # proportional to the row count / batch size instead of one statement per row.
 _BACKFILL_UPDATE_BATCH_SIZE = 400
+_CONVERSATION_ID_LENGTH = 36
 
 
 def _attack_results_table() -> sa.Table:
@@ -80,6 +83,7 @@ def _parse_iso(value: object) -> datetime | None:
 
 def upgrade() -> None:
     """Apply this schema upgrade."""
+    _set_conversation_id_type(length=_CONVERSATION_ID_LENGTH)
     op.create_index(
         "ix_AttackResultEntries_conversation_id",
         "AttackResultEntries",
@@ -102,6 +106,27 @@ def downgrade() -> None:
     _restore_updated_at_to_metadata()
     op.drop_index("ix_AttackResultEntries_timestamp_id", table_name="AttackResultEntries")
     op.drop_index("ix_AttackResultEntries_conversation_id", table_name="AttackResultEntries")
+    _set_conversation_id_type(length=None)
+
+
+def _set_conversation_id_type(*, length: int | None) -> None:
+    """
+    Set ``conversation_id`` to bounded or unbounded ``VARCHAR``.
+
+    Batch alteration recreates the table on SQLite, which does not support ``ALTER COLUMN``
+    directly, and emits an in-place alteration on SQL Server. The bounded type keeps the
+    persisted schema aligned with the ORM model and makes the column indexable on SQL Server.
+
+    Args:
+        length (int | None): The target ``VARCHAR`` length, or ``None`` for ``VARCHAR(MAX)``.
+    """
+    with op.batch_alter_table("AttackResultEntries") as batch_op:
+        batch_op.alter_column(
+            "conversation_id",
+            existing_type=sa.String(),
+            type_=sa.String(length=length),
+            existing_nullable=False,
+        )
 
 
 def _backfill_timestamp_from_metadata() -> None:

pyrit/memory/memory_models.py

@@ -1557,7 +1557,7 @@ class AttackResultEntry(Base):
         {"extend_existing": True},
     )
     id = mapped_column(CustomUUID, nullable=False, primary_key=True)
-    conversation_id = mapped_column(String, nullable=False)
+    conversation_id = mapped_column(String(36), nullable=False)
     objective = mapped_column(Unicode, nullable=False)
     atomic_attack_identifier: Mapped[dict[str, Any] | None] = mapped_column(JSON, nullable=True)
     atomic_attack_identifier_hash: Mapped[str | None] = mapped_column(

tests/unit/memory/test_migration.py

@@ -2315,6 +2315,11 @@ def test_attack_recency_upgrade_creates_indexes_and_backfills_timestamp():
                 command.upgrade(config, _ATTACK_RECENCY_REV)
 
                 index_names = {ix["name"] for ix in inspect(connection).get_indexes("AttackResultEntries")}
+                conversation_id_column = next(
+                    column
+                    for column in inspect(connection).get_columns("AttackResultEntries")
+                    if column["name"] == "conversation_id"
+                )
                 rows = dict(
                     connection.execute(text('SELECT id, attack_metadata FROM "AttackResultEntries"')).fetchall()
                 )
@@ -2324,6 +2329,7 @@ def test_attack_recency_upgrade_creates_indexes_and_backfills_timestamp():
 
             assert "ix_AttackResultEntries_conversation_id" in index_names
             assert "ix_AttackResultEntries_timestamp_id" in index_names
+            assert conversation_id_column["type"].length == 36
 
             edited_metadata = json.loads(rows[edited_id])
             assert "updated_at" not in edited_metadata
@@ -2364,6 +2370,11 @@ def test_attack_recency_downgrade_restores_updated_at_and_drops_indexes():
                 command.downgrade(config, _ATTACK_RECENCY_PREV_REV)
 
                 index_names_down = {ix["name"] for ix in inspect(connection).get_indexes("AttackResultEntries")}
+                conversation_id_column_down = next(
+                    column
+                    for column in inspect(connection).get_columns("AttackResultEntries")
+                    if column["name"] == "conversation_id"
+                )
                 restored = json.loads(
                     connection.execute(
                         text('SELECT attack_metadata FROM "AttackResultEntries" WHERE id = :id'),
@@ -2373,6 +2384,7 @@ def test_attack_recency_downgrade_restores_updated_at_and_drops_indexes():
 
             assert "ix_AttackResultEntries_conversation_id" not in index_names_down
             assert "ix_AttackResultEntries_timestamp_id" not in index_names_down
+            assert conversation_id_column_down["type"].length is None
             assert restored["created_at"] == "2026-06-01T00:00:00+00:00"
             assert restored["updated_at"].startswith("2026-06-01T12:00:00")
         finally: