Files
FabledScribe/tests/test_snippet_unmerge.py
T
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
fix(embeddings): embed in the service, so every caller gets it (#2056)
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
2026-07-31 22:46:57 -04:00

181 lines
7.5 KiB
Python

"""Tests for un-merge (#2165) — reversing one source out of a merged survivor.
The design question this task said to settle first was how to subtract without
stripping call sites the survivor legitimately owns. The answer is per-source
attribution recorded AT MERGE TIME: each `merged_from` entry holds only what
that source actually added. These tests pin that rule, and the refusal that
guards the case where the attribution isn't there.
"""
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from scribe.services import snippets as s
def _loc(path, repo="r", symbol=""):
return {"repo": repo, "path": path, "symbol": symbol}
def _survivor(locations, merged_from, tags=None, owner=7):
"""A survivor note whose `data` carries locations + merge provenance."""
return SimpleNamespace(
id=1, user_id=owner, note_type="snippet", deleted_at=None,
title="f — does a thing", tags=tags or ["python", "snippet"],
body=s.compose_body(code="x = 1", language="python", locations=locations,
merged_from=merged_from),
data=s.compose_data(name="f", when_to_use="does a thing", language="python",
code="x = 1", locations=locations,
merged_from=merged_from),
)
async def _run_unmerge(survivor, *, source_alive=None, restore=1):
"""Drive unmerge_snippet with the DB stubbed; return update_note's kwargs."""
async def fake_get(_uid, sid):
if sid == survivor.id:
return survivor
return source_alive
with (
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
patch("scribe.services.trash.restore_entity", AsyncMock(return_value=restore)),
patch.object(s.notes_svc, "update_note",
AsyncMock(return_value=survivor)) as upd,
):
await s.unmerge_snippet(7, 1, 2)
return upd.await_args.kwargs
# --- the subtraction ------------------------------------------------------
async def test_unmerge_strips_only_what_the_source_contributed():
"""The survivor's own location survives; the source's is removed."""
mine, theirs = _loc("mine.py"), _loc("theirs.py")
survivor = _survivor(
[mine, theirs],
[{"id": 2, "locations": [theirs], "tags": ["helper"]}],
tags=["python", "snippet", "helper", "core"],
)
kwargs = await _run_unmerge(survivor)
assert kwargs["data"]["locations"] == [mine]
assert "helper" not in kwargs["tags"]
assert "core" in kwargs["tags"]
async def test_a_location_the_survivor_also_owned_is_never_stripped():
"""The central hazard the task named. If a source brought a location the
survivor ALREADY had, merge attributes nothing to it — so reversing must
leave that call site in place."""
shared = _loc("shared.py")
survivor = _survivor([shared], [{"id": 2, "locations": [], "tags": ["t"]}])
kwargs = await _run_unmerge(survivor)
assert kwargs["data"]["locations"] == [shared]
async def test_the_reversed_entry_leaves_the_provenance_list():
survivor = _survivor(
[_loc("a.py"), _loc("b.py")],
[{"id": 2, "locations": [_loc("b.py")]}, {"id": 3, "locations": []}],
)
kwargs = await _run_unmerge(survivor)
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [3]
assert "#2" not in kwargs["body"]
# The other source's history is untouched — un-merge reverses one thing.
assert "**Merged from:** #3" in kwargs["body"]
async def test_unmerging_the_last_source_clears_the_provenance_line():
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [], "tags": ["t"]}])
kwargs = await _run_unmerge(survivor)
assert "Merged from" not in kwargs["body"]
# --- restoring the source -------------------------------------------------
async def test_an_already_restored_source_is_repaired_not_refused():
"""The scenario that motivated the feature: the operator restored the source
from the trash by hand, so both records claim its call sites and nothing ever
stripped the survivor's copy. Un-merge must fix that, not reject it."""
theirs = _loc("theirs.py")
survivor = _survivor([_loc("mine.py"), theirs],
[{"id": 2, "locations": [theirs]}])
alive = SimpleNamespace(id=2, user_id=7, note_type="snippet", deleted_at=None,
title="g — x", tags=["snippet"], body="", data=None)
with patch("scribe.services.trash.restore_entity", AsyncMock()) as revive:
kwargs = await _run_unmerge(survivor, source_alive=alive)
# Nothing to revive — it's already alive — but the subtraction still happens.
revive.assert_not_called()
assert kwargs["data"]["locations"] == [_loc("mine.py")]
async def test_a_purged_source_is_refused_and_the_survivor_is_untouched():
"""If the source can't come back, stripping the survivor would lose the
locations entirely — no record would claim them."""
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [_loc("a.py")]}])
async def fake_get(_uid, sid):
return survivor if sid == 1 else None
with (
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
patch("scribe.services.trash.restore_entity", AsyncMock(return_value=None)),
patch.object(s.notes_svc, "update_note", AsyncMock()) as upd,
):
with pytest.raises(s.UnmergeError, match="purged"):
await s.unmerge_snippet(7, 1, 2)
upd.assert_not_called()
# --- refusals -------------------------------------------------------------
async def test_an_entry_without_attribution_is_refused_not_guessed():
"""Bare-id provenance comes from parsing the body, which can only hold ids.
Subtracting a guess could strip call sites the survivor owns — so refuse and
say what to do instead."""
survivor = _survivor([_loc("a.py")], [2])
async def fake_get(_uid, sid):
return survivor if sid == 1 else None
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()) as upd,
):
with pytest.raises(s.UnmergeError, match="provenance"):
await s.unmerge_snippet(7, 1, 2)
upd.assert_not_called()
async def test_unmerging_something_never_absorbed_is_refused():
survivor = _survivor([_loc("a.py")], [{"id": 99, "locations": []}])
with (
patch.object(s, "get_snippet", AsyncMock(return_value=survivor)),
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
):
with pytest.raises(s.UnmergeError, match="no record of absorbing"):
await s.unmerge_snippet(7, 1, 2)
async def test_unmerge_requires_write_access():
"""Same rule as merge: a read-only share can see the record, not rearrange it."""
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": []}])
with (
patch.object(s, "get_snippet", AsyncMock(return_value=survivor)),
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)),
):
with pytest.raises(PermissionError):
await s.unmerge_snippet(7, 1, 2)
async def test_unmerge_on_a_missing_survivor_returns_none():
with patch.object(s, "get_snippet", AsyncMock(return_value=None)):
assert await s.unmerge_snippet(7, 1, 2) is None