bvandeusen
1d65e98ac2
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / integration (push) Successful in 43s
CI & Build / Python tests (push) Successful in 1m16s
CI & Build / Build & push image (push) Successful in 28s
The step that decides whether steps 1-4 were worth building. `verify_with` is a free-text field on the highest-volume record kind in the product; described only as "how to verify this note" it gets filled in on every note within a week, and at that point the sweep returns the whole corpus and means nothing. The signal is not "has a check" — it is "has a check AND almost nothing else does". Rule 119 puts this in the app's own instruction surfaces, never in a Scribe rule. So: - the using-scribe skill gains the reflex, next to "state updates in place; chronicles don't" — its other half, since supersession only fires once somebody has read a note and disagreed, which is the case where it was already believed; - update_note's docstring now states the DEFAULT rather than only deferring to create_note for the test. Found by the new guard on its first run; - plugin 0.1.48. THE _INSTRUCTIONS BUDGET, decided rather than skipped. The payload is ~1980 of the client's ~2048-char cap, so everything in it competes for the last ~68 characters. The operator declined a line for this milestone: the map's own closing sentence says each tool's description carries the full contract, and the sweep is a curation act rather than a session-start reflex like enter_project or list_always_on_rules. That reasoning is now a comment beside the constant, with the accepted cost named — an agent that never opens create_note's docstring never learns the field exists — so the question is not re-litigated blind. The guard pins STRUCTURE, never wording, for the disambiguator's reason (a test that punishes rewriting is a test that gets deleted): each write surface must still draw the norm-vs-constraint distinction, say the empty case is normal, and name where NOT to reach for it — plus that the skill carries the one-question form, because the docstrings only reach a caller who already opened the tool. `_doc` moves to tests/helpers as `tool_doc`; it had been written twice.
82 lines
3.5 KiB
Python
82 lines
3.5 KiB
Python
"""Every create_* tool says what it is NOT for (#3123).
|
|
|
|
WHY THIS EXISTS
|
|
|
|
Scribe's record kinds are reached for interchangeably — a note written where
|
|
a task was owed, a rule written where a snippet belonged — and the moment of
|
|
choice is the only moment a correction is cheap. The tool docstring IS the
|
|
agent-facing contract (rule 119 puts product guidance there and nowhere
|
|
else), so a docstring that only documents parameters answers "how do I call
|
|
this" while leaving "should I be calling this at all" unasked.
|
|
|
|
The gap was lopsided before this: `create_rule` and `start_planning` both
|
|
carried a real disambiguator, and `create_note` — far and away the
|
|
highest-volume surface — carried none. Guidance sat in the rarest tool and
|
|
was missing from the most common one.
|
|
|
|
WHAT THIS PINS, AND WHAT IT DOES NOT
|
|
|
|
It asserts STRUCTURE, never wording: each create surface must name at least
|
|
two sibling surfaces, so a caller who reached for the wrong one is told
|
|
where the right one is. Prose stays free to be rewritten — pinning phrasing
|
|
would make every improvement a test failure, and a test that punishes
|
|
editing is a test that gets deleted.
|
|
|
|
It cannot tell whether the guidance is any GOOD. It only catches the
|
|
regression that actually happens: a docstring rewritten down to its
|
|
parameters, with the "is this even the right tool" paragraph quietly gone.
|
|
"""
|
|
import re
|
|
|
|
import pytest
|
|
from tests.helpers import tool_doc as _doc
|
|
|
|
# The create surfaces and where they live. `start_planning` is here because
|
|
# it is a create in everything but name — it is how a plan comes into being.
|
|
_SURFACES = [
|
|
("scribe.mcp.tools.notes", "create_note"),
|
|
("scribe.mcp.tools.tasks", "create_task"),
|
|
("scribe.mcp.tools.tasks", "start_planning"),
|
|
("scribe.mcp.tools.snippets", "create_snippet"),
|
|
("scribe.mcp.tools.processes", "create_process"),
|
|
("scribe.mcp.tools.rulebooks", "create_rule"),
|
|
("scribe.mcp.tools.rulebooks", "create_project_rule"),
|
|
]
|
|
|
|
# What a caller could have wanted instead. A surface naming two of these has
|
|
# pointed somewhere; naming none has left them where they were.
|
|
_ALTERNATIVES = [
|
|
"create_note", "create_task", "create_rule", "create_project_rule",
|
|
"create_snippet", "create_process", "start_planning", "design system",
|
|
]
|
|
|
|
|
|
@pytest.mark.parametrize(("module", "name"), _SURFACES)
|
|
def test_a_create_surface_names_at_least_two_alternatives(module, name):
|
|
"""Reaching for the wrong tool must still put the right one in view."""
|
|
doc = _doc(module, name)
|
|
named = {
|
|
alt for alt in _ALTERNATIVES
|
|
if alt != name and re.search(re.escape(alt), doc, re.IGNORECASE)
|
|
}
|
|
assert len(named) >= 2, (
|
|
f"{name}'s docstring names {sorted(named) or 'no'} alternative "
|
|
f"surface(s). A caller who reached for it by mistake gets no "
|
|
f"correction. Say what belongs elsewhere and why — see create_rule "
|
|
f"or create_note for the shape."
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(("module", "name"), _SURFACES)
|
|
def test_a_create_surface_still_documents_its_arguments(module, name):
|
|
"""The guard above must not be satisfiable by deleting the parameter docs.
|
|
|
|
Both halves matter and they pull in opposite directions: a docstring can
|
|
be made to pass the disambiguator check by becoming an essay about the
|
|
other tools, which would be a worse contract than the one being fixed.
|
|
"""
|
|
assert "Args:" in _doc(module, name), (
|
|
f"{name}'s docstring lost its Args: block — the parameter contract is "
|
|
f"what a caller reads to make the call at all."
|
|
)
|