feat(lessons): the document shape is the stored record, and it travels (#3730)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m31s
CI & Build / Build & push image (push) Successful in 25s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m31s
CI & Build / Build & push image (push) Successful in 25s
Milestone 385 step 3 — the step where the kind either works or is cosmetic.
THE DOCUMENT, and why there is no `lesson_document()` beside `rule_document()`
in embeddings. The step expected one. The difference is where the sharp shape
LIVES. A rule keeps its trigger in a column and its title is a plain name, so
`{title} — {trigger}` has to be synthesised at embed time and exists nowhere
else. A snippet — the only sharp record in the corpus by #2485's measurement,
0.153 top-to-second against 0.010–0.023 — gets there the other way: its STORED
title is already the join and its stored body already opens with the trigger,
so the ordinary `title\nbody` join IS the sharp document. Step 1 chose the
snippet route and step 2 built it, so `lessons.lesson_document` composes what
is STORED and the generic chunker does the rest.
The consequence the step asked about: `chunk_document` is untouched, so
CHUNKER_VERSION does not move and NOTHING re-embeds. The step's "Re-embed"
section describes a change this design does not make.
THE NARRATIVE stays in the body, departing from the step's instruction to keep
it out. `rule_document` excludes `why` because long dated narrative made
sixteen dev-logs land on the centroid of "development" — but that finding
predates chunking (#280). A body over budget is now split, and every chunk is
prefixed with the title, which for a lesson carries the trigger. The story
occupies its own vectors instead of averaging itself into the trigger's, and
each of those is still anchored to when the lesson applies. A guard asserts
exactly that. Holding the story out would cost the reader the only part that
explains the insight, to buy a sharpness the chunker already provides.
GLOBAL IN THE SEARCH is the real new code: `GLOBAL_NOTE_TYPES` and
`include_global_kinds` on `semantic_search_notes`, widening the PROJECT filter
alone. Off by default, because two callers depend on that filter holding — the
near-duplicate gate compares a record only against its own project on purpose,
and a globally visible kind there would let a lesson block an unrelated note's
create on a project its author never touched. It composes with `note_type`
rather than overriding it, so narrowing to snippets does not quietly acquire
lessons, and it changes nothing about the ACL: `notes_visibility_clause` still
gates every row.
Wired into the explicit MCP search only — the operator asked, and there is no
budget to spend. The unasked-for injection arms are step 5's subject (#3732)
and the legibility of a lesson appearing on a foreign project is step 7's
(#3734), so neither is turned on here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -0,0 +1,143 @@
|
||||
"""A lesson is reachable from a project it was not written on (step 3).
|
||||
|
||||
WHY THIS IS AN INTEGRATION TEST
|
||||
|
||||
The carve-out is one `OR` inside the query's project filter, and what has to be
|
||||
proved is which ROWS come back — a mock session returns whatever it was told to
|
||||
and would pass with the predicate inverted. Every note here embeds identically
|
||||
to the query, so the only thing that can separate them is the scoping: a leak
|
||||
and a correct result are otherwise indistinguishable.
|
||||
|
||||
The embedder is stubbed, as in the other pgvector tests, so this depends on
|
||||
Postgres and the query rather than on a downloaded model. No similarity number
|
||||
is asserted — only membership.
|
||||
"""
|
||||
import uuid
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.embedding import EMBEDDING_DIM, NoteEmbedding
|
||||
from scribe.models.note import Note
|
||||
from scribe.models.project import Project
|
||||
from scribe.services import lessons as lessons_svc
|
||||
from scribe.services.embeddings import CHUNKER_VERSION, semantic_search_notes
|
||||
from tests.helpers import ensure_user
|
||||
|
||||
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
|
||||
|
||||
QUERY_VEC = [1.0] + [0.0] * (EMBEDDING_DIM - 1)
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def corpus():
|
||||
"""A lesson and a plain note on project A, plus a lesson on project B.
|
||||
|
||||
Fresh users per run: every row matches the query equally, so a record left
|
||||
behind by another test would read exactly like a scoping leak.
|
||||
"""
|
||||
tag = uuid.uuid4().hex[:8]
|
||||
async with async_session() as s:
|
||||
owner = await ensure_user(s, f"lesson_reach_owner_{tag}")
|
||||
await s.flush()
|
||||
a = Project(user_id=owner.id, title="Where it was learned")
|
||||
b = Project(user_id=owner.id, title="Somewhere else entirely")
|
||||
s.add_all([a, b])
|
||||
await s.flush()
|
||||
|
||||
rows = {
|
||||
"lesson_on_a": Note(
|
||||
user_id=owner.id, project_id=a.id,
|
||||
note_type=lessons_svc.LESSON_NOTE_TYPE,
|
||||
title="Suspect the guard — a test fails on correct code",
|
||||
body="**When to apply:** a test fails on correct code",
|
||||
),
|
||||
"note_on_a": Note(
|
||||
user_id=owner.id, project_id=a.id, note_type="note",
|
||||
title="An ordinary note", body="ordinary body",
|
||||
),
|
||||
"lesson_on_b": Note(
|
||||
user_id=owner.id, project_id=b.id,
|
||||
note_type=lessons_svc.LESSON_NOTE_TYPE,
|
||||
title="A lesson that lives on B", body="**When to apply:** on B",
|
||||
),
|
||||
}
|
||||
s.add_all(rows.values())
|
||||
await s.flush()
|
||||
for note in rows.values():
|
||||
s.add(NoteEmbedding(
|
||||
note_id=note.id, chunk_index=0, user_id=owner.id,
|
||||
embedding=QUERY_VEC, chunk_text=note.title,
|
||||
chunker_version=CHUNKER_VERSION,
|
||||
))
|
||||
ids = {k: n.id for k, n in rows.items()}
|
||||
ids["owner"], ids["a"], ids["b"] = owner.id, a.id, b.id
|
||||
await s.commit()
|
||||
return ids
|
||||
|
||||
|
||||
async def _search(uid, **kw):
|
||||
with patch(
|
||||
"scribe.services.embeddings.get_embedding", AsyncMock(return_value=QUERY_VEC)
|
||||
):
|
||||
hits = await semantic_search_notes(uid, "when does this apply", limit=20, **kw)
|
||||
return {note.id for _score, note in hits}
|
||||
|
||||
|
||||
async def test_a_lesson_is_found_from_another_project(corpus):
|
||||
"""THE acceptance this step exists for. Searching project B reaches the
|
||||
lesson written on project A — the case the kind was created for, because a
|
||||
transferable insight is most useful on the project that has not learned it
|
||||
yet."""
|
||||
found = await _search(
|
||||
corpus["owner"], project_id=corpus["b"], include_global_kinds=True,
|
||||
)
|
||||
|
||||
assert corpus["lesson_on_a"] in found
|
||||
assert corpus["lesson_on_b"] in found
|
||||
|
||||
|
||||
async def test_an_ordinary_note_stays_where_it_was_written(corpus):
|
||||
"""The other half, and the one that would make this change a bug. Project
|
||||
scoping is deliberate for every other kind; the carve-out admits ONE kind
|
||||
rather than weakening the filter."""
|
||||
found = await _search(
|
||||
corpus["owner"], project_id=corpus["b"], include_global_kinds=True,
|
||||
)
|
||||
|
||||
assert corpus["note_on_a"] not in found
|
||||
|
||||
|
||||
async def test_the_carve_out_is_off_unless_asked_for(corpus):
|
||||
"""Default off, because the near-duplicate gate and ordinary recall both
|
||||
depend on the project filter holding. A globally visible kind arriving
|
||||
there would let a lesson block an unrelated note's create on a project its
|
||||
author never touched."""
|
||||
found = await _search(corpus["owner"], project_id=corpus["b"])
|
||||
|
||||
assert found == {corpus["lesson_on_b"]}
|
||||
|
||||
|
||||
async def test_the_home_project_is_unchanged(corpus):
|
||||
"""Searching the project a lesson was written on returns it either way —
|
||||
the carve-out adds reach, it does not move anything."""
|
||||
for flag in (False, True):
|
||||
found = await _search(
|
||||
corpus["owner"], project_id=corpus["a"], include_global_kinds=flag,
|
||||
)
|
||||
assert corpus["lesson_on_a"] in found
|
||||
assert corpus["note_on_a"] in found
|
||||
|
||||
|
||||
async def test_a_kind_filter_still_means_what_it_says(corpus):
|
||||
"""The carve-out widens the PROJECT filter only. A caller narrowing to
|
||||
snippets asked for snippets, and quietly handing it lessons would make
|
||||
`note_type` mean something different depending on a flag it did not set."""
|
||||
found = await _search(
|
||||
corpus["owner"], project_id=corpus["b"],
|
||||
include_global_kinds=True, note_type="snippet",
|
||||
)
|
||||
|
||||
assert found == set()
|
||||
Reference in New Issue
Block a user