bvandeusenandClaude Opus 5.5
66e21a6c60
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / integration (push) Successful in 52s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m35s
CI & Build / Build & push image (push) Successful in 32s
The title was `subject — trigger` because the stored title WAS the embedded one, and the join is what makes these kinds rank on the situation they apply to (#2485). Every surface that shows a title then showed the trigger too -- menus, lists and search rows ran to kilobytes. - embeddings.document_title(title, note_type, data, body) joins the trigger from `data` (body fallback) at embed time. Idempotent: an un-migrated composed title comes out the same, never doubled. The embed path, the startup backfill and the dedup gate's semantic signal all use it, so the embedded text -- and every vector -- is unchanged. - Writers store the subject: snippet create/update (service, REST, MCP) and lesson_document. Both compose_title helpers are removed. - Readers: dedup takes `data`; the menus strip the embedded title from a passage; list rows project `when_to_use`, which SnippetListView reads. - 0108 rewrites existing rows on an exact `' — ' || <own trigger>` suffix with raw SQL, leaving updated_at alone so the backfill does not re-embed the corpus for identical vectors. Downgrade recomposes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
114 lines
4.4 KiB
Python
114 lines
4.4 KiB
Python
"""A lesson is a row the database actually accepts (milestone 385 step 2).
|
|
|
|
WHY THIS IS THE REAL GUARD, AND WHY IT IS HERE
|
|
|
|
The step asked for "a guard that the CHECK actually accepts `lesson` and
|
|
rejects a typo", citing #3128 — a `spike` kind that could not be written on an
|
|
instance predating its migration. That failure mode belongs to `task_kind`,
|
|
which IS gated (`notes_task_kind_check`, migrations 0056 and 0065).
|
|
`note_type` is not gated at all: migration 0036 added it as plain Text with a
|
|
server default and nothing has constrained it since.
|
|
|
|
So there is no whitelist to expand and no rejection to assert. Writing the
|
|
guard as "the constraint admits lesson" would have pinned a constraint that
|
|
does not exist; writing it as "no constraint exists" would pin today's schema
|
|
rather than the behaviour that matters, and would go red on a change that is
|
|
perfectly fine.
|
|
|
|
Asserting the WRITE covers both worlds. It passes today, it keeps passing if a
|
|
CHECK is added that admits `lesson`, and it goes red the day one is added that
|
|
does not — which is the only outcome anyone needs to be told about.
|
|
|
|
The second test is the cell #3163 asks to be left empty on purpose: a lesson
|
|
must not be a task. `is_task` is a read-only property over `status`, so this
|
|
cannot be asserted by setting a flag — it has to be observed on a real row.
|
|
"""
|
|
import pytest
|
|
import pytest_asyncio
|
|
from sqlalchemy import select
|
|
|
|
from scribe.models import async_session
|
|
from scribe.models.note import Note
|
|
from scribe.services import lessons as lessons_svc
|
|
from scribe.services import notes as notes_svc
|
|
from tests.helpers import ensure_user
|
|
|
|
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
|
|
|
|
OWNER_USERNAME = "lesson_kind_owner"
|
|
|
|
TRIGGER = "the operator pasted a stack trace and said it is still broken"
|
|
SUBJECT = "Change one thing, then look"
|
|
|
|
|
|
@pytest_asyncio.fixture
|
|
async def owner_id():
|
|
async with async_session() as s:
|
|
owner = await ensure_user(s, OWNER_USERNAME)
|
|
await s.commit()
|
|
uid = owner.id
|
|
# Cleaned at SETUP rather than teardown: create_note fires a detached
|
|
# embedding refresh that opens its own connection and writes the row,
|
|
# and a teardown delete would race it. A fresh loop has already
|
|
# cancelled whatever the previous test left in flight.
|
|
for note in (await s.execute(
|
|
select(Note).where(
|
|
Note.user_id == uid,
|
|
Note.note_type == lessons_svc.LESSON_NOTE_TYPE,
|
|
)
|
|
)).scalars().all():
|
|
await s.delete(note)
|
|
await s.commit()
|
|
return uid
|
|
|
|
|
|
async def test_a_lesson_is_a_row_the_database_accepts(owner_id):
|
|
"""The whole point. Goes red if `note_type` is ever gated without this
|
|
value, and stays correct if it is gated with it."""
|
|
lesson = await notes_svc.create_note(
|
|
owner_id,
|
|
title=SUBJECT,
|
|
body=f"**When to apply:** {TRIGGER}\n\nOne change at a time.",
|
|
note_type=lessons_svc.LESSON_NOTE_TYPE,
|
|
data={lessons_svc.TRIGGER_KEY: TRIGGER},
|
|
)
|
|
|
|
async with async_session() as s:
|
|
stored = (await s.execute(
|
|
select(Note).where(Note.id == lesson.id)
|
|
)).scalars().one()
|
|
|
|
assert stored.note_type == "lesson"
|
|
# The trigger survives the round trip on both halves — the indexed mirror
|
|
# and the readable body — because the vector is built from the text and
|
|
# the queries are built from the mirror.
|
|
assert lessons_svc.lesson_trigger(stored) == TRIGGER
|
|
# The subject alone (milestone 427): the trigger is in `data` and the body.
|
|
assert stored.title == SUBJECT
|
|
assert "**When to apply:**" in (stored.body or "")
|
|
|
|
|
|
async def test_a_lesson_is_not_a_task(owner_id):
|
|
"""#3163's cell left empty on purpose.
|
|
|
|
`is_task` is `status is not None` and is read-only, so this is only
|
|
observable on a stored row. A lesson that arrived with a status would join
|
|
the open-work listings — the one way an unfilled field changes what the
|
|
record IS rather than what it says.
|
|
"""
|
|
lesson = await notes_svc.create_note(
|
|
owner_id,
|
|
title=SUBJECT,
|
|
body="One change at a time.",
|
|
note_type=lessons_svc.LESSON_NOTE_TYPE,
|
|
)
|
|
|
|
async with async_session() as s:
|
|
stored = (await s.execute(
|
|
select(Note).where(Note.id == lesson.id)
|
|
)).scalars().one()
|
|
|
|
assert stored.status is None
|
|
assert stored.is_task is False
|
|
assert stored.milestone_id is None
|