refactor(retrieval): the completion-report preferences run through the pipeline (milestone 456 step 3, #4905)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m45s
CI & Build / Build & push image (push) Successful in 28s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m45s
CI & Build / Build & push image (push) Successful in 28s
reply_preferences.completion_preferences was the last hand-written rule search + record_retrieval + record_rule_surfaced triple outside the pipeline. It is now REPORT_PREFERENCE, a RuleArm with kind="preference" over COMPLETION_QUERY, read back as records rather than as lines. - RuleArm gains `kind`. _ranked asks the ranker for that kind and drops anything else on the way out, before the row is logged. - RuleResult gains `shown`, the (score, rule) pairs behind the lines, for callers that return records. - RuleMoment.project_id may be None: logged as given, searched as `project_id or None`. report_preference rows keep their NULL project. - Guards: reply_preferences now has 0 direct rule searches. The registry constant-source example moves from report_preference to preference_slot, the pipeline slot that records through a module constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -58,8 +58,8 @@ bar can be wrong forever without a single call looking unusual.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
import time
|
|
||||||
|
|
||||||
|
from scribe.services import retrieval_pipeline as rp
|
||||||
from scribe.services.embeddings import semantic_search_rules
|
from scribe.services.embeddings import semantic_search_rules
|
||||||
from scribe.services.retrieval_surfaces import SURFACES, budget_for, floor_for
|
from scribe.services.retrieval_surfaces import SURFACES, budget_for, floor_for
|
||||||
from scribe.services.retrieval_telemetry import record_retrieval
|
from scribe.services.retrieval_telemetry import record_retrieval
|
||||||
@@ -67,7 +67,7 @@ from scribe.services.rule_usage import record_rule_surfaced
|
|||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
SOURCE = "report_preference"
|
SOURCE = rp.REPORT_PREFERENCE.source
|
||||||
|
|
||||||
# Written in the vocabulary of the MOMENT, because that is what a trigger is
|
# Written in the vocabulary of the MOMENT, because that is what a trigger is
|
||||||
# written in and what this query is scored against. Domain-neutral on purpose
|
# written in and what this query is scored against. Domain-neutral on purpose
|
||||||
@@ -114,34 +114,27 @@ async def completion_preferences(user_id: int, *, project_id: int | None = None)
|
|||||||
happened, and a lookup that errors must not turn it into a failure.
|
happened, and a lookup that errors must not turn it into a failure.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
threshold = await _threshold(user_id)
|
# The stages — the kind filter, the unconditional call row, the
|
||||||
limit = await _limit(user_id)
|
# fresh-only surfacing rows — are the one pipeline's (milestone 456).
|
||||||
report: dict = {}
|
|
||||||
t0 = time.perf_counter()
|
|
||||||
hits = await semantic_search_rules(
|
|
||||||
user_id, COMPLETION_QUERY, limit=limit, threshold=threshold,
|
|
||||||
kind="preference", report=report, project_id=project_id,
|
|
||||||
)
|
|
||||||
hits = [(score, rule) for score, rule in hits if rule.kind == "preference"]
|
|
||||||
record_retrieval(
|
|
||||||
user_id=user_id, source=SOURCE, query=COMPLETION_QUERY,
|
|
||||||
threshold=threshold, limit=limit, project_id=project_id,
|
|
||||||
is_task=None, results=hits,
|
|
||||||
best_available=report.get("best_available_score"),
|
|
||||||
best_available_id=report.get("best_available_id"),
|
|
||||||
searched=bool(report.get("searched", True)),
|
|
||||||
duration_ms=(time.perf_counter() - t0) * 1000.0,
|
|
||||||
)
|
|
||||||
if not hits:
|
|
||||||
return []
|
|
||||||
# RANKED: this surface chose what it showed, so the name is in
|
# RANKED: this surface chose what it showed, so the name is in
|
||||||
# rule_usage.RANKED_SOURCES and its hits count toward pull-through.
|
# rule_usage.RANKED_SOURCES and its hits count toward pull-through.
|
||||||
record_rule_surfaced(
|
# There is no session ledger here: a completion report is written
|
||||||
user_id=user_id, rule_ids=[rule.id for _s, rule in hits], source=SOURCE,
|
# once, so nothing it could repeat has been shown before.
|
||||||
|
result = await rp.run_rule_arm(
|
||||||
|
rp.REPORT_PREFERENCE,
|
||||||
|
rp.RuleMoment(
|
||||||
|
user_id=user_id, query=COMPLETION_QUERY, project_id=project_id,
|
||||||
|
),
|
||||||
|
floor=await _threshold(user_id), budget=await _limit(user_id),
|
||||||
|
io=rp.RuleIO(
|
||||||
|
search=semantic_search_rules,
|
||||||
|
record_retrieval=record_retrieval,
|
||||||
|
record_rule_surfaced=record_rule_surfaced,
|
||||||
|
),
|
||||||
)
|
)
|
||||||
return [
|
return [
|
||||||
{"id": rule.id, "title": rule.title, "statement": rule.statement, "kind": "preference"}
|
{"id": rule.id, "title": rule.title, "statement": rule.statement, "kind": "preference"}
|
||||||
for _score, rule in hits
|
for _score, rule in result.shown
|
||||||
]
|
]
|
||||||
except Exception: # noqa: BLE001 - a decoration never breaks its payload
|
except Exception: # noqa: BLE001 - a decoration never breaks its payload
|
||||||
logger.warning("completion preference lookup failed", exc_info=True)
|
logger.warning("completion preference lookup failed", exc_info=True)
|
||||||
|
|||||||
@@ -351,6 +351,9 @@ class RuleArm:
|
|||||||
preference_slot: bool
|
preference_slot: bool
|
||||||
"""Reserve one line for a preference that lost the ranking (#3894)."""
|
"""Reserve one line for a preference that lost the ranking (#3894)."""
|
||||||
|
|
||||||
|
kind: str | None = None
|
||||||
|
"""Ask the ranker for one record kind only ("preference"), or every kind."""
|
||||||
|
|
||||||
|
|
||||||
# Today's differences, reproduced exactly (milestone 456 step 2). Whether the
|
# Today's differences, reproduced exactly (milestone 456 step 2). Whether the
|
||||||
# prompt arm should band, and whether the act arms should reserve a
|
# prompt arm should band, and whether the act arms should reserve a
|
||||||
@@ -367,7 +370,16 @@ WRITE_PATH_RULE = RuleArm(
|
|||||||
"write_path_rule", band=True, compact_tail=True, checkpoint=True,
|
"write_path_rule", band=True, compact_tail=True, checkpoint=True,
|
||||||
preference_slot=False,
|
preference_slot=False,
|
||||||
)
|
)
|
||||||
RULE_ARMS: tuple[RuleArm, ...] = (WRITE_PATH_RULE, PRE_TOOL_RULE, PROMPT_RULE)
|
# The completion report's preferences (milestone 409 step 4): a FIXED query,
|
||||||
|
# preferences only, read by update_task as records rather than as lines. Its
|
||||||
|
# query is `reply_preferences.COMPLETION_QUERY`.
|
||||||
|
REPORT_PREFERENCE = RuleArm(
|
||||||
|
"report_preference", band=False, compact_tail=False, checkpoint=False,
|
||||||
|
preference_slot=False, kind="preference",
|
||||||
|
)
|
||||||
|
RULE_ARMS: tuple[RuleArm, ...] = (
|
||||||
|
WRITE_PATH_RULE, PRE_TOOL_RULE, PROMPT_RULE, REPORT_PREFERENCE,
|
||||||
|
)
|
||||||
|
|
||||||
PREFERENCE_SLOT_SOURCE = "preference_slot"
|
PREFERENCE_SLOT_SOURCE = "preference_slot"
|
||||||
|
|
||||||
@@ -396,8 +408,11 @@ class RuleMoment:
|
|||||||
|
|
||||||
user_id: int
|
user_id: int
|
||||||
query: str
|
query: str
|
||||||
project_id: int
|
project_id: int | None
|
||||||
where: str
|
"""The bound project, 0 or None when unbound. Logged as given, searched as
|
||||||
|
`project_id or None` — global rules plus that project's own (milestone 414)."""
|
||||||
|
|
||||||
|
where: str = ""
|
||||||
"""How a line names the moment: "here", "to this Bash call", …"""
|
"""How a line names the moment: "here", "to this Bash call", …"""
|
||||||
|
|
||||||
checkpoint_where: str = ""
|
checkpoint_where: str = ""
|
||||||
@@ -419,6 +434,10 @@ class RuleResult:
|
|||||||
shown_rule_ids: list[int] = field(default_factory=list)
|
shown_rule_ids: list[int] = field(default_factory=list)
|
||||||
"""Every rule LINE, repeats and the reserved slot included."""
|
"""Every rule LINE, repeats and the reserved slot included."""
|
||||||
|
|
||||||
|
shown: list = field(default_factory=list)
|
||||||
|
"""The (score, rule) pairs behind those lines, best first — for a caller
|
||||||
|
that hands back records rather than lines (the completion report)."""
|
||||||
|
|
||||||
checkpoint: dict = field(default_factory=dict)
|
checkpoint: dict = field(default_factory=dict)
|
||||||
|
|
||||||
|
|
||||||
@@ -462,6 +481,13 @@ async def _ranked(
|
|||||||
# slot on it, indistinguishable from a line that earned its place.
|
# slot on it, indistinguishable from a line that earned its place.
|
||||||
kwargs["kind"] = kind
|
kwargs["kind"] = kind
|
||||||
hits = await io.search(moment.user_id, moment.query, **kwargs)
|
hits = await io.search(moment.user_id, moment.query, **kwargs)
|
||||||
|
if kind:
|
||||||
|
# And checked on the way out: a record of another kind that slipped
|
||||||
|
# through must not be logged, shown or counted under a source whose
|
||||||
|
# name claims the kind — a rule handed back as "how the operator likes
|
||||||
|
# this done" asserts a force the record does not have.
|
||||||
|
hits = [(score, rule) for score, rule in hits
|
||||||
|
if getattr(rule, "kind", None) == kind]
|
||||||
kept = _rule_band(hits) if band else hits
|
kept = _rule_band(hits) if band else hits
|
||||||
fresh = [(score, rule) for score, rule in kept if rule.id not in moment.exclude]
|
fresh = [(score, rule) for score, rule in kept if rule.id not in moment.exclude]
|
||||||
try:
|
try:
|
||||||
@@ -578,7 +604,7 @@ async def run_rule_arm(
|
|||||||
try:
|
try:
|
||||||
_hits, kept, fresh = await _ranked(
|
_hits, kept, fresh = await _ranked(
|
||||||
io, moment, source=arm.source, floor=floor, limit=budget,
|
io, moment, source=arm.source, floor=floor, limit=budget,
|
||||||
band=arm.band,
|
band=arm.band, kind=arm.kind,
|
||||||
)
|
)
|
||||||
shown = kept
|
shown = kept
|
||||||
if arm.preference_slot:
|
if arm.preference_slot:
|
||||||
@@ -631,6 +657,7 @@ async def run_rule_arm(
|
|||||||
lines=lines, rule_ids=rule_ids,
|
lines=lines, rule_ids=rule_ids,
|
||||||
shown_rule_ids=[rule.id for _score, rule in shown],
|
shown_rule_ids=[rule.id for _score, rule in shown],
|
||||||
checkpoint=checkpoint,
|
checkpoint=checkpoint,
|
||||||
|
shown=list(shown),
|
||||||
)
|
)
|
||||||
except Exception: # noqa: BLE001 - a recall aid never breaks its act
|
except Exception: # noqa: BLE001 - a recall aid never breaks its act
|
||||||
logger.debug("%s arm failed", arm.source, exc_info=True)
|
logger.debug("%s arm failed", arm.source, exc_info=True)
|
||||||
|
|||||||
@@ -34,8 +34,8 @@ every tunable surface also appears here, so the two cannot drift apart.
|
|||||||
ADDING AN ARM MEANS ADDING A ROW HERE. `tests/test_retrieval_registry.py`
|
ADDING AN ARM MEANS ADDING A ROW HERE. `tests/test_retrieval_registry.py`
|
||||||
walks the call sites with the ast module and fails on a source it cannot find
|
walks the call sites with the ast module and fails on a source it cannot find
|
||||||
below — deliberately not a grep, because two of the sources in this file
|
below — deliberately not a grep, because two of the sources in this file
|
||||||
(`wide_net`, `report_preference`) reach their recorder as `source=SOURCE`
|
(`wide_net`, `preference_slot`) reach their recorder through a
|
||||||
through a module constant and a grep for `source="` misses both. That is the
|
module constant and a grep for `source="` misses both. That is the
|
||||||
narrowing #3191 warns about, caught here in the act.
|
narrowing #3191 warns about, caught here in the act.
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
WHY AST AND NOT GREP, demonstrated rather than asserted. Two of the sources in
|
WHY AST AND NOT GREP, demonstrated rather than asserted. Two of the sources in
|
||||||
this system reach their recorder as `source=SOURCE` through a module-level
|
this system reach their recorder as `source=SOURCE` through a module-level
|
||||||
constant — `wide_net` and `report_preference` — so a grep for `source="` finds
|
constant — `wide_net` and `preference_slot` — so a grep for `source="` finds
|
||||||
neither. A registry test built on that grep would pass while being blind to
|
neither. A registry test built on that grep would pass while being blind to
|
||||||
two arms, which is the narrowing #3191 warns about: a check that looks
|
two arms, which is the narrowing #3191 warns about: a check that looks
|
||||||
thorough and quietly covers less than it claims.
|
thorough and quietly covers less than it claims.
|
||||||
@@ -101,9 +101,15 @@ def test_the_extractor_finds_something() -> None:
|
|||||||
|
|
||||||
|
|
||||||
def test_the_extractor_resolves_the_constant_sources() -> None:
|
def test_the_extractor_resolves_the_constant_sources() -> None:
|
||||||
"""The specific capability a grep would lose. See the module docstring."""
|
"""The specific capability a grep would lose. See the module docstring.
|
||||||
|
|
||||||
|
`report_preference` was the second example until milestone 456 moved it
|
||||||
|
into the retrieval pipeline, where its source is a spec field; the
|
||||||
|
pipeline's reserved slot still records through a module constant, so it
|
||||||
|
carries the case instead.
|
||||||
|
"""
|
||||||
found, _ = call_sites()
|
found, _ = call_sites()
|
||||||
for via_constant in ("wide_net", "report_preference"):
|
for via_constant in ("wide_net", "preference_slot"):
|
||||||
assert via_constant in found, (
|
assert via_constant in found, (
|
||||||
f"{via_constant} reaches its recorder through a module constant; "
|
f"{via_constant} reaches its recorder through a module constant; "
|
||||||
f"an extractor that cannot resolve one is blind to it"
|
f"an extractor that cannot resolve one is blind to it"
|
||||||
|
|||||||
@@ -1702,7 +1702,7 @@ def test_every_hook_rule_search_says_which_project_it_is_for():
|
|||||||
# ranked-search stage, checked below — so a direct search appearing
|
# ranked-search stage, checked below — so a direct search appearing
|
||||||
# here again is a copy of the arm coming back, and fails the count.
|
# here again is a copy of the arm coming back, and fails the count.
|
||||||
"src/scribe/services/plugin_context.py": 0,
|
"src/scribe/services/plugin_context.py": 0,
|
||||||
"src/scribe/services/reply_preferences.py": 1,
|
"src/scribe/services/reply_preferences.py": 0,
|
||||||
}
|
}
|
||||||
for path, expected in sources.items():
|
for path, expected in sources.items():
|
||||||
calls = [
|
calls = [
|
||||||
|
|||||||
Reference in New Issue
Block a user