feat(supersession): declare it — supersedes on both write paths
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 16s
CI & Build / Python tests (push) Failing after 31s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 16s
CI & Build / Python tests (push) Failing after 31s
CI & Build / Build & push image (push) Skipped
Step 2 of #278. Records and reads the claim; the demotion that makes it matter is step 3. `services/supersession.py` with set/get on both directions, following the set_record_systems shape since this is the same kind of mutable M2M at the tool/route layer rather than inside notes_svc. ## Both directions are exposed, and only one is obvious `supersedes` is what the author claimed. `superseded_by` is what a READER needs and what the note itself cannot know — a stale record handed over with no marker gets acted on confidently, which is worse than never surfacing it. So get_note carries it, says so in its docstring, and adds a plain-language line telling the reader to open the newer note first. Both are OMITTED when empty rather than serialised as empty lists. A field that always says nothing trains readers to skip fields — the lesson consolidated_at cost, removed in the previous commit. ## Refuse vs drop, which is the one real judgement here Dropped silently: a target that doesn't exist, is trashed, is the note itself, or would close a cycle. Each is a claim with no subject or no meaning; none is something the caller can act on. REFUSED with PermissionError: a target the caller can read but not write. That is the single case where the caller could believe they succeeded and be wrong in a way that matters — demoting someone else's record out of their retrieval is damage invisible from the outside, with no symptom for the owner to trace. Rule #47, and PermissionError because services/snippets.py already uses it for read-but-not-write with both surfaces catching it. The PATCH/PUT routes scope by the CALLER, not owner_uid: an editor-share holder may edit the note and must not thereby inherit the owner's write access to whatever they name as superseded. ## Cycles A ring claims every member is obsolete. Under flat demotion that demotes them all equally, so the set drops out of ranked retrieval together with nothing in the data saying why. Refused by walking the existing graph from the proposed target — iteratively with a visited set, because the graph is user-supplied and a deep chain must not become a stack overflow on a write path. The visited set also makes the walk terminate on a ring that already exists, which is pinned by its own test rather than trusted. Both surfaces (#33), the instruction surface per #119 — framed as the third answer beside update-instead and force=true: not everything resembling an existing record should be folded into it, and not everything distinct should compete with it forever. Refs #278
This commit is contained in:
@@ -0,0 +1,148 @@
|
||||
"""The supersession claim — who may make it, and what it refuses.
|
||||
|
||||
Step 2 of #278. Ranking behaviour is step 3; this covers only recording and
|
||||
reading the relation.
|
||||
|
||||
The cycle tests are the ones worth reading. Under FLAT demotion a ring of
|
||||
records that supersede each other claims every member is obsolete, so all of
|
||||
them get demoted equally and the whole set drops out of ranked retrieval
|
||||
together — with nothing in the data saying why.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import supersession
|
||||
|
||||
|
||||
def _session(scalars_sequence=None, get_returns=None):
|
||||
"""A mocked async_session whose execute() yields successive scalar lists."""
|
||||
s = AsyncMock()
|
||||
s.__aenter__ = AsyncMock(return_value=s)
|
||||
s.__aexit__ = AsyncMock(return_value=False)
|
||||
|
||||
results = []
|
||||
for scalars in scalars_sequence or []:
|
||||
r = MagicMock()
|
||||
r.scalars.return_value.all.return_value = scalars
|
||||
results.append(r)
|
||||
s.execute = AsyncMock(side_effect=results or None)
|
||||
s.get = AsyncMock(side_effect=get_returns) if get_returns else AsyncMock()
|
||||
s.commit = AsyncMock()
|
||||
s.add = MagicMock()
|
||||
return s
|
||||
|
||||
|
||||
def _live_note(note_id=1):
|
||||
n = MagicMock()
|
||||
n.id, n.deleted_at = note_id, None
|
||||
return n
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_caller_who_cannot_write_the_note_gets_none():
|
||||
with patch("scribe.services.supersession.access.can_write_note",
|
||||
AsyncMock(return_value=False)):
|
||||
assert await supersession.set_supersedes(7, 1, [2]) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_superseding_a_note_you_can_only_READ_is_refused_not_dropped():
|
||||
"""The one case that raises rather than silently skipping.
|
||||
|
||||
Demoting someone else's record out of their retrieval is damage that is
|
||||
invisible from the outside — the caller would believe it worked, and the
|
||||
owner would have no symptom to trace. Rule #47.
|
||||
"""
|
||||
# writable for the superseder (id 1), not for the target (id 2)
|
||||
writable = AsyncMock(side_effect=lambda uid, nid: nid == 1)
|
||||
session = _session(get_returns=[_live_note(2)])
|
||||
with patch("scribe.services.supersession.access.can_write_note", writable), \
|
||||
patch("scribe.services.supersession.async_session", return_value=session):
|
||||
with pytest.raises(PermissionError, match="not yours to supersede"):
|
||||
await supersession.set_supersedes(7, 1, [2])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_self_supersession_is_dropped_silently():
|
||||
"""Meaningless rather than dangerous, and the DB CHECK refuses it anyway —
|
||||
so it is a drop, not an error the caller has to handle."""
|
||||
session = _session(scalars_sequence=[[]])
|
||||
with patch("scribe.services.supersession.access.can_write_note",
|
||||
AsyncMock(return_value=True)), \
|
||||
patch("scribe.services.supersession.async_session", return_value=session):
|
||||
assert await supersession.set_supersedes(7, 5, [5]) == []
|
||||
session.add.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_trashed_target_is_dropped():
|
||||
"""A claim needs a subject. `get` returns the row, `deleted_at` says it is
|
||||
in the trash, so there is nothing to demote."""
|
||||
trashed = MagicMock()
|
||||
trashed.deleted_at = "2026-08-08"
|
||||
session = _session(scalars_sequence=[[]], get_returns=[trashed])
|
||||
with patch("scribe.services.supersession.access.can_write_note",
|
||||
AsyncMock(return_value=True)), \
|
||||
patch("scribe.services.supersession.async_session", return_value=session):
|
||||
assert await supersession.set_supersedes(7, 1, [2]) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_direct_cycle_is_refused():
|
||||
"""B already supersedes A; A may not now supersede B.
|
||||
|
||||
Walk from the proposed target (B) and see whether it reaches the proposer
|
||||
(A). It does — B -> A — so the edge would close a ring.
|
||||
"""
|
||||
session = _session(scalars_sequence=[[1]]) # B supersedes A(=1)
|
||||
with patch("scribe.services.supersession.async_session", return_value=session):
|
||||
assert await supersession._closes_a_cycle(session, 1, 2) is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_indirect_cycle_is_refused():
|
||||
"""A -> B -> C exists; C may not supersede A.
|
||||
|
||||
Walking from A follows A -> B, then B -> C... and the walk must reach the
|
||||
proposer. Here the proposer is C and the target is A, so: A -> B -> C.
|
||||
"""
|
||||
session = _session(scalars_sequence=[[2], [3]]) # A->B, B->C
|
||||
with patch("scribe.services.supersession.async_session", return_value=session):
|
||||
assert await supersession._closes_a_cycle(session, 3, 1) is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_chain_that_does_not_loop_is_allowed():
|
||||
"""A -> B exists; C may supersede A. Walking from A reaches only B."""
|
||||
session = _session(scalars_sequence=[[2], []])
|
||||
with patch("scribe.services.supersession.async_session", return_value=session):
|
||||
assert await supersession._closes_a_cycle(session, 3, 1) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_cycle_walk_terminates_on_an_existing_ring():
|
||||
"""Defensive: if a ring somehow exists (written directly to the DB), the
|
||||
walk must not spin. The visited set is what guarantees it, and this pins
|
||||
that guarantee rather than trusting it."""
|
||||
# 1 -> 2, 2 -> 1: a ring that does not contain the proposer (99).
|
||||
session = _session(scalars_sequence=[[2], [1], []])
|
||||
with patch("scribe.services.supersession.async_session", return_value=session):
|
||||
assert await supersession._closes_a_cycle(session, 99, 1) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_superseded_ids_is_empty_for_an_empty_candidate_set():
|
||||
"""Ranking calls this per query. An empty candidate set must not become a
|
||||
`WHERE id IN ()`, which Postgres accepts and every reader misreads."""
|
||||
assert await supersession.superseded_ids([]) == set()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reads_are_empty_when_the_caller_cannot_read_the_note():
|
||||
with patch("scribe.services.supersession.access.can_read_note",
|
||||
AsyncMock(return_value=False)):
|
||||
assert await supersession.get_supersedes(7, 1) == []
|
||||
assert await supersession.get_superseded_by(7, 1) == []
|
||||
Reference in New Issue
Block a user