feat(supersession): demote what a later note overtook, and label it
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 10s
CI & Build / integration (push) Successful in 18s
CI & Build / Python tests (push) Failing after 30s
CI & Build / Build & push image (push) Skipped

Step 3 of #278. First step with visible effect.

## Where the demotion happens, and why not in SQL

Applied AFTER the pgvector fetch, over an over-fetched candidate set, not as
part of the ORDER BY.

Ordering by `distance + penalty` would be exact and would turn an indexed
top-k into a scan-and-sort of every embedded note — the HNSW index from
migration 0067 can only serve a raw-distance ordering. So the query fetches
3x the requested rows by raw distance and the re-rank happens in Python.

Demoting after a LIMIT k with no over-fetch would have been theatre: the cut
already happened, so a superseded record pushed down still sits in the results
and the live record that should have replaced it was never fetched.

The cost is stated in the code: a live record outside the over-fetch window
cannot be promoted in. With a 0.05 penalty against neighbours ~0.014 apart,
that needs the true answer more than three ranks down, which no observed query
approaches.

## Demote, never hide — enforced in three places

The penalty applies to the RANKING score, not to the relevance threshold. The
floor decides whether a record is relevant at all; the penalty decides which
relevant record comes first. Applying it to the floor would drop a superseded
record out of the results entirely, which is the one thing this must not do.

It is small on purpose. Supersession is a claim about SOME of a record's
content, so one that strongly answers a question nothing else answers still
surfaces — just behind anything comparable that is current. Its test asserts
both bounds, the upper one citing the operator's constraint rather than an
optimisation.

And every test here that could be satisfied by dropping a record instead
asserts the record is still present.

## The dedup gate opts out

A superseded record is still a duplicate of what you are about to write — the
claim is that it is no longer current, not that it is gone. Demoting it there
would let the same note be recorded a second time, and the second copy would be
the one nothing warns about.

## The label

Auto-inject marks a superseded line SUPERSEDED with a pointer to check the
later record. One query for the whole menu. An agent handed stale material with
nothing marking it acts on it with full confidence, which is worse than never
having surfaced it — the ranking is only half the fix.

Fails open: a supersession lookup error returns unpenalised results rather than
none, because unpenalised ranking is the behaviour that shipped for months and
a broken search is not.

