feat(rules): rules become findable by meaning (#3030, milestone 307 step 4)
CI & Build / Python lint (push) Failing after 9s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 44s
CI & Build / integration (push) Successful in 45s
CI & Build / Python tests (push) Successful in 1m26s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Failing after 9s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 44s
CI & Build / integration (push) Successful in 45s
CI & Build / Python tests (push) Successful in 1m26s
CI & Build / Build & push image (push) Skipped
Rules were the only major record type with no vector, so `search` could never return one and a rule could arrive only by being preloaded. That single fact is what made every rule compete for one always-on budget. THE DECISION THE TASK ASKED FOR, made explicitly: a sibling rule_embeddings table, not a polymorphic embedding row. The ROW could have been generalised; the SEARCH could not. semantic_search_notes is Note-specific scoping end to end — the visibility clause, the supersession penalty, note_type/task_kind/system filters — and a rule shares none of it, scoping instead by rulebook ownership or project. Generalising the row while still needing two searches is the worst of both: a key with referential integrity to neither table, on the path every session start runs, to share four columns. What is genuinely common is BEHAVIOUR — get_embedding, chunk_document, embedding_text, CHUNKER_VERSION — and those are reused as-is. Sharing them is the DRY win; sharing the table would have been the DRY costume. The document shape is measured, not chosen (note 2485). That pass found the snippet was the only discriminative record in the corpus — a 0.153 top-to-second gap against 0.010-0.023 — and that the cause was its SHAPE: purpose stated twice in a short single-topic document. rule_document reproduces it: the trigger in the title AND as the body's first line. And it excludes `why`, which matters more than any of it. `why` is dated incident narrative — rule 46's runs to 4,300 characters — and long multi-topic prose is exactly what made sixteen dev-logs mutually indistinguishable. Adding it would not give the vector more to work with; it would give every rule the SAME thing to work with. rule_document takes no `why` parameter at all, so a well-meaning caller cannot pass one. A rule with no trigger degrades to title + statement — findable, less sharp. That is an argument for backfilling triggers (step 6), not for padding the document with whatever text is nearby. search(content_type="rule") returns the rule WITH its why and how_to_apply: they are its operational half, the session payload never carries them, and a caller who went looking should not have to re-fetch. Writes re-index fire-and-forget like notes; startup backfills in its own try block so neither backfill can skip the other. rule_embeddings is derived, so it joins note_embeddings in the backup's explicitly-NOT-included list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,58 @@
|
||||
"""rule_embeddings — rules become findable by meaning (milestone 307 step 4,
|
||||
decision note 3026)
|
||||
|
||||
Revision ID: 0089
|
||||
Revises: 0088
|
||||
Create Date: 2026-08-26
|
||||
|
||||
Rules were the only major record type with no vector, so `search` could never
|
||||
return one and a rule could only ever arrive by being preloaded. That single
|
||||
fact is what made every rule compete for the same always-on budget.
|
||||
|
||||
A sibling table rather than a generalisation of note_embeddings: the row could
|
||||
have been made polymorphic, but the SEARCH could not — semantic_search_notes is
|
||||
Note-specific scoping end to end, and a rule shares none of it. See the model
|
||||
docstring for the full reasoning.
|
||||
|
||||
The vectors are DERIVED data. Nothing is backfilled here: the startup backfill
|
||||
regenerates them, which is also how a chunker-version bump is handled.
|
||||
"""
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
revision = "0089"
|
||||
down_revision = "0088"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
# Matches note_embeddings — bge-small-en-v1.5, 384-dim unit-normalized.
|
||||
_EMBEDDING_DIM = 384
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.create_table(
|
||||
"rule_embeddings",
|
||||
sa.Column("rule_id", sa.BigInteger(), sa.ForeignKey("rules.id", ondelete="CASCADE"), primary_key=True),
|
||||
sa.Column("chunk_index", sa.Integer(), primary_key=True),
|
||||
sa.Column("chunk_text", sa.Text(), nullable=False),
|
||||
sa.Column("chunker_version", sa.Integer(), nullable=False),
|
||||
sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False, server_default=sa.text("now()")),
|
||||
)
|
||||
# The vector column is added by raw DDL for the same reason 0067 did it:
|
||||
# the type comes from the pgvector extension, not from SQLAlchemy's
|
||||
# type system.
|
||||
op.execute(f"ALTER TABLE rule_embeddings ADD COLUMN embedding vector({_EMBEDDING_DIM}) NOT NULL")
|
||||
# HNSW for cosine distance — matches Vector.cosine_distance (`<=>`), so the
|
||||
# search is an indexed ORDER BY ... LIMIT k rather than a full scan.
|
||||
op.execute(
|
||||
"""
|
||||
CREATE INDEX ix_rule_embeddings_embedding_hnsw
|
||||
ON rule_embeddings
|
||||
USING hnsw (embedding vector_cosine_ops)
|
||||
"""
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.execute("DROP INDEX IF EXISTS ix_rule_embeddings_embedding_hnsw")
|
||||
op.drop_table("rule_embeddings")
|
||||
+7
-1
@@ -161,7 +161,7 @@ def create_app() -> Quart:
|
||||
import asyncio
|
||||
|
||||
from scribe.services.auth import start_auth_token_retention_loop
|
||||
from scribe.services.embeddings import backfill_note_embeddings
|
||||
from scribe.services.embeddings import backfill_note_embeddings, backfill_rule_embeddings
|
||||
from scribe.services.logging import start_log_retention_loop
|
||||
from scribe.services.notifications import start_notification_loop
|
||||
|
||||
@@ -176,6 +176,12 @@ def create_app() -> Quart:
|
||||
await backfill_note_embeddings()
|
||||
except Exception:
|
||||
logger.warning("Embedding backfill failed", exc_info=True)
|
||||
# Rules got vectors in milestone 307; every rule written before it
|
||||
# has none, so this is the pass that makes them findable at all.
|
||||
try:
|
||||
await backfill_rule_embeddings()
|
||||
except Exception:
|
||||
logger.warning("Rule embedding backfill failed", exc_info=True)
|
||||
# Snippets written before migration 0070 have no `notes.data` mirror,
|
||||
# and the location reverse lookup queries that column — an unfilled
|
||||
# row would read as "no snippet here" rather than as a gap. Separate
|
||||
|
||||
@@ -11,10 +11,44 @@ import time
|
||||
|
||||
from scribe.mcp._context import current_user_id
|
||||
from scribe.services.access import owner_names_for
|
||||
from scribe.services.embeddings import DEFAULT_SIMILARITY_THRESHOLD, semantic_search_notes
|
||||
from scribe.services.embeddings import (
|
||||
DEFAULT_SIMILARITY_THRESHOLD, semantic_search_notes, semantic_search_rules,
|
||||
)
|
||||
from scribe.services.retrieval_telemetry import record_retrieval, retrieval_summary
|
||||
|
||||
|
||||
async def _search_rules(uid: int, q: str, limit: int) -> dict:
|
||||
"""Rules by meaning — a separate result shape because a rule IS different.
|
||||
|
||||
A rule hit carries `why` and `how_to_apply`: they are the operational half
|
||||
of a rule and the session-start payload never includes them, so a caller
|
||||
who went looking should get the whole thing rather than a summary they then
|
||||
have to re-fetch.
|
||||
|
||||
Rules are not project-scoped the way notes are (a family rule belongs to no
|
||||
project), so `project_id` and `system_id` do not apply here.
|
||||
"""
|
||||
raw = await semantic_search_rules(uid, q, limit=limit)
|
||||
return {
|
||||
"results": [
|
||||
{
|
||||
"id": rule.id,
|
||||
"title": rule.title,
|
||||
"statement": rule.statement,
|
||||
"when_to_apply": rule.when_to_apply or "",
|
||||
"tier": rule.tier,
|
||||
"why": rule.why or "",
|
||||
"how_to_apply": rule.how_to_apply or "",
|
||||
"topic_id": rule.topic_id,
|
||||
"project_id": rule.project_id,
|
||||
"similarity": float(score),
|
||||
}
|
||||
for score, rule in raw
|
||||
],
|
||||
"total": len(raw),
|
||||
}
|
||||
|
||||
|
||||
async def search(
|
||||
q: str,
|
||||
content_type: str = "all",
|
||||
@@ -33,7 +67,13 @@ async def search(
|
||||
|
||||
Args:
|
||||
q: search query string.
|
||||
content_type: 'all' (default), 'note' (notes only), or 'task' (tasks only).
|
||||
content_type: 'all' (default), 'note' (notes only), 'task' (tasks
|
||||
only), or 'rule' (RULES only — the operator's standing
|
||||
instructions, searchable by meaning since milestone 307).
|
||||
Reach for 'rule' when you want to know whether a standing
|
||||
instruction covers something: "is there a rule about release
|
||||
tagging?". A hit carries the rule's `why` and `how_to_apply`,
|
||||
which the session-start payload does not.
|
||||
limit: maximum number of results (1-50).
|
||||
project_id: Scope results to one project. PASS THE ACTIVE PROJECT'S ID
|
||||
whenever a project is in scope (the one you entered with
|
||||
@@ -56,6 +96,8 @@ async def search(
|
||||
"""
|
||||
uid = current_user_id()
|
||||
limit = max(1, min(limit, 50))
|
||||
if content_type == "rule":
|
||||
return await _search_rules(uid, q, limit)
|
||||
is_task = {"note": False, "task": True}.get(content_type) # None => any
|
||||
t0 = time.perf_counter()
|
||||
raw = await semantic_search_notes(
|
||||
|
||||
@@ -25,7 +25,7 @@ from scribe.models.user import User # noqa: E402, F401
|
||||
from scribe.models.app_log import AppLog # noqa: E402, F401
|
||||
from scribe.models.password_reset import PasswordResetToken # noqa: E402, F401
|
||||
from scribe.models.invitation import InvitationToken # noqa: E402, F401
|
||||
from scribe.models.embedding import NoteEmbedding # noqa: E402, F401
|
||||
from scribe.models.embedding import NoteEmbedding, RuleEmbedding # noqa: E402, F401
|
||||
from scribe.models.retrieval_log import RetrievalLog # noqa: E402, F401
|
||||
from scribe.models.note_usage import NoteUsageEvent # noqa: E402, F401
|
||||
from scribe.models.project import Project # noqa: E402, F401
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from pgvector.sqlalchemy import Vector
|
||||
from sqlalchemy import DateTime, ForeignKey, Integer, Text
|
||||
from sqlalchemy import BigInteger, DateTime, ForeignKey, Integer, Text
|
||||
from sqlalchemy.orm import Mapped, mapped_column
|
||||
|
||||
from scribe.models import Base
|
||||
@@ -45,3 +45,49 @@ class NoteEmbedding(Base):
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
|
||||
class RuleEmbedding(Base):
|
||||
"""One embedding vector per CHUNK of a rule (milestone 307, note 3026).
|
||||
|
||||
A SIBLING of NoteEmbedding rather than a generalisation of it, decided
|
||||
deliberately:
|
||||
|
||||
- The embedding ROW could have been made polymorphic. The SEARCH could not.
|
||||
`semantic_search_notes` is a long function of Note-specific scoping —
|
||||
the visibility clause, the supersession penalty, note_type/task_kind and
|
||||
system filters — and a rule shares none of it. Rules scope by rulebook
|
||||
ownership and project applicability instead.
|
||||
- Generalising the row while still needing two searches is the worst of
|
||||
both: a polymorphic key with referential integrity to neither table, on
|
||||
the path every session start runs, to share four columns.
|
||||
- What is genuinely common is BEHAVIOUR, not storage — get_embedding,
|
||||
chunk_document, embedding_text and CHUNKER_VERSION are already free
|
||||
functions and are reused as-is. Sharing those is the DRY win; sharing
|
||||
the table would have been the DRY costume.
|
||||
|
||||
No `user_id`: NoteEmbedding carries one and its own search deliberately
|
||||
ignores it (scoping on the note instead, or shared records become
|
||||
unreachable). Rather than repeat a column that exists to be ignored, a
|
||||
rule's reach is resolved by joining the rule.
|
||||
"""
|
||||
|
||||
__tablename__ = "rule_embeddings"
|
||||
|
||||
rule_id: Mapped[int] = mapped_column(
|
||||
BigInteger,
|
||||
ForeignKey("rules.id", ondelete="CASCADE"),
|
||||
primary_key=True,
|
||||
)
|
||||
chunk_index: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||
embedding: Mapped[list] = mapped_column(Vector(EMBEDDING_DIM), nullable=False)
|
||||
# Exactly what this vector encodes — inspectable when a ranking surprises.
|
||||
# For a rule this is the trigger-first document, NOT the rule's `why`:
|
||||
# `why` is dated incident narrative and would drag every rule toward one
|
||||
# centroid (measured in note 2485).
|
||||
chunk_text: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
chunker_version: Mapped[int] = mapped_column(Integer, nullable=False)
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
@@ -81,7 +81,8 @@ _BACKED_UP = [
|
||||
|
||||
# Tables intentionally NOT in the backup, surfaced in the payload so the gap is
|
||||
# explicit rather than silent. ACL (groups/shares) is a coherent follow-up;
|
||||
# note_embeddings are derived (regenerated from note bodies); api_keys are
|
||||
# note_embeddings and rule_embeddings are derived (regenerated at startup
|
||||
# from the records themselves, which is also how a chunker bump is handled); api_keys are
|
||||
# sensitive credentials; retrieval_logs is observational telemetry that nothing
|
||||
# reads for correctness and that grows per query; the rest are
|
||||
# transient/operational.
|
||||
@@ -91,7 +92,7 @@ _BACKED_UP = [
|
||||
# like coverage while naming nothing the schema could confirm.
|
||||
_NOT_INCLUDED = [
|
||||
"groups", "group_memberships", "project_shares", "note_shares",
|
||||
"api_keys", "note_embeddings", "app_logs", "notifications",
|
||||
"api_keys", "note_embeddings", "rule_embeddings", "app_logs", "notifications",
|
||||
"invitation_tokens", "password_reset_tokens", "user_profiles",
|
||||
"retrieval_logs",
|
||||
# Sensitive credentials, same reasoning as api_keys: a backup that carries
|
||||
|
||||
@@ -19,7 +19,7 @@ from collections.abc import Sequence
|
||||
from sqlalchemy import delete, or_, select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.embedding import NoteEmbedding
|
||||
from scribe.models.embedding import NoteEmbedding, RuleEmbedding
|
||||
from scribe.models.note import Note
|
||||
from scribe.services.access import notes_visibility_clause
|
||||
|
||||
@@ -612,3 +612,186 @@ async def backfill_note_embeddings() -> None:
|
||||
await asyncio.sleep(0.05) # gentle pacing
|
||||
|
||||
logger.info("Embedding backfill complete: %d/%d notes embedded", success, len(notes_to_embed))
|
||||
|
||||
|
||||
# ── Rules (milestone 307, note 3026) ────────────────────────────────────
|
||||
|
||||
def rule_document(
|
||||
title: str | None, statement: str | None, when_to_apply: str | None,
|
||||
) -> tuple[str | None, str | None]:
|
||||
"""The (title, body) a rule is EMBEDDED as — trigger first, `why` never.
|
||||
|
||||
Both halves of this are measured, not guessed (note 2485). That pass found
|
||||
the snippet was the only sharp record in the corpus — a 0.153 top-to-second
|
||||
gap against 0.010–0.023 for everything else — and that the cause was its
|
||||
SHAPE: `{name} — {when_to_use}` as the title and `**When to use:** …`
|
||||
repeated in the body, so purpose appears twice in a short document and
|
||||
dominates the vector. This mirrors that exactly.
|
||||
|
||||
And it excludes `why` on the same evidence. `why` is dated incident
|
||||
narrative — rule 46's runs to 4,300 characters of it — and long,
|
||||
multi-topic prose is precisely what made sixteen dev-logs mutually
|
||||
indistinguishable: the average lands on the centroid of "development",
|
||||
which every one of them shares. Adding `why` would not give the vector more
|
||||
to work with; it would give every rule the same thing to work with.
|
||||
|
||||
A rule with no trigger yet degrades to title + statement. It still embeds,
|
||||
just less sharply — which is an argument for backfilling triggers, not an
|
||||
argument for padding the document with whatever text is lying around.
|
||||
"""
|
||||
trigger = (when_to_apply or "").strip()
|
||||
name = (title or "").strip()
|
||||
body = (statement or "").strip()
|
||||
if not trigger:
|
||||
return name or None, body or None
|
||||
return (
|
||||
f"{name} — {trigger}" if name else trigger,
|
||||
f"When to apply: {trigger}\n\n{body}" if body else f"When to apply: {trigger}",
|
||||
)
|
||||
|
||||
|
||||
async def upsert_rule_embedding(
|
||||
rule_id: int, title: str | None, statement: str | None,
|
||||
when_to_apply: str | None = None,
|
||||
) -> None:
|
||||
"""Chunk, embed and persist a rule's vectors. Safe to fire-and-forget.
|
||||
|
||||
The note twin's contract, for the same reasons: the document is built HERE
|
||||
so the write path, the backfill and any re-embed share one definition, and
|
||||
replacement is atomic per rule so a concurrent read sees the old chunk set
|
||||
or the new one, never a mixture.
|
||||
"""
|
||||
doc_title, doc_body = rule_document(title, statement, when_to_apply)
|
||||
chunks = chunk_document(doc_title, doc_body)
|
||||
try:
|
||||
if not chunks:
|
||||
async with async_session() as session:
|
||||
await session.execute(
|
||||
delete(RuleEmbedding).where(RuleEmbedding.rule_id == rule_id)
|
||||
)
|
||||
await session.commit()
|
||||
return
|
||||
except Exception:
|
||||
logger.warning("Failed to clear embedding for rule %d", rule_id, exc_info=True)
|
||||
return
|
||||
|
||||
try:
|
||||
vectors = await get_embeddings(chunks)
|
||||
except Exception:
|
||||
logger.debug("Skipping embedding for rule %d — embedder unavailable", rule_id)
|
||||
return
|
||||
|
||||
try:
|
||||
async with async_session() as session:
|
||||
await session.execute(
|
||||
delete(RuleEmbedding).where(RuleEmbedding.rule_id == rule_id)
|
||||
)
|
||||
for index, (chunk, vector) in enumerate(zip(chunks, vectors)):
|
||||
session.add(
|
||||
RuleEmbedding(
|
||||
rule_id=rule_id,
|
||||
chunk_index=index,
|
||||
embedding=vector,
|
||||
chunk_text=chunk,
|
||||
chunker_version=CHUNKER_VERSION,
|
||||
)
|
||||
)
|
||||
await session.commit()
|
||||
except Exception:
|
||||
logger.warning("Failed to persist embedding for rule %d", rule_id, exc_info=True)
|
||||
|
||||
|
||||
async def semantic_search_rules(
|
||||
user_id: int,
|
||||
query: str,
|
||||
limit: int = 5,
|
||||
threshold: float = _SIMILARITY_THRESHOLD,
|
||||
) -> list[tuple[float, "Rule"]]:
|
||||
"""Return up to *limit* (score, rule) pairs most relevant to *query*.
|
||||
|
||||
Scoped by OWNERSHIP — a rule is the caller's if they own its rulebook or
|
||||
its project. Deliberately not filtered to what currently BINDS a given
|
||||
project: this answers "is there a rule about this", which a person asking
|
||||
wants answered across their whole rulebook. Deciding which rules bind where
|
||||
is the surfacing question, and it has its own machinery
|
||||
(get_applicable_rules) rather than a second, subtly different copy here.
|
||||
|
||||
Collapses to best-chunk-per-rule like the note search, so a long rule split
|
||||
across chunks competes once rather than crowding the results with itself.
|
||||
|
||||
Returns an empty list if the embedder is unavailable or on any error.
|
||||
"""
|
||||
from scribe.models.project import Project
|
||||
from scribe.models.rulebook import Rule, Rulebook, RulebookTopic
|
||||
|
||||
if not query or not query.strip():
|
||||
return []
|
||||
try:
|
||||
query_vec = await get_embedding(query)
|
||||
except Exception:
|
||||
logger.debug("Rule search skipped — embedder unavailable")
|
||||
return []
|
||||
|
||||
max_distance = min(2.0, max(0.0, 1.0 - threshold))
|
||||
distance = RuleEmbedding.embedding.cosine_distance(query_vec)
|
||||
|
||||
try:
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(Rule, distance.label("distance"))
|
||||
.select_from(RuleEmbedding)
|
||||
.join(Rule, RuleEmbedding.rule_id == Rule.id)
|
||||
.outerjoin(RulebookTopic, Rule.topic_id == RulebookTopic.id)
|
||||
.outerjoin(Rulebook, RulebookTopic.rulebook_id == Rulebook.id)
|
||||
.outerjoin(Project, Rule.project_id == Project.id)
|
||||
.where(
|
||||
Rule.deleted_at.is_(None),
|
||||
distance <= max_distance,
|
||||
# topic_id XOR project_id, so exactly one arm can match.
|
||||
or_(
|
||||
Rulebook.owner_user_id == user_id,
|
||||
Project.user_id == user_id,
|
||||
),
|
||||
)
|
||||
# Overfetch so collapsing chunks to their best row still fills
|
||||
# the page — the same reason the note search overfetches.
|
||||
.order_by(distance)
|
||||
.limit(limit * _CHUNK_OVERFETCH)
|
||||
)).all()
|
||||
except Exception:
|
||||
logger.warning("Rule semantic search failed", exc_info=True)
|
||||
return []
|
||||
|
||||
best: dict[int, tuple[float, object]] = {}
|
||||
for rule, dist in rows:
|
||||
score = 1.0 - float(dist)
|
||||
if rule.id not in best or score > best[rule.id][0]:
|
||||
best[rule.id] = (score, rule)
|
||||
ranked = sorted(best.values(), key=lambda pair: pair[0], reverse=True)
|
||||
return ranked[:limit]
|
||||
|
||||
|
||||
async def backfill_rule_embeddings() -> None:
|
||||
"""Embed rules that have no current vectors. Runs at startup beside the
|
||||
note backfill; a CHUNKER_VERSION bump re-embeds rather than wiping."""
|
||||
from scribe.models.rulebook import Rule
|
||||
|
||||
try:
|
||||
async with async_session() as session:
|
||||
current = select(RuleEmbedding.rule_id).where(
|
||||
RuleEmbedding.chunker_version == CHUNKER_VERSION
|
||||
)
|
||||
stale = (await session.execute(
|
||||
select(Rule.id, Rule.title, Rule.statement, Rule.when_to_apply)
|
||||
.where(Rule.deleted_at.is_(None), Rule.id.notin_(current))
|
||||
)).all()
|
||||
except Exception:
|
||||
logger.warning("Rule embedding backfill: failed to query rules", exc_info=True)
|
||||
return
|
||||
|
||||
if not stale:
|
||||
logger.info("Rule embedding backfill: all rules current at chunker v%d", CHUNKER_VERSION)
|
||||
return
|
||||
logger.info("Rule embedding backfill: embedding %d rule(s)", len(stale))
|
||||
for rule_id, title, statement, when_to_apply in stale:
|
||||
await upsert_rule_embedding(rule_id, title, statement, when_to_apply)
|
||||
|
||||
@@ -334,6 +334,30 @@ def rule_brief(rule: Rule, **extra) -> dict:
|
||||
return out
|
||||
|
||||
|
||||
def _refresh_rule_embedding(rule: Rule) -> None:
|
||||
"""Re-index a rule after a write. Fire-and-forget, like the note twin.
|
||||
|
||||
Lazy import so this module doesn't pull in the embedder; every exception
|
||||
swallowed because a rule that SAVED must not fail on its index refresh —
|
||||
a stale vector costs a missed search hit, a raised exception costs the
|
||||
write. No running loop (unit tests, scripts) is ordinary, not an error.
|
||||
"""
|
||||
try:
|
||||
import asyncio
|
||||
|
||||
from scribe.services.embeddings import upsert_rule_embedding
|
||||
|
||||
asyncio.create_task(
|
||||
upsert_rule_embedding(
|
||||
rule.id, rule.title, rule.statement, rule.when_to_apply,
|
||||
)
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # no running loop — a sync caller, not a failure
|
||||
except Exception: # noqa: BLE001 - never let indexing break a write
|
||||
logger.exception("embedding refresh failed for rule %s", rule.id)
|
||||
|
||||
|
||||
async def rule_detail(user_id: int, rule: Rule, system_ids: list[int] | None = None) -> dict:
|
||||
"""The full record, with its areas and edges attached.
|
||||
|
||||
@@ -380,6 +404,7 @@ async def create_rule(
|
||||
session.add(rule)
|
||||
await session.commit()
|
||||
await session.refresh(rule)
|
||||
_refresh_rule_embedding(rule)
|
||||
return rule
|
||||
|
||||
|
||||
@@ -410,6 +435,7 @@ async def create_project_rule(
|
||||
session.add(rule)
|
||||
await session.commit()
|
||||
await session.refresh(rule)
|
||||
_refresh_rule_embedding(rule)
|
||||
return rule
|
||||
|
||||
|
||||
@@ -609,6 +635,7 @@ async def update_rule(rule_id: int, user_id: int, **fields) -> Optional[Rule]:
|
||||
setattr(rule, key, _valid_tier(value) if key == "tier" else value)
|
||||
await session.commit()
|
||||
await session.refresh(rule)
|
||||
_refresh_rule_embedding(rule)
|
||||
return rule
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
"""The document a rule is EMBEDDED as (milestone 307 step 4, note 3026).
|
||||
|
||||
This shape is measured, not chosen. Note 2485 probed the live corpus and found
|
||||
the snippet was the only discriminative record in it — a 0.153 top-to-second
|
||||
gap against 0.010-0.023 for everything else — and that the cause was its shape:
|
||||
purpose stated twice in a short, single-topic document. These cases pin that
|
||||
recipe onto rules, and pin the exclusion that matters more than any of it.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from scribe.services.embeddings import chunk_document, rule_document
|
||||
|
||||
|
||||
def test_the_trigger_appears_twice_which_is_what_makes_a_vector_sharp():
|
||||
"""Repetition of purpose + brevity is the measured cause of the snippet's
|
||||
separation. The rule document reproduces it exactly: the trigger in the
|
||||
title, and again as the body's first line."""
|
||||
title, body = rule_document(
|
||||
"Release — never without explicit request",
|
||||
"Never cut a release without the operator explicitly asking.",
|
||||
"before cutting any release",
|
||||
)
|
||||
assert title == "Release — never without explicit request — before cutting any release"
|
||||
assert body.startswith("When to apply: before cutting any release")
|
||||
assert "Never cut a release" in body
|
||||
|
||||
|
||||
def test_why_is_never_embedded():
|
||||
"""The exclusion this whole design turns on. `why` is dated incident
|
||||
narrative — rule 46's runs to 4,300 characters — and long multi-topic prose
|
||||
is what made sixteen dev-logs mutually indistinguishable: the average lands
|
||||
on a centroid they all share. Adding it would not give the vector more to
|
||||
work with; it would give every rule the SAME thing to work with.
|
||||
|
||||
rule_document takes no `why` parameter at all, which is the strongest form
|
||||
of this guarantee: it cannot be passed in by a caller who means well.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
assert "why" not in inspect.signature(rule_document).parameters
|
||||
|
||||
|
||||
def test_a_rule_with_no_trigger_still_embeds_just_less_sharply():
|
||||
"""Every rule written before milestone 307 has no trigger. Degrading to
|
||||
title + statement keeps them findable; it does NOT pad the document with
|
||||
whatever text is lying around, which would be the tempting fix and the
|
||||
wrong one."""
|
||||
title, body = rule_document("dev is home", "Work directly on dev.", "")
|
||||
assert title == "dev is home"
|
||||
assert body == "Work directly on dev."
|
||||
assert "When to apply" not in body
|
||||
|
||||
|
||||
def test_an_empty_rule_yields_no_document_and_therefore_no_vector():
|
||||
"""Callers gate on falsiness to skip embedding — the same contract
|
||||
chunk_document has, so an emptied record stops being findable by its old
|
||||
content rather than keeping a stale vector."""
|
||||
assert rule_document("", "", "") == (None, None)
|
||||
assert chunk_document(*rule_document("", "", "")) == []
|
||||
|
||||
|
||||
def test_a_long_rule_chunks_and_every_chunk_carries_the_trigger():
|
||||
"""Rule 46's statement is ~3,600 characters across two headed sections.
|
||||
The existing chunker splits it at headings and prefixes each chunk with the
|
||||
title — which now CONTAINS the trigger, so each half stays anchored to what
|
||||
the rule is for. This is why splitting a merged rule costs nothing at
|
||||
retrieval time."""
|
||||
statement = (
|
||||
"## The tags\n\n" + ("Four tags, four jobs. " * 60)
|
||||
+ "\n\n## The artifact's own version\n\n" + ("Two version values. " * 60)
|
||||
)
|
||||
title, body = rule_document("Versioning", statement, "when cutting a release")
|
||||
chunks = chunk_document(title, body)
|
||||
assert len(chunks) > 1
|
||||
assert all("when cutting a release" in chunk for chunk in chunks)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("trigger,expected_title", [
|
||||
("before any git push", "dev is home — before any git push"),
|
||||
(" before any git push ", "dev is home — before any git push"),
|
||||
])
|
||||
def test_the_trigger_is_trimmed_before_it_reaches_the_vector(trigger, expected_title):
|
||||
title, _ = rule_document("dev is home", "Work on dev.", trigger)
|
||||
assert title == expected_title
|
||||
Reference in New Issue
Block a user