CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 50s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / Python tests (push) Failing after 1m5s
CI & Build / Build & push image (push) Skipped
Milestone 385 step 2, implementing decision #4157 from the step-1 spike. The kind: `note_type='lesson'`, a note findable by WHEN IT APPLIES rather than by what it is about. The trigger lives in `notes.data.when_to_apply`, mirrored into the title and the head of the body — the shape snippets already use, and the reason nothing re-embeds: chunk_document is untouched, so CHUNKER_VERSION does not move. NO MIGRATION, and the step assumed there would be one. `note_type` carries no CHECK — only `task_kind` does (0056, 0065). Migration 0036 added it as plain Text with a server default and nothing has gated it since, so rule 36 has no whitelist to expand and #3128's failure mode (a value the database refuses) cannot arise for this column. The vocabulary that actually decides what a reader can reach is services.knowledge._FACETS, which since #3161 is one table feeding the door's validation, the counts and both dialects of the type filter — so the kind lands there in a single edit. ONE JOIN, not a fourth copy. `{subject} — {trigger}` had three implementations: rule_document, snippets.compose_title, and this step needed another. #3207 records what that costs, so the join moves to embeddings.trigger_title beside embedding_text and all three delegate. Behaviour is unchanged for rules and snippets; the guard calls each through its own public name, so a re-implementation fails it. The #3163 bill is stated in the service docstring rather than left to be inferred: versions, supersession, trash, the share ACL, tags, project and System tagging, chunked embeddings and the duplicate gate are all inherited; status/task_kind/milestone_id and recurrence are not, and verify_with/expires_when are available but outside the kind's contract. The status cell is the one that matters — `is_task` IS `status is not None`, so a lesson that acquired one would become a task. The integration guard asserts the WRITE rather than the constraint: it holds whether or not note_type is ever gated, and goes red only if it is gated without this value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
113 lines
4.4 KiB
Python
113 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=lessons_svc.compose_title(SUBJECT, TRIGGER),
|
|
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
|
|
assert stored.title == f"{SUBJECT} — {TRIGGER}"
|
|
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=lessons_svc.compose_title(SUBJECT, TRIGGER),
|
|
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
|