From 7faaa311d7b12ad705d81e98ef6f2b53a669de21 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 26 Oct 2026 16:29:40 -0400 Subject: [PATCH] fix(6470): a moment proposal counts only opens no hook explained, and asks the reader to judge it The signal counted every get_rule open against every moment from the previous three minutes, so a rule a search arm had just shown was proposed for whatever moment happened to fire. All nine pending signal proposals were wrong. - The hook skips an open of a rule a hook named this session, and sends the last three acts from 90 seconds instead of 30 from 180. - A rule already mounted anywhere gathers no signal evidence. - A signal pair keeps the actions that reached its moment and shows them as its why. - The proposal line says it is co-occurrence, not a recommendation, and asks the reader to check the rule against the moment before offering it; moments.md says the same. Plugin 2026.10.26.2029. Co-Authored-By: Claude Opus 5.5 --- plugin/.claude-plugin/plugin.json | 2 +- plugin/hooks/grimoire_record_opened.sh | 24 ++++-- plugin/skills/using-grimoire/moments.md | 16 ++-- .../services/rule_moment_judgments.py | 82 +++++++++++++++---- .../test_integration_rule_moment_judgments.py | 18 +++- tests/test_rule_moment_judgments.py | 23 ++++++ tests/test_rule_moment_signal_hooks.py | 43 ++++++++++ 7 files changed, 178 insertions(+), 30 deletions(-) diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 030f3036..905a0391 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "grimoire", "description": "Grimoire for Claude Code: connects the grimoire MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Grimoire skills (using-grimoire, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting, family-canon), and syncs your saved Grimoire Processes as skills (/grimoire:sync).", - "version": "2026.10.22.1945", + "version": "2026.10.26.2029", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/grimoire_record_opened.sh b/plugin/hooks/grimoire_record_opened.sh index 9d1a7ff1..a33b116a 100644 --- a/plugin/hooks/grimoire_record_opened.sh +++ b/plugin/hooks/grimoire_record_opened.sh @@ -72,19 +72,29 @@ printf '%s\n' "$rule_id" | grimoire_rules_append "$state_dir/${safe_sid}.opened. # ── What came just before the open ─────────────────────────────────────── # -# grimoire_moment.sh writes every call to `grimoire-moment/.acts`. The ones -# from the last three minutes go to the server with this rule's id: a rule -# opened just after a moment fired is evidence it belongs on that moment, and -# once that repeats across sessions the server answers with one line asking -# the reader to offer the operator the mount. Nothing is mounted by this. +# grimoire_moment.sh writes every call to `grimoire-moment/.acts`. The +# last three, from the last ninety seconds, go to the server with this rule's +# id: a rule the agent went and opened right after a moment fired is evidence +# it belongs on that moment, and once that repeats across sessions the server +# answers with one line putting the question to the reader. Nothing is +# mounted by this. +# +# An open of a rule a hook has just NAMED is not reported. A ranked arm or a +# moment line put it in front of the reader, and that explains the open; the +# moment beside it did not, so counting it would propose mounts on whatever +# moment happened to fire while search was surfacing the rule. The window is +# short for the same reason: a few minutes of agent work reaches many moments. command -v curl >/dev/null 2>&1 || exit 0 grimoire_config || exit 0 +case ",$(grimoire_rules_live "$state_dir/${safe_sid}.rules.ids")," in + *",${rule_id},"*) exit 0 ;; +esac acts_file="${TMPDIR:-/tmp}/grimoire-moment/${safe_sid}.acts" [ -f "$acts_file" ] || exit 0 now=$(date +%s 2>/dev/null) || exit 0 -acts=$(awk -F '\t' -v since=$((now - 180)) \ +acts=$(awk -F '\t' -v since=$((now - 90)) \ '$1 + 0 >= since { sub(/^[^\t]*\t/, ""); print }' "$acts_file" 2>/dev/null \ - | tail -n 30 | paste -sd ',' - 2>/dev/null) + | tail -n 3 | paste -sd ',' - 2>/dev/null) [ -n "$acts" ] || exit 0 sid_esc=$(printf '%s' "$session_id" | grimoire_json_escape) || exit 0 diff --git a/plugin/skills/using-grimoire/moments.md b/plugin/skills/using-grimoire/moments.md index 7f499590..e9c9bd6b 100644 --- a/plugin/skills/using-grimoire/moments.md +++ b/plugin/skills/using-grimoire/moments.md @@ -73,12 +73,16 @@ than a mount is fixed through its trigger instead. ## When a line proposes a mount -A rule that keeps being opened just after the same moment, across several -sessions, probably belongs on that moment. A line says so, naming the rule, -the moment and how often. It is a question for the operator, not a change you -make: offer it in one line, and record their answer with -`judge_rule_moments` — `confirm` mounts the rule, `reject` with their reason -stops the question being asked again. +A rule that sessions keep going and opening just after the same moment — +without a hook having shown it to them first — may belong on that moment. A +line says so, naming the rule, the moment, the action that reached it and how +often. That is co-occurrence, not a recommendation, so judge it before it +reaches the operator: read the rule's trigger against what the moment means +(`list_moments`). Offer it only when the rule governs that point in the work +whatever the work is about, as a question with your reason; a rule about a +subject (a library, a file, a style) is reached by meaning, and the line is +left alone. Record their answer with `judge_rule_moments` — `confirm` mounts +the rule, `reject` with their reason stops the question being asked again. The same tool answers proposals from a pass over the rules (`rules_to_mount`, `propose_rule_moments`, `rule_moment_proposals`): a pass diff --git a/src/grimoire/services/rule_moment_judgments.py b/src/grimoire/services/rule_moment_judgments.py index b461e10b..26f7d549 100644 --- a/src/grimoire/services/rule_moment_judgments.py +++ b/src/grimoire/services/rule_moment_judgments.py @@ -494,17 +494,50 @@ def _keep(row: RuleMomentJudgment, note: str, now: datetime) -> None: # ── The signal ─────────────────────────────────────────────────────────────── +# How many distinct actions a signal pair remembers as having reached its +# moment. Enough to show the operator what the opens followed. +_SIGNAL_ACTIONS_CAP = 5 + + +def _reached_phrase(evidence: dict) -> str: + reached = (evidence or {}).get("reached_by") or {} + if not isinstance(reached, dict) or not reached: + return "" + top = sorted(reached.items(), key=lambda kv: (-int(kv[1] or 0), kv[0]))[:3] + return " (reached by " + ", ".join(f"`{match}`" for match, _n in top) + ")" + + +def _signal_note(moment: str, evidence: dict) -> str: + """The `why` a signal proposal shows the operator: what the opens followed.""" + counts = lesson_rules.evidence_summary(evidence) + return ( + f"Opened just after `{moment}`{_reached_phrase(evidence)} in " + f"{counts['situations']} distinct sessions, each time without a hook having " + f"shown the rule first. Co-occurrence only: confirm it if the rule governs " + f"that point in the work whatever the work is about." + ) + + def _proposal_line(rule: Rule, moment: str, evidence: dict) -> str: + """The line an open past the bar returns to the session. + + It says what the evidence is, and makes offering it conditional on the + reader's own reading: a session relays an unqualified "offer this" as a + recommendation, and co-occurrence is not one. + """ counts = lesson_rules.evidence_summary(evidence) kind = rule.kind or "rule" return ( - f"> {kind.capitalize()} #{rule.id} \"{rule.title}\" has been opened just after " - f"`{moment}` in {counts['situations']} distinct sessions — it may belong on " - f"that moment, where it would arrive without being searched for. Offer the " - f"operator the mount in one line; on their yes, `judge_rule_moments(" - f"[{{\"rule_id\": {rule.id}, \"moment\": \"{moment}\", \"verdict\": \"confirm\", " - f"\"note\": \"why\"}}])` mounts it; on a no, `\"reject\"` with their why stops " - f"the asking." + f"> {kind.capitalize()} #{rule.id} \"{rule.title}\" was opened just after " + f"`{moment}`{_reached_phrase(evidence)} in {counts['situations']} distinct " + f"sessions, each time without a hook having shown it first. That is " + f"co-occurrence, not a recommendation. Read the {kind}'s trigger against what " + f"`{moment}` means (`list_moments`): only if the {kind} governs that point in " + f"the work whatever the work is about, ask the operator whether to mount it, " + f"with your reason; if it is about a subject, leave it. On their yes, " + f"`judge_rule_moments([{{\"rule_id\": {rule.id}, \"moment\": \"{moment}\", " + f"\"verdict\": \"confirm\", \"note\": \"why\"}}])` mounts it; on a no, " + f"`\"reject\"` with their why stops the asking." ) @@ -521,24 +554,39 @@ def signal_moments(moments) -> list[str]: async def co_occurred( user_id: int, rule_id: int, moments, *, situation: str, - project_id: int | None = None, + project_id: int | None = None, reached: dict | None = None, ) -> str: """Record that a rule was opened shortly after these moments fired, and return a proposal line once a (rule, moment) pair has earned one. `situation` is what makes two opens distinct — the session, so a rule opened after every push in one long session counts once. A pair already - mounted or judged gathers nothing. One proposal per open at most. Fails - open: this rides a hook, and bookkeeping must never break the read. + judged gathers nothing, and neither does a rule already mounted anywhere: + it arrives by lookup at its own moments, so an open of it is far likelier + to follow that delivery than to reveal a missing moment. `reached` maps a + moment to the action that reached it, kept as the proposal's evidence. + One proposal per open at most. Fails open: this rides a hook, and + bookkeeping must never break the read. """ try: - return await _co_occurred(user_id, rule_id, moments, situation, project_id) + return await _co_occurred(user_id, rule_id, moments, situation, project_id, + reached or {}) except Exception: logger.warning("rule/moment co-occurrence could not be recorded", exc_info=True) return "" -async def _co_occurred(user_id, rule_id, moments, situation, project_id) -> str: +def _add_reached(evidence: dict, match: str) -> dict: + """`evidence` with one more open counted against the action that reached + the moment, keeping at most _SIGNAL_ACTIONS_CAP distinct actions.""" + reached = dict(evidence.get("reached_by") or {}) + match = (match or "").strip()[:80] + if match and (match in reached or len(reached) < _SIGNAL_ACTIONS_CAP): + reached[match] = int(reached.get(match) or 0) + 1 + return {**evidence, "reached_by": reached} + + +async def _co_occurred(user_id, rule_id, moments, situation, project_id, reached) -> str: wanted = signal_moments(moments) key = lesson_rules.situation_key("s", situation) rid = _int(rule_id) @@ -551,11 +599,10 @@ async def _co_occurred(user_id, rule_id, moments, situation, project_id) -> str: rule = owned.get(rid) if rule is None: return "" - mounted = (await _mounts(session, [rid])).get(rid, set()) + if (await _mounts(session, [rid])).get(rid): + return "" existing = await _rows(session, [rid]) for moment in wanted: - if moment in mounted: - continue row = existing.get((rid, moment)) if row is not None and row.state != SUGGESTED: continue @@ -563,11 +610,13 @@ async def _co_occurred(user_id, rule_id, moments, situation, project_id) -> str: row = RuleMomentJudgment(rule_id=rid, moment=moment, state=SUGGESTED, source="signal") session.add(row) ev = lesson_rules.add_evidence(row.evidence, key, project_id, now) + ev = _add_reached(ev, reached.get(moment, "")) if proposal is None and lesson_rules.proposal_due(ev, now): ev["proposed_at"] = now.isoformat() ev["proposed_count"] = int(ev.get("proposed_count") or 0) + 1 proposal = (moment, ev) row.evidence = ev + row.note = _signal_note(moment, ev) try: await session.commit() except IntegrityError: @@ -599,6 +648,7 @@ async def opened_after( from grimoire.services.moment_actions import moments_for reached: list[str] = [] + reached_by: dict[str, str] = {} for act in list(acts or [])[:_ACTS_CAP]: if not isinstance(act, dict): continue @@ -615,11 +665,13 @@ async def opened_after( name = hit.get("moment") if isinstance(hit, dict) else None if name and name not in reached: reached.append(name) + reached_by[name] = str(hit.get("match") or tool) moments = signal_moments(reached) context = "" if moments and (session_id or "").strip(): context = await co_occurred( user_id, rule_id, moments, situation=session_id.strip(), project_id=project_id, + reached=reached_by, ) return {"moments": moments, "context": context} diff --git a/tests/test_integration_rule_moment_judgments.py b/tests/test_integration_rule_moment_judgments.py index f3cebb14..739ba798 100644 --- a/tests/test_integration_rule_moment_judgments.py +++ b/tests/test_integration_rule_moment_judgments.py @@ -195,7 +195,23 @@ async def test_the_open_resolves_acts_through_the_installs_mappings(world): out = await judgments_svc.opened_after(uid, done.id, acts, session_id="s1") assert "work.deliver" in out["moments"] assert "work.run" not in out["moments"] - assert (await _row(done.id, "work.deliver")).source == "signal" + row = await _row(done.id, "work.deliver") + assert row.source == "signal" + # The action that reached the moment is kept, and shown as the why. + assert row.evidence["reached_by"] == {"git push": 1} + assert row.note.startswith("Opened just after `work.deliver` (reached by `git push`)") + + +async def test_a_rule_already_mounted_gathers_no_signal(world): + """A mounted rule arrives by lookup at its own moments, so an open of it + is no evidence it is missing another.""" + uid, done = world["uid"], world["done"] + await rulebooks_svc.set_rule_moments(done.id, uid, ["work.finish"]) + for situation in ("a", "b", "c", "d"): + assert await judgments_svc.co_occurred( + uid, done.id, ["work.verify"], situation=situation, + ) == "" + assert await _row(done.id, "work.verify") is None # ── The misfire ────────────────────────────────────────────────────────────── diff --git a/tests/test_rule_moment_judgments.py b/tests/test_rule_moment_judgments.py index 8e7475ae..a4973868 100644 --- a/tests/test_rule_moment_judgments.py +++ b/tests/test_rule_moment_judgments.py @@ -61,6 +61,29 @@ def test_the_proposal_line_names_the_tool_that_answers_it(): assert "judge_rule_moments" in line and '"reject"' in line +def test_the_proposal_line_is_a_question_to_check_not_advice_to_relay(): + rule = SimpleNamespace(id=11, title="Definition of done", kind="rule") + evidence = {"situations": ["a", "b", "c"], + "reached_by": {"git push": 3, "git push --tags": 1}} + line = svc._proposal_line(rule, "work.deliver", evidence) + assert "co-occurrence, not a recommendation" in line + assert "`list_moments`" in line and "if it is about a subject, leave it" in line + assert "(reached by `git push`, `git push --tags`)" in line + note = svc._signal_note("work.deliver", evidence) + assert note.startswith("Opened just after `work.deliver` (reached by `git push`") + assert "3 distinct sessions" in note + + +def test_the_actions_a_signal_remembers_are_bounded(): + ev: dict = {} + for i in range(svc._SIGNAL_ACTIONS_CAP + 3): + ev = svc._add_reached(ev, f"cmd {i}") + ev = svc._add_reached(ev, "cmd 0") + assert len(ev["reached_by"]) == svc._SIGNAL_ACTIONS_CAP + assert ev["reached_by"]["cmd 0"] == 2 + assert svc._add_reached({}, "")["reached_by"] == {} + + # ── The misfire ────────────────────────────────────────────────────────────── diff --git a/tests/test_rule_moment_signal_hooks.py b/tests/test_rule_moment_signal_hooks.py index a14c7b20..c0371a4b 100644 --- a/tests/test_rule_moment_signal_hooks.py +++ b/tests/test_rule_moment_signal_hooks.py @@ -133,6 +133,49 @@ def test_an_open_sends_the_window_and_prints_the_answer(sandbox): assert (tmp / "grimoire-priorart" / "s1.opened.ids").read_text().startswith("11\t") +def test_the_window_is_the_last_three_acts(sandbox): + tmp, env = sandbox + ledger = tmp / "grimoire-moment" + ledger.mkdir() + now = int(time.time()) + acts = [{"tool_name": "Bash", "tool_input": {"command": f"step {i}"}} for i in range(5)] + (ledger / "s1.acts").write_text( + "".join(f"{now - 10 + i}\t{json.dumps(a)}\n" for i, a in enumerate(acts)) + ) + _run("grimoire_record_opened.sh", + {"session_id": "s1", "tool_name": "mcp__grimoire__get_rule", + "tool_input": {"rule_id": 11}}, env) + body = json.loads((tmp / "curl.body").read_text()) + assert body["acts"] == acts[-3:] + + +def test_an_open_of_a_rule_a_hook_just_named_is_not_reported(sandbox): + """A ranked arm or a moment line explains that open; the moment beside it + does not, so it is no evidence for a mount.""" + tmp, env = sandbox + ledger = tmp / "grimoire-moment" + ledger.mkdir() + now = int(time.time()) + (ledger / "s1.acts").write_text( + f"{now - 5}\t{json.dumps({'tool_name': 'Bash', 'tool_input': {'command': 'git push'}})}\n" + ) + named = tmp / "grimoire-priorart" + named.mkdir() + (named / "s1.rules.ids").write_text(f"7\t{now - 30}\n11\t{now - 20}\n") + out = _run("grimoire_record_opened.sh", + {"session_id": "s1", "tool_name": "mcp__grimoire__get_rule", + "tool_input": {"rule_id": 11}}, env) + assert out.stdout == "" + assert not (tmp / "curl.log").exists() + # The open itself is still recorded. + assert (named / "s1.opened.ids").read_text().startswith("11\t") + # A rule no hook named is still reported. + _run("grimoire_record_opened.sh", + {"session_id": "s1", "tool_name": "mcp__grimoire__get_rule", + "tool_input": {"rule_id": 1}}, env) + assert json.loads((tmp / "curl.body").read_text())["rule_id"] == 1 + + def test_an_open_with_nothing_in_the_window_sends_nothing(sandbox): tmp, env = sandbox ledger = tmp / "grimoire-moment"