diff --git a/alembic/versions/0070_notes_data_jsonb.py b/alembic/versions/0070_notes_data_jsonb.py new file mode 100644 index 0000000..5518c36 --- /dev/null +++ b/alembic/versions/0070_notes_data_jsonb.py @@ -0,0 +1,60 @@ +"""add notes.data JSONB — queryable structured fields for typed records + +Revision ID: 0070 +Revises: 0069 +Create Date: 2026-07-26 + +Snippets (note_type='snippet') carry structured fields — name, language, +signature, and a list of canonical locations (repo · path · symbol). Those were +stored as a markdown body-convention, which reads well and feeds the embedding +but cannot be QUERIED: answering "which snippets live in this file?" meant +scanning every snippet and regexing its body. + +This adds a general `data` JSONB column plus a GIN index, so those fields become +indexable. The body stays exactly as it was — it is still the human-readable +form and still what gets embedded. `data` is a queryable mirror of the same +facts, not a replacement, and the code itself is deliberately NOT copied into it +(the body already holds it; duplicating a blob to index fields around it would +be waste). + +Relationship to 0069: that migration DROPPED `notes.metadata`, a JSONB column +which only ever held person/place/list entity fields, when those surfaces were +removed. This is not a revival of it — different name, different purpose, and +nothing reads the old shape. The column is named `data` rather than `metadata` +because `metadata` collides with SQLAlchemy's declarative `Base.metadata`, which +is why the old model had to map an awkward `entity_metadata` attribute onto it. + +Nullable with no backfill, deliberately: rows written before this migration keep +working because the service falls back to parsing the body when `data` is +absent. That means no migration deadline and no risk of a backfill mangling a +hand-edited body. + +Downgrade drops the index and the column. Any structured fields it held remain +recoverable from the body convention, which is the same source they mirror. +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import JSONB + + +revision = "0070" +down_revision = "0069" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("notes", sa.Column("data", JSONB, nullable=True)) + # GIN supports containment (`data @> '{"locations":[{"repo":"x"}]}'`), which + # covers exact repo/path/symbol and language lookups. Path PREFIX matching + # ("everything under frontend/src/") is not an index-served operation here + # and still filters after the fact — acceptable while snippet counts are + # small, and a generated column is the escape hatch if that changes. + op.create_index( + "ix_notes_data_gin", "notes", ["data"], postgresql_using="gin", + ) + + +def downgrade() -> None: + op.drop_index("ix_notes_data_gin", table_name="notes") + op.drop_column("notes", "data") diff --git a/src/scribe/models/note.py b/src/scribe/models/note.py index 6682c6d..1b85701 100644 --- a/src/scribe/models/note.py +++ b/src/scribe/models/note.py @@ -69,6 +69,13 @@ class Note(Base, TimestampMixin, SoftDeleteMixin): # notes keep the 'work' default and ignore it. Orthogonal to note_type # (which is the note/entity axis). task_kind: Mapped[str] = mapped_column(Text, default="work", server_default="work") + # Queryable structured fields for typed records — currently snippets, whose + # name/language/signature/locations live here so they can be INDEXED. The + # body keeps the same facts in readable markdown and remains what gets + # embedded; this is a mirror for querying, not the source of truth for + # display. NULL on every row written before migration 0070, so readers fall + # back to parsing the body (see services/snippets.snippet_fields). + data: Mapped[dict | None] = mapped_column(JSONB, nullable=True) __table_args__ = ( Index("ix_notes_tags", "tags", postgresql_using="gin"), @@ -79,6 +86,9 @@ class Note(Base, TimestampMixin, SoftDeleteMixin): Index("ix_notes_milestone_id", "milestone_id"), Index("ix_notes_note_type", "note_type"), Index("ix_notes_arose_from_id", "arose_from_id"), + # Containment queries into `data` — e.g. which snippets name a given + # repo/path in their locations. See migration 0070. + Index("ix_notes_data_gin", "data", postgresql_using="gin"), ) @property diff --git a/src/scribe/services/notes.py b/src/scribe/services/notes.py index 3df0486..f36d08b 100644 --- a/src/scribe/services/notes.py +++ b/src/scribe/services/notes.py @@ -65,6 +65,7 @@ async def create_note( note_type: str = "note", task_kind: str = "work", arose_from_id: int | None = None, + data: dict | None = None, ) -> Note: # Validate status/priority here so the MCP create_task path (which passes # them straight through) can't persist an out-of-enum value that the REST @@ -108,6 +109,7 @@ async def create_note( note_type=note_type, task_kind=task_kind, arose_from_id=arose_from_id, + data=data, ) session.add(note) await session.commit() diff --git a/src/scribe/services/snippets.py b/src/scribe/services/snippets.py index 0c4636e..94a37b5 100644 --- a/src/scribe/services/snippets.py +++ b/src/scribe/services/snippets.py @@ -250,11 +250,78 @@ def parse_snippet_fields( return fields +# --- the queryable mirror (notes.data, migration 0070) ----------------------- + +# Fields kept in `data`. Code is deliberately absent: the body already holds it, +# and copying a blob into the column we index *around* would be pure weight. +_DATA_FIELDS = ("name", "when_to_use", "signature", "language", "locations") + + +def compose_data( + *, + name: str = "", + when_to_use: str = "", + signature: str = "", + language: str = "", + locations: list[dict] | None = None, +) -> dict: + """Build the `notes.data` mirror of a snippet's structured fields. + + Same facts as the body convention, in a shape Postgres can index — so + "which snippets live in this path?" is a containment query rather than a + regex over every body. Empty values are omitted so the column stays sparse + and containment matches don't trip over blanks. + """ + out: dict = {} + for key, value in ( + ("name", (name or "").strip()), + ("when_to_use", (when_to_use or "").strip()), + ("signature", (signature or "").strip()), + ("language", (language or "").strip().lower()), + ): + if value: + out[key] = value + locs = _normalize_locations(locations) + if locs: + out["locations"] = locs + return out + + +def snippet_fields(note) -> dict: + """Structured fields for a snippet, preferring the indexed `data` column and + falling back to parsing the body. + + Both paths must agree, because rows written before migration 0070 have no + `data` and are never backfilled — a hand-edited body is the authority for + those, and there is no deadline by which they must be converted. `code` only + ever comes from the body, since `data` doesn't carry it. + """ + parsed = parse_snippet_fields(note.title, note.body, note.tags) + stored = getattr(note, "data", None) + if not stored: + return parsed + merged = dict(parsed) + for key in _DATA_FIELDS: + if stored.get(key): + merged[key] = stored[key] + # Keep the single-location back-compat mirror consistent with whichever + # location list won. + locs = merged.get("locations") or [] + merged["repo"] = locs[0]["repo"] if locs else "" + merged["path"] = locs[0]["path"] if locs else "" + merged["symbol"] = locs[0]["symbol"] if locs else "" + return merged + + def snippet_to_dict(note) -> dict: """Note serialization plus a parsed ``snippet`` sub-object of structured - fields, so callers get both the raw record and the typed view.""" + fields, so callers get both the raw record and the typed view. + + The raw `data` column is intentionally NOT exposed: it mirrors what + ``snippet`` already reports, and shipping both would give API consumers two + sources of truth for the same facts.""" data = note.to_dict() - data["snippet"] = parse_snippet_fields(note.title, note.body, note.tags) + data["snippet"] = snippet_fields(note) return data @@ -278,17 +345,24 @@ async def create_snippet( """Create a snippet note (embedded on create for immediate recall). Returns the created Note. Pass ``locations`` for the multi-location case; the single ``repo``/``path``/``symbol`` are the one-location shorthand.""" + if locations is None: + locations = [{"repo": repo, "path": path, "symbol": symbol}] note = await notes_svc.create_note( user_id, title=compose_title(name, when_to_use), body=compose_body( code=code, language=language, signature=signature, - when_to_use=when_to_use, repo=repo, path=path, symbol=symbol, - locations=locations, + when_to_use=when_to_use, locations=locations, ), note_type=SNIPPET_NOTE_TYPE, tags=compose_tags(language, tags), project_id=project_id, + # The indexed mirror of the same fields (0070). Written together with the + # body so the two can never describe different things. + data=compose_data( + name=name, when_to_use=when_to_use, signature=signature, + language=language, locations=locations, + ), ) _embed_snippet(note) return note @@ -379,7 +453,7 @@ async def update_snippet( f"for edit access, or record your own version" ) - cur = parse_snippet_fields(note.title, note.body, note.tags) + cur = snippet_fields(note) overlay = { "name": name, "code": code, "language": language, "signature": signature, "when_to_use": when_to_use, @@ -405,6 +479,13 @@ async def update_snippet( signature=merged["signature"], when_to_use=merged["when_to_use"], locations=merged_locations, ), + # Re-derived from the same merged field set as the body, so an edit can't + # leave the indexed mirror describing the previous version. + "data": compose_data( + name=merged["name"], when_to_use=merged["when_to_use"], + signature=merged["signature"], language=merged["language"], + locations=merged_locations, + ), } # Recompute tags: keep any non-language, non-marker tags the note already had # (or the caller's replacement set), then re-derive language + marker. @@ -511,10 +592,8 @@ async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]): continue sources.append(s) - tgt_fields = parse_snippet_fields(target.title, target.body, target.tags) - parsed_sources = [ - (parse_snippet_fields(s.title, s.body, s.tags), s.tags) for s in sources - ] + tgt_fields = snippet_fields(target) + parsed_sources = [(snippet_fields(s), s.tags) for s in sources] locations, extra_tags = merge_snippet_fields(tgt_fields, target.tags, parsed_sources) # Owner-scoped write, authorised above — same reason as update_snippet. @@ -526,6 +605,14 @@ async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]): locations=locations, ), tags=compose_tags(tgt_fields["language"], extra_tags), + # The survivor's location set grew, so its mirror has to grow with it — + # otherwise a merged snippet would be unfindable at the very call sites + # the merge just recorded. + data=compose_data( + name=tgt_fields["name"], when_to_use=tgt_fields["when_to_use"], + signature=tgt_fields["signature"], language=tgt_fields["language"], + locations=locations, + ), ) if updated is None: return None diff --git a/tests/test_mcp_tool_snippets.py b/tests/test_mcp_tool_snippets.py index adadc80..4c10f95 100644 --- a/tests/test_mcp_tool_snippets.py +++ b/tests/test_mcp_tool_snippets.py @@ -24,6 +24,10 @@ def _fake_snippet(user_id: int = 7): # decide whether to attach a shared/owner marker; an auto-MagicMock would read # as another user's record and send them off to look up a username. n.user_id = user_id + # Explicitly None, not an auto-attribute: snippet_fields prefers `data` when + # truthy, and a MagicMock is truthy — every parsed field would come back as a + # MagicMock instead of a string. + n.data = None n.to_dict.return_value = { "id": 1, "title": n.title, "note_type": "snippet", "tags": n.tags, } diff --git a/tests/test_services_snippets.py b/tests/test_services_snippets.py index 55487a9..31ad56f 100644 --- a/tests/test_services_snippets.py +++ b/tests/test_services_snippets.py @@ -158,6 +158,95 @@ def test_merge_snippet_fields_unions_locations_and_tags(): assert extra == ["core", "helper"] +# --- the queryable mirror (notes.data, migration 0070) ----------------------- + +def test_compose_data_keeps_only_populated_fields_and_no_code(): + got = s.compose_data( + name="formatDuration", when_to_use="humanize a ms count", + signature="f(ms) -> string", language="TS", + locations=[{"repo": "web", "path": "a.ts", "symbol": "f"}, + {"repo": "", "path": "", "symbol": ""}], + ) + assert got == { + "name": "formatDuration", + "when_to_use": "humanize a ms count", + "signature": "f(ms) -> string", + "language": "ts", + "locations": [{"repo": "web", "path": "a.ts", "symbol": "f"}], + } + # Code stays in the body — duplicating a blob into the column we index + # around would be pure weight. + assert "code" not in got + + +def test_compose_data_omits_blanks_entirely(): + """A sparse column keeps containment matches from tripping over empties.""" + assert s.compose_data(name="x") == {"name": "x"} + assert s.compose_data() == {} + + +class _Note: + """Minimal stand-in — snippet_fields only reads title/body/tags/data.""" + + def __init__(self, title="", body="", tags=None, data=None): + self.title, self.body, self.tags, self.data = title, body, tags or [], data + + +def test_snippet_fields_prefers_the_data_column(): + body = s.compose_body(code="x = 1", language="py", signature="old()", + locations=[{"repo": "old", "path": "o.py", "symbol": "o"}]) + note = _Note( + title="thing — old blurb", body=body, tags=["py", "snippet"], + data=s.compose_data(name="thing", when_to_use="new blurb", + signature="new()", language="py", + locations=[{"repo": "new", "path": "n.py", "symbol": "n"}]), + ) + got = s.snippet_fields(note) + assert got["when_to_use"] == "new blurb" + assert got["signature"] == "new()" + assert got["locations"] == [{"repo": "new", "path": "n.py", "symbol": "n"}] + # The back-compat single-location mirror follows whichever list won. + assert (got["repo"], got["path"], got["symbol"]) == ("new", "n.py", "n") + # Code has no home in `data`, so it still comes from the body. + assert got["code"] == "x = 1" + + +def test_snippet_fields_falls_back_to_the_body_when_data_is_absent(): + """Rows written before 0070 are never backfilled, so the body stays + authoritative for them — with no deadline to convert.""" + body = s.compose_body(code="y = 2", language="rb", signature="g()", + when_to_use="do a thing", + locations=[{"repo": "r", "path": "p.rb", "symbol": "g"}]) + got = s.snippet_fields(_Note(title="g — do a thing", body=body, + tags=["rb", "snippet"], data=None)) + assert got["signature"] == "g()" + assert got["language"] == "rb" + assert got["locations"] == [{"repo": "r", "path": "p.rb", "symbol": "g"}] + assert got["code"] == "y = 2" + + +def test_data_and_body_round_trip_to_the_same_fields(): + """The two representations must agree — they're written together, and a + disagreement would make a snippet read one way and query another.""" + fields = dict(name="debounce", when_to_use="rate-limit a callback", + signature="debounce(fn, ms)", language="ts") + locs = [{"repo": "web", "path": "src/util.ts", "symbol": "debounce"}] + from_body = s.snippet_fields(_Note( + title=s.compose_title(fields["name"], fields["when_to_use"]), + body=s.compose_body(code="const x = 1", locations=locs, **fields), + tags=s.compose_tags(fields["language"]), + )) + from_data = s.snippet_fields(_Note( + title=s.compose_title(fields["name"], fields["when_to_use"]), + body=s.compose_body(code="const x = 1", locations=locs, **fields), + tags=s.compose_tags(fields["language"]), + data=s.compose_data(locations=locs, **fields), + )) + for key in ("name", "when_to_use", "signature", "language", "locations", + "repo", "path", "symbol", "code"): + assert from_body[key] == from_data[key], key + + def test_snippet_to_dict_includes_parsed_fields(): class FakeNote: title = "debounce — rate-limit" diff --git a/tests/test_shared_write_access.py b/tests/test_shared_write_access.py index c68abb7..99e498a 100644 --- a/tests/test_shared_write_access.py +++ b/tests/test_shared_write_access.py @@ -24,6 +24,9 @@ def _snippet(id=1, owner=9): n.tags = ["ts", "snippet"] n.note_type = "snippet" n.deleted_at = None + # Explicitly None — snippet_fields prefers `data` when truthy, and an + # auto-MagicMock attribute is truthy (see note 2109). + n.data = None return n