From d9b3d684c46e9766c3fafd70a02461729c75bcc9 Mon Sep 17 00:00:00 2001 From: jessegmeyerlab Date: Sun, 12 Jul 2026 21:06:16 -0700 Subject: [PATCH] DB migration helper + dialog/database updates + .gitignore - migrate_db.py: schema migration helper for the request-visualization store. - database.py and dialog updates. - Add .gitignore for Python artifacts and local sqlite stores. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 15 + pyproject.toml | 2 +- src/request_visualization/database.py | 70 ++++- src/request_visualization/migrate_db.py | 280 +++++++++++++++++++ src/request_visualization/widgets/dialogs.py | 49 +++- 5 files changed, 400 insertions(+), 16 deletions(-) create mode 100644 .gitignore create mode 100644 src/request_visualization/migrate_db.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..ccf5acc --- /dev/null +++ b/.gitignore @@ -0,0 +1,15 @@ +# Python +__pycache__/ +*.py[cod] +*.egg-info/ +build/ +dist/ +.venv/ +venv/ +.pytest_cache/ + +# Local data / secrets +*.sqlite +*.sqlite-journal +.env +*.key diff --git a/pyproject.toml b/pyproject.toml index 4234a67..e95a6c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ dynamic = ["version"] description = "Desktop GUI for visualizing and managing agent requests" readme = "README.md" requires-python = ">=3.10" -licese = {text = "GPLv3"} +license = {text = "GPLv3"} dependencies = [ "PyQt6>=6.4.0", "httpx>=0.24.0", diff --git a/src/request_visualization/database.py b/src/request_visualization/database.py index 1b07430..ee64d06 100644 --- a/src/request_visualization/database.py +++ b/src/request_visualization/database.py @@ -16,11 +16,25 @@ DEFAULT_API_KEY_FILE = Path.home() / ".claude" / "azure.key" DEFAULT_EMBEDDING_MODEL = "text-embedding-3-small" -# Valid request statuses -VALID_STATUSES = ("pending", "approved", "rejected", "implemented", "completed", "cancelled") - -# Inactive statuses (requests that don't need attention) -INACTIVE_STATUSES = ("completed", "rejected", "cancelled") +# Valid request statuses (aligned with the canonical schema in +# odda_utils/src/odda_utils/static/schema.sql). +VALID_STATUSES = ( + "pending", + "approved", + "rejected", + "in_progress", + "implemented", + "incomplete", + "completed", + "cancelled", +) + +# Inactive statuses (requests that no longer need attention). These are the +# terminal states: an accepted implementation (completed), a rejected request, +# a cancelled request, or an implementation that could not be finished +# (incomplete). Active requests are everything else: pending, approved, +# in_progress, and implemented. +INACTIVE_STATUSES = ("completed", "rejected", "cancelled", "incomplete") def get_connection(db_path: Path = DEFAULT_DB_PATH) -> sqlite3.Connection: @@ -44,11 +58,16 @@ def get_connection(db_path: Path = DEFAULT_DB_PATH) -> sqlite3.Connection: def ensure_table_exists(db_path: Path = DEFAULT_DB_PATH) -> None: """Create the agent_requests table if it doesn't exist. - The table supports the following statuses: + The schema matches the canonical definition in + ``odda_utils/src/odda_utils/static/schema.sql`` and supports the following + statuses: + - pending: Initial state, awaiting review - approved: Approved for implementation - rejected: Rejected with a reason + - in_progress: Implementation is underway - implemented: Implementation completed, awaiting verification + - incomplete: Implementation attempted but could not be finished - completed: Implementation verified and accepted - cancelled: Cancelled after implementation @@ -67,8 +86,9 @@ def ensure_table_exists(db_path: Path = DEFAULT_DB_PATH) -> None: request TEXT NOT NULL, reason_for_request TEXT, request_status TEXT DEFAULT 'pending' - CHECK (request_status IN ('pending', 'approved', 'rejected', 'implemented', 'completed', 'cancelled')), + CHECK (request_status IN ('pending', 'approved', 'rejected', 'in_progress', 'implemented', 'incomplete', 'completed', 'cancelled')), status_reason TEXT, + assigned_time TIMESTAMP, updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, embedding BLOB, embedding_model VARCHAR(100) @@ -86,6 +106,10 @@ def ensure_table_exists(db_path: Path = DEFAULT_DB_PATH) -> None: "CREATE INDEX IF NOT EXISTS idx_agent_requests_agent " "ON agent_requests(agent_name)" ) + conn.execute( + "CREATE INDEX IF NOT EXISTS idx_agent_requests_assigned " + "ON agent_requests(assigned_time)" + ) conn.commit() finally: conn.close() @@ -170,7 +194,7 @@ def get_all_requests( Path to the SQLite database file. status_filter : str | None Optional status to filter by ('pending', 'approved', 'rejected', - 'implemented', 'completed', 'cancelled'). + 'in_progress', 'implemented', 'incomplete', 'completed', 'cancelled'). Returns ------- @@ -510,6 +534,9 @@ def _embedding_to_blob(embedding: list[float]) -> bytes: return struct.pack(f"{len(embedding)}f", *embedding) +# TODO: Consolidate this credential reader and get_text_embedding with the +# shared model layer being built in odda_utils; kept local for now so the GUI +# continues to work standalone. def _get_azure_credentials( endpoint_file: Path | None = None, api_key_file: Path | None = None, @@ -577,8 +604,31 @@ def get_text_embedding( Raises ------ httpx.HTTPError - If the API request fails. + If the API request fails (standalone fallback path only). + + Notes + ----- + When the ``odda_utils`` package is importable (the normal deployment), this + delegates to the shared bring-your-own-key model layer so the GUI honours the + same provider configuration as the rest of ODDA. If ``odda_utils`` is not + available, it falls back to a self-contained Azure OpenAI request so the GUI + still works standalone. """ + try: + from odda_utils.utils import get_text_embedding as _shared_embedding + except ImportError: + _shared_embedding = None + + if _shared_embedding is not None: + return _shared_embedding( + text, + endpoint_file=endpoint_file, + api_key_file=api_key_file, + deployment_name=deployment_name, + api_version=api_version, + ) + + # Standalone fallback: direct Azure OpenAI embeddings request. endpoint, api_key = _get_azure_credentials(endpoint_file, api_key_file) url = f"{endpoint}/openai/deployments/{deployment_name}/embeddings" @@ -662,7 +712,7 @@ def set_request_status( ID of the request to update. status : str New status value. Must be one of: 'pending', 'approved', 'rejected', - 'implemented', 'completed', 'cancelled'. + 'in_progress', 'implemented', 'incomplete', 'completed', 'cancelled'. reason : str | None Optional reason for the status change. db_path : Path diff --git a/src/request_visualization/migrate_db.py b/src/request_visualization/migrate_db.py new file mode 100644 index 0000000..548166f --- /dev/null +++ b/src/request_visualization/migrate_db.py @@ -0,0 +1,280 @@ +# Standalone migration for the agent_requests table. +# Brings an existing articles.sqlite agent_requests table up to the canonical +# schema (odda_utils/src/odda_utils/static/schema.sql): adds the assigned_time +# column when missing and rebuilds the table when its request_status CHECK +# constraint does not allow all eight canonical statuses. The migration is +# idempotent and safe to run repeatedly. Exposed as the ``request-viz-migrate`` +# console script via its ``main`` function. + +import argparse +import sqlite3 +from pathlib import Path + +DEFAULT_DB_PATH = Path("./articles.sqlite") + +# Canonical column set for agent_requests, in schema order. Used to copy data +# across a table rebuild. +CANONICAL_COLUMNS = ( + "id", + "agent_name", + "creation_time", + "request", + "reason_for_request", + "request_status", + "status_reason", + "assigned_time", + "updated_at", + "embedding", + "embedding_model", +) + +# All eight statuses the canonical request_status CHECK constraint must allow. +REQUIRED_STATUSES = ( + "pending", + "approved", + "rejected", + "in_progress", + "implemented", + "incomplete", + "completed", + "cancelled", +) + +# Columns that can be added in place with ALTER TABLE ADD COLUMN when they are +# missing but the CHECK constraint is already correct (no rebuild required). +_ADDABLE_COLUMNS = ( + ("assigned_time", "TIMESTAMP"), + ("embedding", "BLOB"), + ("embedding_model", "VARCHAR(100)"), +) + +# Canonical CREATE TABLE statement, matching schema.sql exactly. +CANONICAL_TABLE_SQL = """ + CREATE TABLE IF NOT EXISTS agent_requests ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + agent_name TEXT NOT NULL, + creation_time TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + request TEXT NOT NULL, + reason_for_request TEXT, + request_status TEXT DEFAULT 'pending' + CHECK (request_status IN ('pending', 'approved', 'rejected', 'in_progress', 'implemented', 'incomplete', 'completed', 'cancelled')), + status_reason TEXT, + assigned_time TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + embedding BLOB, + embedding_model VARCHAR(100) + ) +""" + +# Canonical indexes for agent_requests. +CANONICAL_INDEX_SQL = ( + "CREATE INDEX IF NOT EXISTS idx_agent_requests_status ON agent_requests(request_status)", + "CREATE INDEX IF NOT EXISTS idx_agent_requests_creation ON agent_requests(creation_time)", + "CREATE INDEX IF NOT EXISTS idx_agent_requests_agent ON agent_requests(agent_name)", + "CREATE INDEX IF NOT EXISTS idx_agent_requests_assigned ON agent_requests(assigned_time)", +) + +_BACKUP_TABLE_NAME = "agent_requests_migration_backup" + + +def _get_table_sql(conn: sqlite3.Connection) -> str | None: + """Return the CREATE statement stored for the agent_requests table. + + Parameters + ---------- + conn : sqlite3.Connection + Open connection to the target database. + + Returns + ------- + str | None + The SQL text used to create the table, or None if it does not exist. + """ + row = conn.execute( + "SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'agent_requests'" + ).fetchone() + return row[0] if row and row[0] else None + + +def _get_column_names(conn: sqlite3.Connection) -> list[str]: + """Return the column names of the agent_requests table. + + Parameters + ---------- + conn : sqlite3.Connection + Open connection to the target database. + + Returns + ------- + list[str] + Column names in table order. + """ + cursor = conn.execute("PRAGMA table_info(agent_requests)") + return [row[1] for row in cursor.fetchall()] + + +def _check_covers_all_statuses(table_sql: str) -> bool: + """Determine whether the table's CHECK constraint allows all statuses. + + Parameters + ---------- + table_sql : str + The CREATE statement text for the table. + + Returns + ------- + bool + True if every canonical status appears as a quoted literal in the + table definition, False otherwise (including when there is no CHECK + constraint at all). + """ + lowered = table_sql.lower() + return all(f"'{status}'" in lowered for status in REQUIRED_STATUSES) + + +def _rebuild_table_with_canonical_schema( + conn: sqlite3.Connection, existing_cols: list[str] +) -> None: + """Recreate agent_requests with the canonical schema, preserving data. + + SQLite cannot alter a CHECK constraint in place, so the table is renamed, + recreated from the canonical definition, repopulated from the columns the + old and new tables have in common, and finally the backup is dropped. + + Parameters + ---------- + conn : sqlite3.Connection + Open connection to the target database. + existing_cols : list[str] + Column names present in the current table, used to build a safe + column intersection for copying rows. + """ + common = [col for col in CANONICAL_COLUMNS if col in existing_cols] + col_list = ", ".join(common) + + conn.execute(f"DROP TABLE IF EXISTS {_BACKUP_TABLE_NAME}") + conn.execute(f"ALTER TABLE agent_requests RENAME TO {_BACKUP_TABLE_NAME}") + conn.execute(CANONICAL_TABLE_SQL) + if col_list: + conn.execute( + f"INSERT INTO agent_requests ({col_list}) " + f"SELECT {col_list} FROM {_BACKUP_TABLE_NAME}" + ) + conn.execute(f"DROP TABLE {_BACKUP_TABLE_NAME}") + + +def migrate_agent_requests(db_path: Path = DEFAULT_DB_PATH) -> list[str]: + """Migrate the agent_requests table to the canonical schema. + + The migration is idempotent: running it against an already-canonical table + performs no changes. If the table does not exist it is created. Otherwise + the migration ensures the ``assigned_time`` (and, for completeness, + ``embedding``/``embedding_model``) columns exist and that the + ``request_status`` CHECK constraint allows all eight canonical statuses, + rebuilding the table only when the constraint needs to change. + + Parameters + ---------- + db_path : Path + Path to the SQLite database file. Connecting creates the file if it + does not already exist. + + Returns + ------- + list[str] + Human-readable descriptions of the changes made. An empty list means + the schema was already canonical and nothing was changed. + """ + changes: list[str] = [] + conn = sqlite3.connect(str(db_path)) + try: + table_sql = _get_table_sql(conn) + + if table_sql is None: + conn.execute(CANONICAL_TABLE_SQL) + for index_sql in CANONICAL_INDEX_SQL: + conn.execute(index_sql) + conn.commit() + changes.append("created agent_requests table with canonical schema") + return changes + + existing_cols = _get_column_names(conn) + + if not _check_covers_all_statuses(table_sql): + _rebuild_table_with_canonical_schema(conn, existing_cols) + changes.append( + "rebuilt agent_requests so request_status allows all eight " + "canonical statuses" + ) + # The rebuilt table has every canonical column; report the ones + # that the old table lacked (e.g. assigned_time). + for col, _decl in _ADDABLE_COLUMNS: + if col not in existing_cols: + changes.append(f"added missing '{col}' column") + else: + for col, decl in _ADDABLE_COLUMNS: + if col not in existing_cols: + conn.execute( + f"ALTER TABLE agent_requests ADD COLUMN {col} {decl}" + ) + changes.append(f"added missing '{col}' column") + + # Ensure the canonical indexes exist (idempotent). + for index_sql in CANONICAL_INDEX_SQL: + conn.execute(index_sql) + + conn.commit() + return changes + finally: + conn.close() + + +def main(argv: list[str] | None = None) -> int: + """Run the agent_requests migration from the command line. + + Parameters + ---------- + argv : list[str] | None + Optional argument list (primarily for testing). When None, arguments + are read from ``sys.argv``. + + Returns + ------- + int + Process exit code (0 on success). + """ + parser = argparse.ArgumentParser( + description=( + "Migrate an existing articles.sqlite agent_requests table to the " + "canonical schema (adds assigned_time and ensures all eight " + "request statuses are allowed). Safe to run repeatedly." + ) + ) + parser.add_argument( + "--db", + type=Path, + default=DEFAULT_DB_PATH, + help="Path to the SQLite database file (default: ./articles.sqlite).", + ) + args = parser.parse_args(argv) + + if not args.db.exists(): + print( + f"Database {args.db} does not exist; it will be created with the " + "canonical agent_requests table." + ) + + changes = migrate_agent_requests(args.db) + + if changes: + print(f"Migration of {args.db} complete:") + for change in changes: + print(f" - {change}") + else: + print(f"No migration needed for {args.db}; schema is already canonical.") + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/request_visualization/widgets/dialogs.py b/src/request_visualization/widgets/dialogs.py index b19d425..7948e0c 100644 --- a/src/request_visualization/widgets/dialogs.py +++ b/src/request_visualization/widgets/dialogs.py @@ -1,7 +1,10 @@ # Dialog widgets for the request visualization GUI. # Provides modal dialogs for approving, rejecting, revising, and creating requests. # The NewRequestDialog supports selecting existing projects or creating new ones. +# The base directory used to discover projects is configurable via the +# ODDA_PROJECTS_DIR environment variable, with a cross-platform fallback. +import os from pathlib import Path from PyQt6.QtWidgets import ( @@ -225,8 +228,9 @@ class NewRequestDialog(QDialog): by selecting "New Project" and entering a project name. """ - # Default base directory for discovering projects - DEFAULT_BASE_DIR = Path("./") + # Environment variable used to configure the base directory for project + # discovery. Kept as a class attribute so it can be overridden/inspected. + PROJECTS_DIR_ENV_VAR = "ODDA_PROJECTS_DIR" # Special option for creating a new project NEW_PROJECT_OPTION = "New Project" @@ -239,15 +243,18 @@ def __init__(self, parent=None, base_dir: Path | str | None = None): parent : QWidget, optional Parent widget. base_dir : Path | str | None, optional - Base directory to search for project directories. - Defaults to /home/lex/projects/mcp. + Base directory to search for project directories. When not + provided, the directory named by the ``ODDA_PROJECTS_DIR`` + environment variable is used (``~`` is expanded); if that variable + is unset, the current working directory is used as a + cross-platform fallback. """ super().__init__(parent) self.setWindowTitle("New Request") self.setMinimumWidth(500) self.setMinimumHeight(400) - self._base_dir = Path(base_dir) if base_dir else self.DEFAULT_BASE_DIR + self._base_dir = self._resolve_base_dir(base_dir) layout = QVBoxLayout(self) @@ -300,6 +307,38 @@ def __init__(self, parent=None, base_dir: Path | str | None = None): self.button_box.rejected.connect(self.reject) layout.addWidget(self.button_box) + @classmethod + def _resolve_base_dir(cls, base_dir: Path | str | None) -> Path: + """Resolve the base directory used to discover projects. + + Resolution order: + + 1. An explicit ``base_dir`` argument, if provided. + 2. The path named by the ``ODDA_PROJECTS_DIR`` environment variable + (with ``~`` expanded), if set and non-empty. + 3. The current working directory, as a cross-platform fallback. + + Parameters + ---------- + base_dir : Path | str | None + Explicit base directory override, or None to use the environment + variable / fallback. + + Returns + ------- + Path + The resolved base directory. It is not guaranteed to exist; callers + handle a missing directory gracefully. + """ + if base_dir: + return Path(base_dir).expanduser() + + env_value = os.environ.get(cls.PROJECTS_DIR_ENV_VAR) + if env_value: + return Path(env_value).expanduser() + + return Path.cwd() + def _populate_projects(self) -> None: """Populate the project dropdown with directories from the base directory.