bvandeusenandClaude Opus 5
5c51e29f26
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 57s
CI & Build / Build & push image (push) Successful in 29s
A note or task created through MCP was not semantically searchable until the next restart's backfill ran. Embedding fired at the five REST route handlers and nowhere else; the MCP tools call the service directly, so they skipped it. The shape of this bug is the reason to care: it is invisible on an instance that redeploys constantly (this one does, per rule 46) and permanent on one that doesn't. Rule 115 — the product has to stand up for the install that restarts twice a year, not just for the one that restarts hourly. Moved to services/notes.embed_note(), called from create_note and update_note, and deleted from all five routes. Every caller — REST, MCP, recurrence, snippets — now gets it by construction rather than by remembering. Two things fall out of having one implementation instead of six: - It uses note.user_id, the OWNER. The routes were inconsistent: some passed the caller's uid, some the owner's. On a shared record the caller's id mints a second embedding row that nothing reads. - services/snippets.py's _embed_snippet existed only because snippets are created via MCP and the routes couldn't cover them. Every one of its four call sites goes through notes_svc, so the helper and its four calls are gone, along with the eight test patches that existed to neutralise it. RuntimeError (no running loop — unit tests, scripts) and any indexing failure are both swallowed: a write that succeeded must not be failed by its index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
111 lines
4.8 KiB
Python
111 lines
4.8 KiB
Python
"""Merge records what it absorbed (#2087).
|
|
|
|
Merging keeps the target's scalar fields, unions locations + tags, and trashes
|
|
the sources. Without provenance the fact that a variant ever existed survives
|
|
only in the trash — and only for someone who already knew to go looking. So the
|
|
survivor carries `merged_from`, written to the body and the indexed mirror from
|
|
one value, and ordinary edits must carry it forward rather than erase it.
|
|
"""
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from scribe.services import snippets as s
|
|
|
|
|
|
def _snippet(id=1, owner=9, body=None, data=None, tags=None):
|
|
n = MagicMock()
|
|
n.id = id
|
|
n.user_id = owner
|
|
n.title = "formatDuration — humanize a millisecond count"
|
|
n.body = body if body is not None else s.compose_body(code="x = 1", language="ts")
|
|
n.tags = tags if tags is not None else ["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 (note 2109).
|
|
n.data = data
|
|
return n
|
|
|
|
|
|
async def _run_merge(target, sources, source_ids):
|
|
"""Drive merge_snippets with the DB stubbed out; return update_note's kwargs."""
|
|
by_id = {target.id: target, **{x.id: x for x in sources}}
|
|
|
|
async def fake_get(_uid, sid):
|
|
return by_id.get(sid)
|
|
|
|
with patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)), \
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \
|
|
patch.object(s.notes_svc, "update_note",
|
|
AsyncMock(return_value=target)) as mock_update, \
|
|
patch("scribe.services.trash.delete", AsyncMock(return_value=object())):
|
|
await s.merge_snippets(7, target.id, source_ids)
|
|
return mock_update.await_args.kwargs
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_merge_records_what_it_absorbed_in_body_and_mirror():
|
|
kwargs = await _run_merge(
|
|
_snippet(id=1), [_snippet(id=2), _snippet(id=3)], [2, 3],
|
|
)
|
|
assert "**Merged from:** #2, #3" in kwargs["body"]
|
|
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2, 3]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_merge_accumulates_across_merges():
|
|
"""A target merged twice keeps both histories — the second merge must not
|
|
replace the record of the first."""
|
|
target = _snippet(id=1, data=s.compose_data(name="f", merged_from=[2]))
|
|
kwargs = await _run_merge(target, [_snippet(id=3)], [3])
|
|
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2, 3]
|
|
assert "**Merged from:** #2, #3" in kwargs["body"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_merge_does_not_record_a_skipped_cross_owner_source():
|
|
"""A source owned by someone else is skipped, not folded in (#231) — so it
|
|
must not appear in the provenance either, or the record would claim to
|
|
contain something it never absorbed."""
|
|
kwargs = await _run_merge(
|
|
_snippet(id=1, owner=9),
|
|
[_snippet(id=2, owner=9), _snippet(id=3, owner=42)],
|
|
[2, 3],
|
|
)
|
|
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2]
|
|
assert "#3" not in kwargs["body"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_ordinary_edit_carries_provenance_forward():
|
|
"""Only a merge may add to it, but nothing else may drop it — update
|
|
recomposes body and mirror from scratch, so an omission here would silently
|
|
erase the history on the next unrelated edit."""
|
|
note = _snippet(id=1, data=s.compose_data(name="f", language="ts",
|
|
merged_from=[2, 3]))
|
|
with patch.object(s, "get_snippet", AsyncMock(return_value=note)), \
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \
|
|
patch.object(s.notes_svc, "update_note",
|
|
AsyncMock(return_value=note)) as mock_update:
|
|
await s.update_snippet(7, 1, signature="f(ms) -> string")
|
|
|
|
kwargs = mock_update.await_args.kwargs
|
|
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [2, 3]
|
|
assert "**Merged from:** #2, #3" in kwargs["body"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_pre_0070_row_keeps_provenance_through_the_body():
|
|
"""Rows written before the `data` column are never backfilled, so the body
|
|
line is their only copy — and it has to survive an edit too."""
|
|
body = s.compose_body(code="x = 1", language="ts", merged_from=[5])
|
|
note = _snippet(id=1, body=body, data=None)
|
|
with patch.object(s, "get_snippet", AsyncMock(return_value=note)), \
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \
|
|
patch.object(s.notes_svc, "update_note",
|
|
AsyncMock(return_value=note)) as mock_update:
|
|
await s.update_snippet(7, 1, when_to_use="humanize a ms count")
|
|
|
|
assert "**Merged from:** #5" in mock_update.await_args.kwargs["body"]
|