feat(rules): the write path can notice a standing rule it was never given (#3031, milestone 307 step 5, hook arm)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Successful in 24s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Successful in 24s
A conditional rule is not resident, so a session can be about to violate one it was never handed. This arm notices: when what is being written resembles a rule's trigger, the hint names it and says to read it before deciding it does not apply. A SUGGESTION, and the plan was wrong about why it could be more. It claimed the hook "already resolves a path to an area" — it does not, and nothing in Scribe maps a path to a System or a canonical area (build_write_path_hint resolves paths against snippet LOCATIONS, a different index; the learned-alias idea belongs to another project). Correction logged on the task. Rather than invent path→area inference to make a stale claim true, the arm does what D7 already decided and what this surface already IS: tags bind at enter_project, meaning suggests here. The header of the hook says NEVER BLOCKS; dressing a hint up as binding would have been the actual mistake. CONDITIONAL RULES ONLY. An always-on rule is already in the session, so re-offering it is noise — and noise on a hint that fires on every write is how a hint gets ignored. Telemetry goes to retrieval_logs, NOT note_usage_events, and that is a correctness call rather than a preference: note_usage ids are REMAPPED on a backup restore, so a rule id written there would come back attached to whatever note took that number — silently corrupting the evidence the next true-up is supposed to read. retrieval_logs is never restored and `source` already separates surfaces. record_retrieval's `results` type widened to match what it actually needs (an `.id`), instead of passing a Rule to something annotated Note. The rule dedup gets its OWN state file and query parameter, like the three channels before it — #2708's lesson was that one shared channel lets a hint of one class silence a different class that had never been shown. Plugin version bumped: a hook change clients cannot see did not ship (#1040). The stub is autouse in conftest rather than added to forty-odd call sites: the arm loads an embedding model, and every existing test that stubs the NOTES search would otherwise pull a real model in through the one arm it had no way to know about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -711,6 +711,7 @@ async def semantic_search_rules(
|
||||
query: str,
|
||||
limit: int = 5,
|
||||
threshold: float = _SIMILARITY_THRESHOLD,
|
||||
tier: str | None = None,
|
||||
) -> list[tuple[float, "Rule"]]:
|
||||
"""Return up to *limit* (score, rule) pairs most relevant to *query*.
|
||||
|
||||
@@ -721,6 +722,11 @@ async def semantic_search_rules(
|
||||
is the surfacing question, and it has its own machinery
|
||||
(get_applicable_rules) rather than a second, subtly different copy here.
|
||||
|
||||
`tier` narrows to one tier. The write-path hint passes "conditional",
|
||||
because an always-on rule is ALREADY in the session — surfacing it again as
|
||||
a suggestion is pure noise, and noise on a hint that fires on every write
|
||||
is how a hint gets ignored.
|
||||
|
||||
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.
|
||||
|
||||
@@ -757,6 +763,7 @@ async def semantic_search_rules(
|
||||
Rulebook.owner_user_id == user_id,
|
||||
Project.user_id == user_id,
|
||||
),
|
||||
*( [Rule.tier == tier] if tier else [] ),
|
||||
)
|
||||
# Overfetch so collapsing chunks to their best row still fills
|
||||
# the page — the same reason the note search overfetches.
|
||||
|
||||
@@ -30,7 +30,7 @@ from scribe.services import rulebooks as rulebooks_svc
|
||||
from scribe.services import shape_ledger as shape_ledger_svc
|
||||
from scribe.services import snippets as snippets_svc
|
||||
from scribe.services.access import label_shared_items, owner_names_for
|
||||
from scribe.services.embeddings import semantic_search_notes
|
||||
from scribe.services.embeddings import semantic_search_notes, semantic_search_rules
|
||||
from scribe.services.note_usage import record_surfaced
|
||||
from scribe.services.supersession import superseded_ids
|
||||
from scribe.services.retrieval_telemetry import record_retrieval
|
||||
@@ -707,6 +707,7 @@ async def build_write_path_hint(
|
||||
stamp_shapes: list[tuple[str, str]] | None = None,
|
||||
repo_key: str = "",
|
||||
exclude_derive: list[str] | None = None,
|
||||
exclude_rule_ids: list[int] | None = None,
|
||||
) -> dict:
|
||||
"""Prior-art hint for the plugin's PreToolUse hook on Write/Edit.
|
||||
|
||||
@@ -766,7 +767,8 @@ async def build_write_path_hint(
|
||||
"""
|
||||
cfg = await get_writepath_config(user_id)
|
||||
empty = {"context": "", "note_ids": [], "sync_note_ids": [], "config": cfg,
|
||||
"stamped": [], "divergence": [], "derive": [], "derive_keys": []}
|
||||
"stamped": [], "divergence": [], "derive": [], "derive_keys": [],
|
||||
"rule_ids": []}
|
||||
path = (path or "").strip()
|
||||
if not cfg["enabled"] or not path:
|
||||
return empty
|
||||
@@ -1036,6 +1038,50 @@ async def build_write_path_hint(
|
||||
for arm, ids in by_arm.items():
|
||||
record_surfaced(user_id=user_id, note_ids=ids, source=arm)
|
||||
|
||||
# ── Standing rules that may apply here (milestone 307) ──────────────
|
||||
#
|
||||
# A SUGGESTION, not a binding surface, and the distinction is the design
|
||||
# (D7): a rule BINDS by being tagged to an area the project works in,
|
||||
# resolved deterministically at enter_project. This arm reaches for
|
||||
# something weaker and still useful — a conditional rule whose trigger
|
||||
# resembles what is being written, noticed at the moment it is relevant
|
||||
# rather than by being resident in every session.
|
||||
#
|
||||
# CONDITIONAL ONLY. An always-on rule is already in the session; repeating
|
||||
# it here would be noise, and noise on a hint that fires on every write is
|
||||
# how a hint gets ignored.
|
||||
#
|
||||
# Fails open like every other arm: a rule hint must never break a write.
|
||||
rule_ids: list[int] = []
|
||||
try:
|
||||
already = set(exclude_rule_ids or [])
|
||||
hits = await semantic_search_rules(
|
||||
user_id, code or path, limit=2,
|
||||
threshold=cfg["threshold"], tier="conditional",
|
||||
)
|
||||
fresh = [(score, rule) for score, rule in hits if rule.id not in already]
|
||||
for _score, rule in fresh:
|
||||
trigger = (rule.when_to_apply or "").strip()
|
||||
lines.append(
|
||||
f"Standing rule that may apply here — \u201c{rule.title}\u201d"
|
||||
+ (f" ({trigger})" if trigger else "")
|
||||
+ f". Read it with get_rule({rule.id}) before deciding it "
|
||||
"does not apply; it is not in this session's loaded set."
|
||||
)
|
||||
rule_ids.append(rule.id)
|
||||
if fresh:
|
||||
# retrieval_logs, NOT note_usage_events: that table's ids are
|
||||
# remapped on a backup restore, so a rule id there would return
|
||||
# attached to whatever note took that number. This one is never
|
||||
# restored, and `source` already separates the surfaces.
|
||||
record_retrieval(
|
||||
user_id=user_id, source="write_path_rule", query=code or path,
|
||||
threshold=cfg["threshold"], limit=2, project_id=project_id,
|
||||
is_task=None, results=fresh,
|
||||
)
|
||||
except Exception:
|
||||
logger.debug("write-path rule arm failed", exc_info=True)
|
||||
|
||||
return {
|
||||
"context": "\n".join(lines),
|
||||
"note_ids": note_ids,
|
||||
@@ -1045,6 +1091,7 @@ async def build_write_path_hint(
|
||||
"divergence": divergence,
|
||||
"derive": derive,
|
||||
"derive_keys": [d["key"] for d in derive],
|
||||
"rule_ids": rule_ids,
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
@@ -108,11 +109,19 @@ def record_retrieval(
|
||||
limit: int | None,
|
||||
project_id: int | None,
|
||||
is_task: bool | None,
|
||||
results: list[tuple[float, Note]],
|
||||
results: list[tuple[float, Any]],
|
||||
duration_ms: float | None = None,
|
||||
) -> None:
|
||||
"""Fire-and-forget: record one retrieval call.
|
||||
|
||||
`results` needs only `.id` on each record, which is why it is not typed to
|
||||
Note: rules are retrieved too (milestone 307) and land here rather than in
|
||||
note_usage_events. That table's ids are REMAPPED on a backup restore, so a
|
||||
rule id written into it would come back attached to whatever note happened
|
||||
to take that number — silent corruption of the very evidence this exists to
|
||||
provide. retrieval_logs is not restored at all, so it has no such hazard,
|
||||
and `source` already distinguishes the surfaces.
|
||||
|
||||
Builds the payload inline (synchronously) then schedules the insert so the
|
||||
caller returns immediately. Never raises — telemetry must not affect search.
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user