diff --git a/src/scribe/services/lesson_rules.py b/src/scribe/services/lesson_rules.py index eac1c2d3..95114ffe 100644 --- a/src/scribe/services/lesson_rules.py +++ b/src/scribe/services/lesson_rules.py @@ -37,6 +37,7 @@ from __future__ import annotations import hashlib import logging import re +import statistics from datetime import datetime, timedelta, timezone from sqlalchemy import and_, delete, exists, func, not_, or_, select @@ -735,17 +736,47 @@ async def confirmed_rules_in_scope( # answered, and never by a sweep or a timer (#4183): the reader is in the # situation then, and a nudge arriving anywhere else is one nobody acts on. # +# WHAT "RESEMBLE" MEANS (#5193). Not a fixed similarity. Lessons are written +# in one register — a sentence of advice and the moment it applies — so an +# embedder scores any two of them well above two unrelated texts, and a +# constant bar sits inside that band: every no-rule lesson then "resembles" +# most of the others, and the group grows with the pool rather than with any +# situation recurring. The bar is instead relative to each lesson's own +# background: how far a neighbour stands above the typical score that lesson +# gets against every lesson in its register, measured in that register's own +# spread (median and MAD, so the near neighbours being judged do not move the +# yardstick). An install with a different embedder, or lessons in a different +# style, gets a different band and the same bar. +# +# And resemblance must be MUTUAL, among every member, not just to the lesson +# being answered. A lesson broad enough to stand near many others is a hub, +# and hub-and-spoke is how three unrelated lessons used to make a "group" +# through one general one. A clique is the shape of one situation met again. +# # DEFAULTS, stated as defaults (rules 32, 115). Three lessons — the new one # and two it resembles — is the smallest group that is a pattern rather than -# a pair. The similarity bar sits above the notes menu's ("worth showing") -# and below the duplicate gate's ("the same record"): these lessons should be -# about one situation without being one lesson written twice, which the -# duplicate gate already catches. +# a pair. One spread above the background is a modest bar for one direction +# alone; the strictness comes from requiring it of every pair in both +# directions, which unrelated lessons rarely all clear. Raise it if groups +# name lessons a reader would not call one situation; lower it if lessons a +# reader would group never meet. CONVERGENCE_LESSONS = 3 -CONVERGENCE_THRESHOLD = 0.65 -# Candidates fetched before keeping the no-rule ones; most lessons near a -# situation may well have rules. -_CONVERGENCE_FETCH = 20 +CONVERGENCE_STANDOUT = 1.0 +# Below this many other lessons a median and its spread describe too little +# to say what stands out, so nothing is named. +CONVERGENCE_MIN_BACKGROUND = 8 +# How many of a lesson's register the background is read from. A register +# larger than this is measured on its nearest part, which reads the +# background HIGH — the bar errs strict, toward naming nothing, never toward +# a group that is only the pool's size. +_CONVERGENCE_BACKGROUND = 200 +# Neighbours checked for mutual resemblance, nearest first. Each costs one +# more search at the write, and a group large enough to need more is named +# just as well by its nearest members. +_CONVERGENCE_CANDIDATES = 8 +# The consistency constant that makes a MAD estimate a standard deviation's +# scale on normal data, so CONVERGENCE_STANDOUT reads as "spreads". +_MAD_SCALE = 1.4826 def convergence_group(members: list[dict]) -> dict | None: @@ -812,6 +843,38 @@ async def convergence_for(user_id: int, lesson_id: int) -> dict | None: return None +def standout(scores: dict[int, float]) -> dict[int, float]: + """How far each lesson stands above this one's background, in spreads. + + `scores` is one lesson's similarity to every other lesson in its register, + itself left out. Pure, so the bar is testable. Empty when there is too + little background to measure, or none of it varies. + """ + if len(scores) < CONVERGENCE_MIN_BACKGROUND: + return {} + middle = statistics.median(scores.values()) + spread = statistics.median(abs(v - middle) for v in scores.values()) * _MAD_SCALE + if spread <= 0: + return {} + return {i: (v - middle) / spread for i, v in scores.items()} + + +def converging(lesson_id: int, standouts: dict[int, dict[int, float]], + candidates: list[int]) -> list[int]: + """The lesson, then each candidate that stands out to EVERY member so far + and every member to it. Candidates are taken nearest first, so the clique + grows around the closest situation rather than the first id. Pure.""" + def near(a: int, b: int) -> bool: + return (standouts.get(a, {}).get(b, float("-inf")) >= CONVERGENCE_STANDOUT + and standouts.get(b, {}).get(a, float("-inf")) >= CONVERGENCE_STANDOUT) + + group = [lesson_id] + for c in candidates: + if c not in group and all(near(c, m) for m in group): + group.append(c) + return group + + async def _convergence_for(user_id: int, lesson_id: int) -> dict | None: from scribe.services import lessons as lessons_svc from scribe.services.embeddings import semantic_search_notes, trigger_title @@ -819,14 +882,34 @@ async def _convergence_for(user_id: int, lesson_id: int) -> dict | None: lesson = await lessons_svc.get_lesson(user_id, lesson_id) if lesson is None: return None - data = lesson.data if isinstance(lesson.data, dict) else {} - query = trigger_title(data.get("what") or lesson.title, lessons_svc.lesson_trigger(lesson)) - found = await semantic_search_notes( - user_id, query, exclude_ids={int(lesson.id)}, limit=_CONVERGENCE_FETCH, - threshold=CONVERGENCE_THRESHOLD, note_type=(lessons_svc.LESSON_NOTE_TYPE,), - include_global_kinds=True, scope="browse", - ) - answered = await _no_rule_ids([int(n.id) for _s, n in found]) + + async def background_of(note) -> list[tuple[float, Note]]: + """`note`'s similarity to every other lesson it can reach — the + background it is measured against. No threshold: the bar is relative, + and needs the scores a fixed bar would have turned away. No + supersession penalty either: it would shift some scores and not + others, and the background is a measure of resemblance alone.""" + data = note.data if isinstance(note.data, dict) else {} + query = trigger_title(data.get("what") or note.title, lessons_svc.lesson_trigger(note)) + return await semantic_search_notes( + user_id, query, exclude_ids={int(note.id)}, limit=_CONVERGENCE_BACKGROUND, + threshold=-1.0, note_type=(lessons_svc.LESSON_NOTE_TYPE,), + include_global_kinds=True, scope="browse", demote_superseded=False, + ) + + lid = int(lesson.id) + found = await background_of(lesson) + notes = {int(n.id): n for _s, n in found} + standouts = {lid: standout({int(n.id): s for s, n in found})} + standing = [i for i, z in standouts[lid].items() if z >= CONVERGENCE_STANDOUT] + answered = await _no_rule_ids(standing) + candidates = sorted(answered, key=lambda i: (-standouts[lid][i], i))[:_CONVERGENCE_CANDIDATES] + # Too few stand out from here for any clique to reach the bar — skip the + # searches the mutual check would cost. + if len(candidates) < CONVERGENCE_LESSONS - 1: + return None + for c in candidates: + standouts[c] = standout({int(n.id): s for s, n in await background_of(notes[c])}) def member(note) -> dict: return { @@ -834,6 +917,5 @@ async def _convergence_for(user_id: int, lesson_id: int) -> dict | None: "sources": lessons_svc.lesson_sources(note), "project_id": note.project_id, } - return convergence_group( - [member(lesson)] + [member(n) for _s, n in found if int(n.id) in answered] - ) + group = converging(lid, standouts, candidates) + return convergence_group([member(lesson)] + [member(notes[i]) for i in group[1:]]) diff --git a/tests/test_lesson_convergence.py b/tests/test_lesson_convergence.py index 5d0f4441..a718ab3c 100644 --- a/tests/test_lesson_convergence.py +++ b/tests/test_lesson_convergence.py @@ -53,30 +53,104 @@ def test_one_incident_written_up_three_times_is_not_a_recurring_situation(): assert links_svc.convergence_group(same) is None -def _note(lid, *, title="t", project=None, sources=()): - data = {"what": title, "when_to_apply": "a CI run overran"} +# ── The bar is relative to each lesson's own register (#5193) ────────────── + + +def test_too_little_background_says_nothing_stands_out(): + few = {i: 0.6 for i in range(links_svc.CONVERGENCE_MIN_BACKGROUND - 1)} + assert links_svc.standout(few) == {} + + +def test_a_background_that_never_varies_says_nothing_stands_out(): + assert links_svc.standout({i: 0.7 for i in range(20)}) == {} + + +def test_standing_out_is_measured_in_the_registers_own_spread(): + """The same neighbour score stands out in a tight register and not in a + loose one — the bar moves with the corpus, not with a constant.""" + tight = {i: 0.70 + 0.01 * (i % 5) for i in range(20)} | {99: 0.80} + loose = {i: 0.60 + 0.06 * (i % 5) for i in range(20)} | {99: 0.80} + assert links_svc.standout(tight)[99] >= links_svc.CONVERGENCE_STANDOUT + assert links_svc.standout(loose)[99] < links_svc.CONVERGENCE_STANDOUT + + +def test_a_hub_near_two_lessons_that_are_not_near_each_other_is_no_clique(): + up = links_svc.CONVERGENCE_STANDOUT + 1 + standouts = {1: {2: up, 3: up}, 2: {1: up, 3: 0.0}, 3: {1: up, 2: 0.0}} + assert links_svc.converging(1, standouts, [2, 3]) == [1, 2] + + +def test_resemblance_must_run_both_ways(): + up = links_svc.CONVERGENCE_STANDOUT + 1 + standouts = {1: {2: up, 3: up}, 2: {1: 0.0, 3: up}, 3: {1: up, 2: up}} + assert links_svc.converging(1, standouts, [2, 3]) == [1, 3] + + +def _note(lid, *, title=None, project=None, sources=()): + data = {"what": title or f"lesson {lid}", "when_to_apply": "a CI run overran"} if sources: data["taught_by"] = list(sources) return SimpleNamespace( - id=lid, title=title, note_type="lesson", body="", data=data, + id=lid, title=title or f"lesson {lid}", note_type="lesson", body="", data=data, project_id=project, arose_from_id=None, tags=[], created_at=None, updated_at=None, ) +# A register: lessons 1-3 resemble each other well above a background of +# filler lessons 10-29, which all sit in one flat band — the shape every +# lesson-to-lesson score has, because lessons share one register. +_TRIO = {1, 2, 3} +_FILLER = range(10, 30) +_NOTES = {i: _note(i, sources=[100 + i]) for i in (*_TRIO, *_FILLER)} + + +def _sim(a, b): + if a in _TRIO and b in _TRIO: + return 0.82 + return 0.66 + 0.01 * ((a + b) % 5) + + +def _search_over(sim=_sim): + async def search(user_id, query, *, exclude_ids, **_kw): + (me,) = exclude_ids + return sorted(((sim(me, i), n) for i, n in _NOTES.items() if i != me), + key=lambda pair: -pair[0]) + return search + + +def _run(no_rule, sim=_sim): + return ( + patch.object(lessons_svc, "get_lesson", AsyncMock(return_value=_NOTES[1])), + patch("scribe.services.embeddings.semantic_search_notes", _search_over(sim)), + patch.object(links_svc, "_no_rule_ids", + AsyncMock(side_effect=lambda ids: {i for i in ids if i in no_rule})), + ) + + +@pytest.mark.asyncio +async def test_lessons_that_stand_out_together_are_named(): + a, b, c = _run(no_rule=set(_NOTES)) + with a, b, c: + group = await _REAL(7, 1) + assert [m["id"] for m in group["lessons"]] == [1, 2, 3] + + +@pytest.mark.asyncio +async def test_a_large_no_rule_pool_in_one_flat_band_names_nothing(): + """The #5193 failure: every lesson answered "no rule", every score above + a fixed bar, and a group the size of the pool. Nothing stands out of a + flat band, so nothing is named however large the pool grows.""" + a, b, c = _run(no_rule=set(_NOTES), sim=lambda x, y: 0.66 + 0.01 * ((x + y) % 5)) + with a, b, c: + assert await _REAL(7, 1) is None + + @pytest.mark.asyncio async def test_only_lessons_answered_no_rule_join_the_group(): - found = [(0.8, _note(2, sources=[11])), (0.7, _note(3, sources=[12])), - (0.7, _note(4, sources=[13]))] - with patch.object(lessons_svc, "get_lesson", AsyncMock(return_value=_note(1, sources=[10]))), \ - patch("scribe.services.embeddings.semantic_search_notes", AsyncMock(return_value=found)), \ - patch.object(links_svc, "_no_rule_ids", AsyncMock(return_value={2})): - assert await _REAL(7, 1) is None # only #2 answered: a pair - with patch.object(lessons_svc, "get_lesson", AsyncMock(return_value=_note(1, sources=[10]))), \ - patch("scribe.services.embeddings.semantic_search_notes", AsyncMock(return_value=found)), \ - patch.object(links_svc, "_no_rule_ids", AsyncMock(return_value={2, 4})): - group = await _REAL(7, 1) - assert [m["id"] for m in group["lessons"]] == [1, 2, 4] + a, b, c = _run(no_rule={1, 2}) # #3 has a rule: a pair is no group + with a, b, c: + assert await _REAL(7, 1) is None @pytest.mark.asyncio