fix(retrieval): the rules never-opened warning stops sending the reader to a review tool that refuses rules (#4798)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m7s
CI & Build / Python tests (push) Successful in 1m55s
CI & Build / Build & push image (push) Successful in 26s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m7s
CI & Build / Python tests (push) Successful in 1m55s
CI & Build / Build & push image (push) Successful in 26s
#4798 "The rules corpus's surfaced_never_pulled warning sends the reader to menus_to_review, which can only review auto_inject". #4772 gave the warning one remedy for both corpora: "judge a sample with menus_to_review". That is right for notes, where a line carries its passage. For rules it is a dead end: retrieval_review.REVIEWABLE holds only auto_inject, so the tool refuses every rule arm. - The reading is now per corpus (_NEVER_PULLED_READING). For rules, the text says: - the count includes rules that only arrived in a listing; - a rule can rightly be set aside on its trigger alone; - no judged sample exists for the rule arms; - a rule set aside again and again is a trigger to fix (update_rule when_to_apply, then what_might_apply), not a floor. - The tool docstring says the same. - The guard is tied to REVIEWABLE, so it can fail in both directions: if the rules text names menus_to_review while no rule arm is reviewable, or if a rule arm becomes reviewable and the text still says there is no sample. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -624,6 +624,8 @@ It is an UPPER BOUND per surface: a pull records the door it came
|
|||||||
corpus. Not a verdict on them: a line carries its matched passage, so
|
corpus. Not a verdict on them: a line carries its matched passage, so
|
||||||
an unopened record may have been unrelated, enough as shown, or already
|
an unopened record may have been unrelated, enough as shown, or already
|
||||||
in context. Judge a sample (`menus_to_review`) before acting on it.
|
in context. Judge a sample (`menus_to_review`) before acting on it.
|
||||||
|
For RULES there is no judged sample: a rule set aside again and again
|
||||||
|
is a trigger to fix (`update_rule(when_to_apply=...)`), not a floor.
|
||||||
- `read_and_unacted` — distinct rules OPENED in the window that recorded
|
- `read_and_unacted` — distinct rules OPENED in the window that recorded
|
||||||
no outcome, against the ones that did. The failure milestone 419 was
|
no outcome, against the ones that did. The failure milestone 419 was
|
||||||
opened on, and the worse sibling of `surfaced_never_pulled` above: a
|
opened on, and the worse sibling of `surfaced_never_pulled` above: a
|
||||||
|
|||||||
@@ -492,6 +492,34 @@ def _num(raw: str, fallback):
|
|||||||
return fallback
|
return fallback
|
||||||
|
|
||||||
|
|
||||||
|
# What "surfaced and never opened" can and cannot mean, per corpus, and where
|
||||||
|
# to look next. ONE SENTENCE PER CORPUS because the two lines are built
|
||||||
|
# differently (#4798): a note line carries its matched passage, so an unopened
|
||||||
|
# note may have done its job, and the judged sample is the instrument that can
|
||||||
|
# tell. A rule line carries only its trigger and asks to be opened, and no
|
||||||
|
# judged sample covers the rule arms (`retrieval_review.REVIEWABLE`), so
|
||||||
|
# sending that reader to `menus_to_review` sent them to a refusal.
|
||||||
|
_NEVER_PULLED_READING = {
|
||||||
|
"notes": (
|
||||||
|
"That is not a verdict on them: a line carries its matched passage, "
|
||||||
|
"so an unopened record may have been unrelated, enough as shown, or "
|
||||||
|
"already in context. Before acting on it — a title, a floor, a "
|
||||||
|
"budget — judge a sample with `menus_to_review` and read the "
|
||||||
|
"`judged` block."
|
||||||
|
),
|
||||||
|
"rules": (
|
||||||
|
"Not a verdict either, and read differently from notes: the count "
|
||||||
|
"includes rules that only arrived in a listing (the project "
|
||||||
|
"handshake, a planning read), and a rule line shows its trigger, so a "
|
||||||
|
"session can rightly set one aside on the trigger alone. There is no "
|
||||||
|
"judged sample for the rule arms. A rule set aside again and again "
|
||||||
|
"where it does not apply is a trigger to fix, not a floor to move — "
|
||||||
|
"`update_rule(when_to_apply=...)`, then `what_might_apply` with the "
|
||||||
|
"moment's own words to see where it ranks."
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def _warn(code, detail, source=None, **numbers) -> dict:
|
def _warn(code, detail, source=None, **numbers) -> dict:
|
||||||
"""One finding, carrying the numbers that produced it.
|
"""One finding, carrying the numbers that produced it.
|
||||||
|
|
||||||
@@ -767,11 +795,7 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict,
|
|||||||
out.append(_warn(
|
out.append(_warn(
|
||||||
"surfaced_never_pulled",
|
"surfaced_never_pulled",
|
||||||
f"{never} of {shown} distinct {label} were surfaced in this "
|
f"{never} of {shown} distinct {label} were surfaced in this "
|
||||||
f"window and never opened. That is not a verdict on them: a "
|
f"window and never opened. {_NEVER_PULLED_READING[label]}",
|
||||||
f"line carries its matched passage, so an unopened record may "
|
|
||||||
f"have been unrelated, enough as shown, or already in context. "
|
|
||||||
f"Before acting on it — a title, a floor, a budget — judge a "
|
|
||||||
f"sample with `menus_to_review` and read the `judged` block.",
|
|
||||||
source=None, corpus=label,
|
source=None, corpus=label,
|
||||||
surfaced=int(shown), pulled=int(pulled or 0), never_pulled=never,
|
surfaced=int(shown), pulled=int(pulled or 0), never_pulled=never,
|
||||||
))
|
))
|
||||||
|
|||||||
@@ -182,6 +182,29 @@ def test_records_shown_and_never_opened_are_reported_per_corpus() -> None:
|
|||||||
assert found["rules"]["never_pulled"] == 58
|
assert found["rules"]["never_pulled"] == 58
|
||||||
|
|
||||||
|
|
||||||
|
def test_each_corpus_is_sent_only_to_a_tool_that_takes_it() -> None:
|
||||||
|
"""#4798: one remedy for both corpora sent the rules reader to
|
||||||
|
`menus_to_review`, which refuses every source but the notes menu. Tied to
|
||||||
|
`REVIEWABLE` itself, so it goes red in either direction: the rules text
|
||||||
|
naming the tool while no rule arm is reviewable, or a rule arm becoming
|
||||||
|
reviewable while the text still says there is no sample."""
|
||||||
|
from scribe.services.retrieval_review import REVIEWABLE
|
||||||
|
|
||||||
|
ws = warn(
|
||||||
|
{},
|
||||||
|
usage={"distinct_notes_surfaced": 9, "distinct_notes_pulled": 1},
|
||||||
|
rule_usage={"distinct_rules_surfaced": 9, "distinct_rules_pulled": 1},
|
||||||
|
)
|
||||||
|
detail = {w["numbers"]["corpus"]: w["detail"]
|
||||||
|
for w in ws if w["code"] == "surfaced_never_pulled"}
|
||||||
|
assert set(detail) == {"notes", "rules"}
|
||||||
|
assert "auto_inject" in REVIEWABLE
|
||||||
|
assert "menus_to_review" in detail["notes"]
|
||||||
|
rule_arms = {"prompt_rule", "pre_tool_rule", "write_path_rule"}
|
||||||
|
assert ("menus_to_review" in detail["rules"]) == bool(rule_arms & set(REVIEWABLE))
|
||||||
|
assert "when_to_apply" in detail["rules"], "the rules text names no next step"
|
||||||
|
|
||||||
|
|
||||||
def test_everything_opened_reports_nothing() -> None:
|
def test_everything_opened_reports_nothing() -> None:
|
||||||
ws = warn({}, usage={"distinct_notes_surfaced": 5, "distinct_notes_pulled": 5})
|
ws = warn({}, usage={"distinct_notes_surfaced": 5, "distinct_notes_pulled": 5})
|
||||||
assert "surfaced_never_pulled" not in codes(ws)
|
assert "surfaced_never_pulled" not in codes(ws)
|
||||||
|
|||||||
Reference in New Issue
Block a user