diff --git a/packages/core/alembic/versions/0064_widen_git_function_blame_symbol_id.py b/packages/core/alembic/versions/0064_widen_git_function_blame_symbol_id.py new file mode 100644 index 000000000..79d6ef974 --- /dev/null +++ b/packages/core/alembic/versions/0064_widen_git_function_blame_symbol_id.py @@ -0,0 +1,74 @@ +"""Widen git_function_blame.symbol_id to ``Text`` (PostgreSQL only). + +``symbol_id`` stores ``"{path}::{name}"`` and mirrors ``WikiSymbol.symbol_id``, +which is unbounded ``Text`` — but this column was declared ``VARCHAR(512)``. +A deeply-nested generated file (protobuf/OpenAPI bindings, minified bundles) +routinely produces a ``path::name`` combination past 512 characters, and +PostgreSQL enforces the declared length where SQLite ignores it. So the +narrow type is invisible on the default backend and aborts the run on the one +the architecture docs recommend for production, in the persistence phase, +after the whole blame index has already been computed (issue #2162). + +Same class of bug as #1565, fixed for six other columns by +``0062_widen_symbol_name_columns.py``. This column predates that migration — +``git_function_blame`` was created in ``0029_git_function_blame.py`` — but +``0062`` targeted columns holding a symbol *name* at ``VARCHAR(255)``, and +this one holds a symbol *id* at ``VARCHAR(512)``, so it fell outside the +pattern that migration was matching. + +This is the last bounded symbol-id-shaped column in ``models.py``, so this +closes the class rather than starting another sweep. + +Only PostgreSQL is altered. SQLite does not enforce ``VARCHAR`` length — the +column already behaves as ``TEXT`` there — and local SQLite stores never run +Alembic anyway (``init_db``'s reconciler is additive-only), so a +``batch_alter_table`` would rebuild the table to change nothing. + +Revision ID: 0064 +Revises: 0063 +Create Date: 2026-09-08 +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers +revision: str = "0064" +down_revision: str | None = "0063" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + if op.get_bind().dialect.name != "postgresql": + return + op.alter_column( + "git_function_blame", + "symbol_id", + existing_type=sa.String(length=512), + type_=sa.Text(), + existing_nullable=False, + ) + + +def downgrade() -> None: + """Narrow the column back. + + This fails on a database that already stores a value longer than 512 + characters, which is the correct outcome: casting with ``left(col, 512)`` + would silently destroy the key the row is about in order to make the + downgrade look clean. + """ + if op.get_bind().dialect.name != "postgresql": + return + op.alter_column( + "git_function_blame", + "symbol_id", + existing_type=sa.Text(), + type_=sa.String(length=512), + existing_nullable=False, + ) diff --git a/packages/core/src/repowise/core/persistence/models.py b/packages/core/src/repowise/core/persistence/models.py index b43f4f558..9bf11fc2c 100644 --- a/packages/core/src/repowise/core/persistence/models.py +++ b/packages/core/src/repowise/core/persistence/models.py @@ -836,8 +836,10 @@ class GitFunctionBlame(Base): repository_id: Mapped[str] = mapped_column( String(32), ForeignKey("repositories.id", ondelete="CASCADE"), nullable=False ) - # "{path}::{name}" — mirrors WikiSymbol.symbol_id. - symbol_id: Mapped[str] = mapped_column(String(512), nullable=False) + # "{path}::{name}" — mirrors WikiSymbol.symbol_id. Text, not a bounded + # VARCHAR: a deeply-nested generated file (protobuf/OpenAPI bindings) + # routinely produces a path::name past any fixed width (#2162). + symbol_id: Mapped[str] = mapped_column(Text, nullable=False) file_path: Mapped[str] = mapped_column(Text, nullable=False, default="") function_name: Mapped[str] = mapped_column(Text, nullable=False, default="") start_line: Mapped[int] = mapped_column(Integer, nullable=False, default=0) diff --git a/tests/integration/test_postgres_git_function_blame_symbol_id.py b/tests/integration/test_postgres_git_function_blame_symbol_id.py new file mode 100644 index 000000000..3d4998008 --- /dev/null +++ b/tests/integration/test_postgres_git_function_blame_symbol_id.py @@ -0,0 +1,88 @@ +"""PostgreSQL regression for issue #2162 — a long git_function_blame.symbol_id +must persist. + +SQLite ignores ``VARCHAR`` length, so this never surfaced there. Only +PostgreSQL enforces it, and that is where a ``"{path}::{name}"`` past 512 +characters — routine for a deeply-nested generated file — aborted the +persistence phase after the whole blame index had already been computed. + +Skipped unless a PostgreSQL URL is configured, so a local ``pytest`` run is +unchanged:: + + REPOWISE_TEST_PG_URL=postgresql+asyncpg://user@localhost:5432/repowise_test \\ + uv run pytest tests/integration/test_postgres_git_function_blame_symbol_id.py +""" + +from __future__ import annotations + +import os +from datetime import UTC, datetime + +import pytest +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine + +from repowise.core.persistence.database import init_db +from repowise.core.persistence.models import Base, GitFunctionBlame, Repository, _new_uuid + +PG_URL = os.environ.get("REPOWISE_TEST_PG_URL") + +pytestmark = pytest.mark.skipif( + not PG_URL, reason="REPOWISE_TEST_PG_URL not set — PostgreSQL-only regression" +) + +# Longer than the 512 the column used to declare. A deeply-nested generated +# file's "{path}::{name}" reaches this routinely. +LONG_PATH = "packages/generated/vendor/protobuf/" + "sub_module/" * 20 +LONG_NAME = "process_" + "generated_protobuf_message_field_descriptor_handler_" * 6 + "value" +LONG_SYMBOL_ID = f"{LONG_PATH}::{LONG_NAME}" + + +@pytest.fixture +async def pg_session(): + # The fixture drops every table, so it refuses a database that is not + # named like a scratch one. Pointing the env var at a real index and + # losing it should take more than a typo. + database = (PG_URL or "").rsplit("/", 1)[-1].split("?")[0] + if "test" not in database and "scratch" not in database: + pytest.skip(f"refusing to drop tables in {database!r}: name it *test* or *scratch*") + + engine = create_async_engine(PG_URL or "", poolclass=None) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.drop_all) + await init_db(engine) + factory = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + async with factory() as session: + yield session + await engine.dispose() + + +async def test_long_symbol_id_round_trips(pg_session: AsyncSession) -> None: + """git_function_blame.symbol_id accepts a "{path}::{name}" past 512 chars.""" + assert len(LONG_SYMBOL_ID) > 512 + + now = datetime.now(UTC) + repo = Repository(id=_new_uuid(), name="repro", local_path="/tmp/repro", url="") + pg_session.add(repo) + await pg_session.flush() + + pg_session.add( + GitFunctionBlame( + id=_new_uuid(), + repository_id=repo.id, + symbol_id=LONG_SYMBOL_ID, + file_path=LONG_PATH, + function_name=LONG_NAME, + start_line=1, + end_line=10, + line_count=10, + created_at=now, + updated_at=now, + ) + ) + await pg_session.commit() + + stored = await pg_session.scalar( + select(GitFunctionBlame.symbol_id).where(GitFunctionBlame.repository_id == repo.id) + ) + assert stored == LONG_SYMBOL_ID, "the symbol_id came back truncated"