feat(rulings): a command or edit touching an area's files shows its rulings, once per session (milestone 444 step 4, #4757)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m6s
CI & Build / Python tests (push) Failing after 1m22s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m6s
CI & Build / Python tests (push) Failing after 1m22s
CI & Build / Build & push image (push) Skipped
A System's rulings (the Rulings section of its description) now reach the work by path, not by similarity. Both PreToolUse arms resolve the files a command or edit names to the Systems whose path_patterns cover them, and the first touch in a session shows each area's rulings in one line; a repeat is a one-line reference. A lookup, so no floor, no budget, no retrieval_logs row. - services/system_rulings: parse_rulings, command_paths (reads and writes, relative to the repo root from any cwd; flags, URLs, globs skipped), rulings_for_paths - /tool-rules takes root, cwd and seen_ruling_systems; /prior-art takes seen_ruling_systems; both return ruling_system_ids - hooks share <sid>.rulings.ids (cleared on compaction by the ledger naming convention); the Bash hook sends the repo root and cwd - system_usage_events (migration 0114): surfacings by source, pulls from get_system; carried by backup (v20) through the system map - writing-records: rulings also arrive when the area's files are touched Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -116,6 +116,21 @@ def _no_system_labels():
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_rulings_arm():
|
||||
"""Stub the rulings arm both PreToolUse builders run (milestone 444).
|
||||
|
||||
Autouse for _no_system_labels' reason: every write-path and tool-rule
|
||||
test with a project in scope reaches it, and its first act is reading the
|
||||
project's Systems from Postgres. Stubbed to "no area's files touched".
|
||||
tests/test_system_rulings.py binds the real function at import time,
|
||||
before this patch runs.
|
||||
"""
|
||||
with patch("scribe.services.system_rulings.rulings_for_paths",
|
||||
AsyncMock(return_value={"lines": [], "system_ids": []})):
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_task_log_arm():
|
||||
"""Stub the task-log read arm that get_task / list_tasks / get_milestone
|
||||
|
||||
@@ -27,7 +27,7 @@ def test_backup_version_is_current():
|
||||
|
||||
(Named for the number it asserted until v10, which is exactly the drift a
|
||||
name-carrying-a-value invites; it now says what it checks.)"""
|
||||
assert backup.BACKUP_VERSION == 19
|
||||
assert backup.BACKUP_VERSION == 20
|
||||
|
||||
|
||||
def _exportable_note(**over):
|
||||
@@ -139,6 +139,7 @@ def _column_guard_targets():
|
||||
from scribe.models.note_supersession import NoteSupersession
|
||||
from scribe.models.note_usage import NoteUsageEvent
|
||||
from scribe.models.rule_usage import RuleUsageEvent
|
||||
from scribe.models.system_usage import SystemUsageEvent
|
||||
from scribe.models.retrieval_tuning import RetrievalTuningEvent
|
||||
from scribe.models.note_version import NoteVersion
|
||||
from scribe.models.rule_version import RuleVersion
|
||||
@@ -172,6 +173,7 @@ def _column_guard_targets():
|
||||
"lesson_no_rule": (LessonNoRule, backup._lesson_no_rule_rows),
|
||||
"note_usage_events": (NoteUsageEvent, backup._usage_event_rows),
|
||||
"rule_usage_events": (RuleUsageEvent, backup._rule_usage_event_rows),
|
||||
"system_usage_events": (SystemUsageEvent, backup._system_usage_event_rows),
|
||||
"retrieval_tuning_events": (
|
||||
RetrievalTuningEvent, backup._retrieval_tuning_event_rows,
|
||||
),
|
||||
@@ -290,6 +292,7 @@ def _import_guard_targets():
|
||||
"lesson_no_rule": backup._build_lesson_no_rule,
|
||||
"note_usage_events": backup._build_usage_event,
|
||||
"rule_usage_events": backup._build_rule_usage_event,
|
||||
"system_usage_events": backup._build_system_usage_event,
|
||||
"retrieval_tuning_events": backup._build_retrieval_tuning_event,
|
||||
"design_systems": backup._build_design_system,
|
||||
"design_tokens": backup._build_design_token,
|
||||
@@ -543,7 +546,9 @@ async def test_export_full_backup_contains_every_declared_section():
|
||||
# v18: which rule each lesson is an instance of.
|
||||
"lesson_rule_links",
|
||||
# v19: the lessons judged to fall under no rule.
|
||||
"lesson_no_rule"):
|
||||
"lesson_no_rule",
|
||||
# v20: whether an area's rulings were read once shown.
|
||||
"system_usage_events"):
|
||||
assert key in out, f"missing export section: {key}"
|
||||
assert out[key] == []
|
||||
|
||||
|
||||
@@ -0,0 +1,241 @@
|
||||
"""An area's rulings reach the work that touches its files (milestone 444, #4757).
|
||||
|
||||
These pin:
|
||||
|
||||
- WHAT A RULING IS, as read: a bullet under the last `Rulings` heading of a
|
||||
System's description, wrapped lines joined, the section ending at the first
|
||||
line that is neither.
|
||||
- WHICH PATHS A COMMAND NAMES: reads and writes alike, relative to the repo
|
||||
root whatever the command's cwd; flags, URLs and globs are not paths, and a
|
||||
path outside the repo is nobody's area.
|
||||
- THE ARM: a lookup, not a search. No rulings or no patterns says nothing; a
|
||||
path in two areas hears both; a repeat is one short reference line and is
|
||||
neither counted as a surfacing nor handed back as newly shown.
|
||||
- THE WIRING: both PreToolUse builders carry it, ahead of what they rank.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import plugin_context as pc
|
||||
from scribe.services import system_rulings as sr
|
||||
# Bound at import, before conftest's autouse stub replaces the module attribute.
|
||||
from scribe.services.system_rulings import rulings_for_paths as real_rulings_for_paths
|
||||
from tests.helpers import writepath_cfg
|
||||
|
||||
DOWNLOADS = """Fetching and keeping downloads healthy: stalls, retries, the blocklist.
|
||||
|
||||
Rulings
|
||||
- Failed work is retried until it succeeds; no attempt limit. (Operator, 2026-09-19, #1234)
|
||||
- A blocklisted release falls off the list after a while
|
||||
and may be tried again. (Operator, 2026-09-20, #1240)
|
||||
"""
|
||||
|
||||
|
||||
def _system(sid, name, description, patterns):
|
||||
return SimpleNamespace(id=sid, name=name, description=description, path_patterns=patterns)
|
||||
|
||||
|
||||
# ── reading the section ──────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_rulings_are_the_bullets_under_the_heading_with_wrapped_lines_joined():
|
||||
assert sr.parse_rulings(DOWNLOADS) == [
|
||||
"Failed work is retried until it succeeds; no attempt limit. (Operator, 2026-09-19, #1234)",
|
||||
"A blocklisted release falls off the list after a while and may be tried again. "
|
||||
"(Operator, 2026-09-20, #1240)",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("heading", ["Rulings", "## Rulings", "**Rulings**", "Rulings:", "### rulings"])
|
||||
def test_the_heading_may_be_written_several_ways(heading):
|
||||
assert sr.parse_rulings(f"Charter.\n\n{heading}\n- One. (Operator)\n") == ["One. (Operator)"]
|
||||
|
||||
|
||||
def test_no_section_and_an_empty_description_have_no_rulings():
|
||||
assert sr.parse_rulings("A charter that mentions rulings in passing.") == []
|
||||
assert sr.parse_rulings("") == []
|
||||
assert sr.parse_rulings(None) == []
|
||||
|
||||
|
||||
def test_the_section_ends_where_a_paragraph_starts():
|
||||
text = "Rulings\n- Kept. (Operator)\n\nNotes written after the section.\n- Not a ruling.\n"
|
||||
assert sr.parse_rulings(text) == ["Kept. (Operator)"]
|
||||
|
||||
|
||||
# ── the paths a command names ────────────────────────────────────────────
|
||||
|
||||
|
||||
ROOT = "/home/me/repo"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("command,cwd,expected", [
|
||||
("sed -n 1,80p src/app/downloads.py", ROOT, ["src/app/downloads.py"]),
|
||||
# Absolute inside the root, and outside it.
|
||||
(f"cat {ROOT}/src/app/x.py /etc/hosts", ROOT, ["src/app/x.py"]),
|
||||
# A command run from a subdirectory names paths relative to it.
|
||||
("grep -n retry downloads.py", f"{ROOT}/src/app", ["src/app/downloads.py"]),
|
||||
("cat ../../../elsewhere/x.py", f"{ROOT}/src", []),
|
||||
# Flags, URLs and globs are not paths; a `--flag=path` and a `path:line` are.
|
||||
("pytest -q --rootdir=tests/unit tests/test_x.py::test_y", ROOT,
|
||||
["tests/unit", "tests/test_x.py"]),
|
||||
("curl https://example.com/a/b.json", ROOT, []),
|
||||
("ls src/*.py", ROOT, []),
|
||||
("vim src/app/x.py:120", ROOT, ["src/app/x.py"]),
|
||||
# Repeats collapse; a bare word with no slash or extension is not a path.
|
||||
("git add src/a.py src/a.py && git commit -m done", ROOT, ["src/a.py"]),
|
||||
])
|
||||
def test_command_paths(command, cwd, expected):
|
||||
assert sr.command_paths(command, root=ROOT, cwd=cwd) == expected
|
||||
|
||||
|
||||
def test_without_a_root_relative_paths_are_taken_as_given_and_absolute_ones_dropped():
|
||||
assert sr.command_paths("cat src/a.py /abs/b.py") == ["src/a.py"]
|
||||
|
||||
|
||||
def test_an_unbalanced_quote_still_yields_paths():
|
||||
assert sr.command_paths("echo 'oops src/a.py") == ["src/a.py"]
|
||||
|
||||
|
||||
# ── the arm ──────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
async def _arm(systems, paths, seen=None):
|
||||
lister = AsyncMock(return_value=systems)
|
||||
recorder = MagicMock()
|
||||
with patch.object(sr.systems_svc, "list_systems", lister), \
|
||||
patch.object(sr, "record_system_surfaced", recorder):
|
||||
out = await real_rulings_for_paths(1, 2, paths, seen=seen, source="rulings_test")
|
||||
return out, recorder
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_touched_area_with_rulings_shows_them_once_in_full_and_records_it():
|
||||
downloads = _system(104, "Download lifecycle", DOWNLOADS, ["src/app/downloads"])
|
||||
out, recorder = await _arm([downloads], ["src/app/downloads/retry.py"])
|
||||
[line] = out["lines"]
|
||||
assert line.startswith("> Rulings for `src/app/downloads/retry.py`")
|
||||
assert "Download lifecycle (System 104)" in line
|
||||
assert "(1) Failed work is retried until it succeeds" in line
|
||||
assert "(2) A blocklisted release falls off" in line
|
||||
assert out["system_ids"] == [104]
|
||||
recorder.assert_called_once()
|
||||
assert recorder.call_args.kwargs["system_ids"] == [104]
|
||||
assert recorder.call_args.kwargs["source"] == "rulings_test"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_area_without_rulings_says_nothing():
|
||||
plain = _system(5, "Billing", "What billing is for, and nothing decided.", ["src/billing"])
|
||||
out, recorder = await _arm([plain], ["src/billing/invoice.py"])
|
||||
assert out == {"lines": [], "system_ids": []}
|
||||
recorder.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_area_without_patterns_claims_no_files():
|
||||
unnamed = _system(104, "Download lifecycle", DOWNLOADS, [])
|
||||
out, _ = await _arm([unnamed], ["src/app/downloads/retry.py"])
|
||||
assert out["lines"] == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_path_in_two_areas_hears_both():
|
||||
downloads = _system(104, "Download lifecycle", DOWNLOADS, ["src/app/downloads"])
|
||||
storage = _system(7, "Storage", "Where files go.\n\nRulings\n- Never delete a user file. (Operator)\n",
|
||||
["src/app/**/*.py"])
|
||||
out, _ = await _arm([downloads, storage], ["src/app/downloads/retry.py"])
|
||||
assert len(out["lines"]) == 2
|
||||
assert out["system_ids"] == [104, 7]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_repeat_is_a_reference_and_is_not_counted_again():
|
||||
downloads = _system(104, "Download lifecycle", DOWNLOADS, ["src/app/downloads"])
|
||||
out, recorder = await _arm([downloads], ["src/app/downloads/retry.py"], seen=[104])
|
||||
[line] = out["lines"]
|
||||
assert "shown earlier this session" in line
|
||||
assert "Failed work is retried" not in line
|
||||
assert out["system_ids"] == []
|
||||
recorder.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_project_or_no_paths_reads_nothing():
|
||||
lister = AsyncMock(return_value=[])
|
||||
with patch.object(sr.systems_svc, "list_systems", lister):
|
||||
assert await real_rulings_for_paths(1, 0, ["src/a.py"], source="t") == {"lines": [], "system_ids": []}
|
||||
assert await real_rulings_for_paths(1, 2, [], source="t") == {"lines": [], "system_ids": []}
|
||||
lister.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_failing_lookup_fails_open():
|
||||
with patch.object(sr.systems_svc, "list_systems", AsyncMock(side_effect=RuntimeError("db"))):
|
||||
out = await real_rulings_for_paths(1, 2, ["src/a.py"], source="t")
|
||||
assert out == {"lines": [], "system_ids": []}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_long_list_is_bounded_per_area():
|
||||
many = "Rulings\n" + "".join(f"- Ruling {i}. (Operator)\n" for i in range(12))
|
||||
area = _system(9, "Busy", many, ["src"])
|
||||
out, _ = await _arm([area], ["src/x.py"])
|
||||
assert "(8) Ruling 7." in out["lines"][0]
|
||||
assert "Ruling 8." not in out["lines"][0]
|
||||
assert "+4 more in `get_system(9)`" in out["lines"][0]
|
||||
|
||||
|
||||
# ── the wiring ───────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
RULING_LINE = "> Rulings for `src/a.py` — the operator's decisions about A (System 3): (1) x"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_tool_arm_leads_with_rulings_and_hands_back_what_it_showed():
|
||||
rulings = AsyncMock(return_value={"lines": [RULING_LINE], "system_ids": [3]})
|
||||
with patch.object(pc, "_tool_rule_hint", AsyncMock(return_value={
|
||||
"context": "> a rule line", "rule_ids": [8], "checkpoint": {}})), \
|
||||
patch.object(pc, "get_writepath_config", AsyncMock(return_value=writepath_cfg())), \
|
||||
patch.object(pc.system_rulings_svc, "rulings_for_paths", rulings):
|
||||
out = await pc.build_tool_rule_hint(
|
||||
1, "Bash", f"cat {ROOT}/src/a.py", project_id=2,
|
||||
root=ROOT, cwd=ROOT, seen_ruling_systems=[5],
|
||||
)
|
||||
assert out["context"].splitlines() == [RULING_LINE, "> a rule line"]
|
||||
assert out["ruling_system_ids"] == [3]
|
||||
args, kwargs = rulings.call_args
|
||||
assert args[2] == ["src/a.py"]
|
||||
assert kwargs["seen"] == [5] and kwargs["source"] == "rulings_pre_tool"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_tool_arm_adds_no_key_when_no_ruling_was_shown():
|
||||
with patch.object(pc, "_tool_rule_hint", AsyncMock(return_value={
|
||||
"context": "", "rule_ids": [], "checkpoint": {}})), \
|
||||
patch.object(pc, "get_writepath_config", AsyncMock(return_value=writepath_cfg())):
|
||||
out = await pc.build_tool_rule_hint(1, "Bash", "ls", project_id=2)
|
||||
assert "ruling_system_ids" not in out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_write_with_no_prior_art_still_carries_its_areas_rulings():
|
||||
rulings = AsyncMock(return_value={"lines": [RULING_LINE], "system_ids": [3]})
|
||||
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=writepath_cfg())), \
|
||||
patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
|
||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||
patch.object(pc.projects_svc, "get_project", AsyncMock(return_value=None)), \
|
||||
patch.object(pc.system_rulings_svc, "rulings_for_paths", rulings):
|
||||
out = await pc.build_write_path_hint(
|
||||
1, "src/a.py", code="x = 1", project_id=2, seen_ruling_systems=[9],
|
||||
)
|
||||
assert out["context"] == RULING_LINE
|
||||
assert out["ruling_system_ids"] == [3]
|
||||
args, kwargs = rulings.call_args
|
||||
assert args[2] == ["src/a.py"]
|
||||
assert kwargs["seen"] == [9] and kwargs["source"] == "rulings_write_path"
|
||||
Reference in New Issue
Block a user