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
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:
@@ -281,6 +281,12 @@ async def find_duplicate_note(
|
||||
# would refuse their write and point them at something they may not
|
||||
# be able to edit.
|
||||
scope="own",
|
||||
# NOT demoted by supersession (#278). 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 here would let
|
||||
# the same note be recorded a second time, and the second copy would
|
||||
# be the one nothing warns about.
|
||||
demote_superseded=False,
|
||||
)
|
||||
for score, note in hits:
|
||||
# semantic_search_notes doesn't filter note_type — enforce it here so
|
||||
|
||||
@@ -86,6 +86,69 @@ def _cosine_similarity(a: list[float], b: list[float]) -> float:
|
||||
return dot / (mag_a * mag_b)
|
||||
|
||||
|
||||
# How much a superseded record is pushed down the ranking (#278).
|
||||
#
|
||||
# Chosen against a measurement, not by feel. On 2026-08-07 dev-log #2420 sat at
|
||||
# 0.6120 on a query made of its own title phrase, 8th, behind #1759 at 0.6506 —
|
||||
# a deficit of 0.039 to the top and ~0.014 to its nearest neighbours. A penalty
|
||||
# of 0.05 clears that whole band, so demoting a cluster's stale members actually
|
||||
# reorders it rather than shuffling within a tie.
|
||||
#
|
||||
# It is deliberately NOT large. Supersession is a claim about SOME of a record's
|
||||
# content, so a superseded note that strongly answers a question nothing else
|
||||
# answers should still surface — just behind anything comparable that is
|
||||
# current. A penalty big enough to bury it outright would be hiding by another
|
||||
# name, which is the thing the operator ruled out.
|
||||
_SUPERSESSION_PENALTY = 0.05
|
||||
|
||||
# Candidates fetched per requested result when a re-rank follows. Three ranks of
|
||||
# headroom is far more than a 0.05 penalty can move anything through in a corpus
|
||||
# whose neighbours sit ~0.01-0.02 apart.
|
||||
_SUPERSESSION_OVERFETCH = 3
|
||||
|
||||
|
||||
async def _apply_supersession_penalty(
|
||||
scored: list[tuple[float, "Note"]], limit: int
|
||||
) -> list[tuple[float, "Note"]]:
|
||||
"""Push superseded records below their equals, then take the top `limit`.
|
||||
|
||||
The penalty is applied to the RANKING score and the returned score, so
|
||||
downstream gates 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.
|
||||
|
||||
It is NOT applied 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 — hiding, which is the one thing this must not do.
|
||||
|
||||
Stable within a tie: Python's sort preserves the distance order the database
|
||||
already established, so equal-scoring records keep their original sequence
|
||||
rather than reshuffling per call.
|
||||
"""
|
||||
if not scored:
|
||||
return []
|
||||
from scribe.services.supersession import superseded_ids
|
||||
|
||||
try:
|
||||
stale = await superseded_ids([int(note.id) for _score, note in scored])
|
||||
except Exception:
|
||||
# Fail OPEN, and the direction matters: ranking without the penalty is
|
||||
# the behaviour that shipped for months. Returning nothing, or raising,
|
||||
# would turn a supersession-lookup hiccup into a broken search.
|
||||
logger.warning("Supersession lookup failed — ranking unpenalised", exc_info=True)
|
||||
return scored[:limit]
|
||||
|
||||
if not stale:
|
||||
return scored[:limit]
|
||||
adjusted = [
|
||||
(score - _SUPERSESSION_PENALTY if int(note.id) in stale else score, note)
|
||||
for score, note in scored
|
||||
]
|
||||
adjusted.sort(key=lambda pair: pair[0], reverse=True)
|
||||
return adjusted[:limit]
|
||||
|
||||
|
||||
def embedding_text(title: str | None, body: str | None) -> str:
|
||||
"""The document a record is embedded AS.
|
||||
|
||||
@@ -145,6 +208,7 @@ async def semantic_search_notes(
|
||||
task_kind: str | Sequence[str] | None = None,
|
||||
orphan_only: bool = False,
|
||||
scope: str = "own",
|
||||
demote_superseded: bool = True,
|
||||
) -> list[tuple[float, Note]]:
|
||||
"""Return up to *limit* (score, note) pairs most relevant to *query*.
|
||||
|
||||
@@ -176,6 +240,13 @@ async def semantic_search_notes(
|
||||
so a similarity floor of *threshold* is a distance ceiling of
|
||||
``1 - threshold`` and similarity is recovered as ``1 - distance``.
|
||||
|
||||
`demote_superseded` applies the supersession penalty (#278): a record a
|
||||
later note claims to have overtaken ranks below its equals. Callers asking
|
||||
"what is the current answer" want it; the near-duplicate gate does NOT, and
|
||||
passes False — a superseded record is still a duplicate of what you are
|
||||
about to write, and demoting it there would let the same note be recorded
|
||||
twice, the second time invisibly.
|
||||
|
||||
Returns an empty list if the embedder is unavailable or on any error.
|
||||
"""
|
||||
if not query or not query.strip():
|
||||
@@ -232,14 +303,38 @@ async def semantic_search_notes(
|
||||
)
|
||||
if exclude_ids:
|
||||
stmt = stmt.where(NoteEmbedding.note_id.notin_(exclude_ids))
|
||||
stmt = stmt.where(distance <= max_distance).order_by(distance.asc()).limit(limit)
|
||||
# OVER-FETCH when a re-rank follows, so the demotion can actually
|
||||
# move something. Demoting after a LIMIT k would be 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.
|
||||
#
|
||||
# Ordering stays on RAW distance so pgvector's HNSW index still
|
||||
# serves it (migration 0067). Ordering by `distance + penalty`
|
||||
# instead would be exact, and would turn an indexed top-k into a
|
||||
# scan-and-sort of every embedded note.
|
||||
#
|
||||
# The cost of that trade, stated plainly: a live record outside the
|
||||
# over-fetch window cannot be promoted into the results. With a
|
||||
# penalty far smaller than the window's score spread, that case
|
||||
# needs the true answer to be more than _SUPERSESSION_OVERFETCH
|
||||
# ranks down, which no observed query comes close to.
|
||||
fetch = limit * _SUPERSESSION_OVERFETCH if demote_superseded else limit
|
||||
stmt = (
|
||||
stmt.where(distance <= max_distance)
|
||||
.order_by(distance.asc())
|
||||
.limit(fetch)
|
||||
)
|
||||
rows = list((await session.execute(stmt)).all())
|
||||
except Exception:
|
||||
logger.warning("Failed to query note embeddings", exc_info=True)
|
||||
return []
|
||||
|
||||
# Recover similarity (1 - distance) and preserve the highest-first contract.
|
||||
return [(1.0 - float(dist), note) for note, dist in rows]
|
||||
scored = [(1.0 - float(dist), note) for note, dist in rows]
|
||||
if not demote_superseded:
|
||||
return scored[:limit]
|
||||
return await _apply_supersession_penalty(scored, limit)
|
||||
|
||||
|
||||
async def backfill_note_embeddings() -> None:
|
||||
|
||||
@@ -31,6 +31,7 @@ from scribe.services import snippets as snippets_svc
|
||||
from scribe.services.access import label_shared_items, owner_names_for
|
||||
from scribe.services.embeddings import semantic_search_notes
|
||||
from scribe.services.note_usage import record_surfaced
|
||||
from scribe.services.supersession import superseded_ids
|
||||
from scribe.services.retrieval_telemetry import record_retrieval
|
||||
from scribe.services.settings import get_setting
|
||||
|
||||
@@ -425,11 +426,19 @@ async def build_autoinject_hint(
|
||||
"`get_note(id)`, or `get_snippet` / `get_process` for those kinds "
|
||||
"(titles only; injected once per session):",
|
||||
]
|
||||
# A superseded record is DEMOTED, not removed (#278) — so one can still reach
|
||||
# this menu, and when it does the reader has to be told. An agent handed
|
||||
# stale material with nothing marking it acts on it with full confidence,
|
||||
# which is worse than never having surfaced it. One query for the whole menu.
|
||||
stale = await superseded_ids([int(n.id) for _s, n in kept])
|
||||
|
||||
note_ids: list[int] = []
|
||||
for score, note in kept:
|
||||
note_ids.append(int(note.id))
|
||||
title = (note.title or "(untitled)").replace("\n", " ").strip()
|
||||
line = f"> - #{note.id} [{_record_kind(note)}] \"{title}\" ({score:.2f})"
|
||||
if int(note.id) in stale:
|
||||
line += " — SUPERSEDED, a later record covers this; check that first"
|
||||
if note.user_id != user_id:
|
||||
who = owners.get(int(note.user_id)) or "another user"
|
||||
line += f" — shared by {who}, treat as a suggestion"
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
Reference in New Issue
Block a user