Refs #278
This commit is contained in:
2026-08-08 02:00:06 -04:00
parent 984407f931
commit f20c019f2a
4 changed files with 266 additions and 2 deletions
+154
View File
@@ -0,0 +1,154 @@
"""Demotion: superseded records rank behind their equals, and are never hidden.
Step 3 of #278. The distinction these tests exist to protect is DEMOTE vs
FILTER. The operator was explicit:
"the failure is pollution, not existence"
Hiding a superseded record would turn every one of them into something you must
already know exists in order to find, and would destroy "what did we think
then" — half the reason a log is kept. So every test here that could be
satisfied by dropping a record instead checks that it is still present.
"""
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from scribe.services.embeddings import (
_SUPERSESSION_PENALTY,
_apply_supersession_penalty,
)
def _note(note_id: int):
n = MagicMock()
n.id = note_id
return n
def _stale(*ids):
return patch(
"scribe.services.supersession.superseded_ids",
AsyncMock(return_value=set(ids)),
)
@pytest.mark.asyncio
async def test_a_superseded_record_falls_behind_an_equal_live_one():
scored = [(0.70, _note(1)), (0.69, _note(2))] # 1 leads on raw score
with _stale(1):
out = await _apply_supersession_penalty(scored, limit=5)
assert [int(n.id) for _s, n in out] == [2, 1]
@pytest.mark.asyncio
async def test_a_strong_superseded_record_still_beats_a_weak_live_one():
"""Demote, not filter — and this is why the penalty is small.
Supersession is a claim about SOME of a record's content. One that strongly
answers a question nothing else answers should still surface, just behind
anything comparable that is current.
"""
scored = [(0.90, _note(1)), (0.50, _note(2))]
with _stale(1):
out = await _apply_supersession_penalty(scored, limit=5)
assert [int(n.id) for _s, n in out] == [1, 2]
assert out[0][0] == pytest.approx(0.90 - _SUPERSESSION_PENALTY)
@pytest.mark.asyncio
async def test_the_superseded_record_is_still_returned():
"""The whole point. A test that only checked ordering would pass just as
happily against an implementation that dropped it."""
scored = [(0.70, _note(1))]
with _stale(1):
out = await _apply_supersession_penalty(scored, limit=5)
assert [int(n.id) for _s, n in out] == [1]
@pytest.mark.asyncio
async def test_the_returned_score_is_the_adjusted_one():
"""Downstream gates must see the adjusted value — the auto-inject margin
band in particular, which exists to stop near-ties dragging in neighbours
and would otherwise re-tie exactly what this just separated."""
scored = [(0.70, _note(1)), (0.68, _note(2))]
with _stale(1):
out = await _apply_supersession_penalty(scored, limit=5)
by_id = {int(n.id): s for s, n in out}
assert by_id[1] == pytest.approx(0.65)
assert by_id[2] == pytest.approx(0.68)
@pytest.mark.asyncio
async def test_nothing_superseded_leaves_the_order_untouched():
scored = [(0.70, _note(1)), (0.69, _note(2)), (0.60, _note(3))]
with _stale():
out = await _apply_supersession_penalty(scored, limit=5)
assert [int(n.id) for _s, n in out] == [1, 2, 3]
@pytest.mark.asyncio
async def test_ties_keep_their_database_order():
"""Stable sort. Equal scores must not reshuffle per call — a menu that
reorders between identical queries reads as nondeterminism and sends
someone hunting for a bug that isn't there."""
scored = [(0.70, _note(1)), (0.70, _note(2)), (0.70, _note(3))]
with _stale():
out = await _apply_supersession_penalty(scored, limit=5)
assert [int(n.id) for _s, n in out] == [1, 2, 3]
@pytest.mark.asyncio
async def test_the_limit_is_applied_after_reordering():
"""Over-fetching is pointless if the cut happens first. Three candidates,
limit 2, and the demoted leader must be the one that falls out."""
scored = [(0.70, _note(1)), (0.69, _note(2)), (0.68, _note(3))]
with _stale(1):
out = await _apply_supersession_penalty(scored, limit=2)
assert [int(n.id) for _s, n in out] == [2, 3]
@pytest.mark.asyncio
async def test_a_failed_lookup_returns_unpenalised_results_not_none():
"""Fail OPEN, and the direction matters. Ranking without the penalty is the
behaviour that shipped for months; returning nothing would turn a
supersession hiccup into a broken search."""
scored = [(0.70, _note(1)), (0.69, _note(2))]
with patch("scribe.services.supersession.superseded_ids",
AsyncMock(side_effect=RuntimeError("db gone"))):
out = await _apply_supersession_penalty(scored, limit=5)
assert [int(n.id) for _s, n in out] == [1, 2]
assert out[0][0] == pytest.approx(0.70)
@pytest.mark.asyncio
async def test_an_empty_candidate_set_short_circuits():
"""No candidates means no lookup — this runs on every ranked query, and a
round trip to learn nothing is a round trip too many."""
called = AsyncMock(return_value=set())
with patch("scribe.services.supersession.superseded_ids", called):
assert await _apply_supersession_penalty([], limit=5) == []
called.assert_not_awaited()
def test_the_penalty_is_sized_to_reorder_a_cluster_not_shuffle_within_it():
"""Sized against a measurement, not by feel.
Measured 2026-08-07 (#2486): the notes competing on a dev-log's own title
phrase sat within ~0.014 of each other, spanning 0.6506 down to 0.6120. A
penalty smaller than that spread would move a record within a tie without
changing which one wins — the failure #2486 already proved cannot be tuned
away, because the neighbours are not barely passing, they are tied.
The upper bound is the operator's constraint, not an optimisation: a penalty
large enough to bury a superseded record outright is hiding by another name.
"""
assert _SUPERSESSION_PENALTY > 0.014, (
"must exceed the measured neighbour spread, or it reorders nothing"
)
assert _SUPERSESSION_PENALTY < 0.15, (
"must not bury a superseded record outright — that is hiding, which "
"the operator ruled out"
)