Merge pull request 'No heading-only chunks, agent-message skip, refusals reach the agent (#4784, #4785, #4794)' (#198) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m14s
CI & Build / Python tests (push) Successful in 2m2s
CI & Build / Build & push image (push) Successful in 18s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m14s
CI & Build / Python tests (push) Successful in 2m2s
CI & Build / Build & push image (push) Successful in 18s
This commit was merged in pull request #198.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "scribe",
|
||||
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
||||
"version": "2026.10.03.1934",
|
||||
"version": "2026.10.04.0125",
|
||||
"author": {
|
||||
"name": "Bryan Van Deusen"
|
||||
},
|
||||
|
||||
@@ -79,6 +79,7 @@ scribe_skip_prompt() {
|
||||
_scribe_prompt=${1#"${1%%[![:space:]]*}"}
|
||||
case "$_scribe_prompt" in
|
||||
"<task-notification>"*|\
|
||||
"<agent-message>"*|"<agent-message "*|\
|
||||
"<command-name>"*|"<command-message>"*|"<command-args>"*|\
|
||||
"<local-command-stdout>"*|"<local-command-stderr>"*|"<local-command-caveat>"*)
|
||||
return 0 ;;
|
||||
|
||||
@@ -4,7 +4,7 @@ from __future__ import annotations
|
||||
import difflib
|
||||
|
||||
from mcp.server.mcpserver import MCPServer
|
||||
from mcp.server.mcpserver.exceptions import ToolError
|
||||
from mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError
|
||||
from mcp.server.transport_security import TransportSecuritySettings
|
||||
from quart import Quart
|
||||
|
||||
@@ -282,9 +282,10 @@ class StrictArgsMCPServer(MCPServer):
|
||||
declared cannot have been meant to be dropped.
|
||||
"""
|
||||
|
||||
# ToolError, not ValueError: the SDK returns either one's text to the
|
||||
# caller as an error result, but logs anything that is not a ToolError as
|
||||
# an unexpected crash. A misnamed argument is the caller's mistake.
|
||||
# ToolError, not ValueError: since SDK 2.x only a ToolError's text reaches
|
||||
# the caller. Anything else is wrapped as UnexpectedToolError("Error
|
||||
# executing tool X") and its message stays on the server. A misnamed
|
||||
# argument is the caller's mistake, and the caller needs to read why.
|
||||
async def call_tool(self, name, arguments, context=None):
|
||||
try:
|
||||
tool = self._tool_manager.get_tool(name)
|
||||
@@ -304,7 +305,17 @@ class StrictArgsMCPServer(MCPServer):
|
||||
f"It accepts: {', '.join(sorted(declared))}. Nothing was "
|
||||
"created or changed — retry with the declared names."
|
||||
)
|
||||
return await super().call_tool(name, arguments, context)
|
||||
try:
|
||||
return await super().call_tool(name, arguments, context)
|
||||
except UnexpectedToolError as exc:
|
||||
# ValueError is how every Scribe tool refuses (#4794): what was
|
||||
# refused, why, and what to do instead, written for the agent.
|
||||
# SDK 2.x masks it like a crash, so the agent got a failure with
|
||||
# no reason and could only retry blind. Other exceptions ARE
|
||||
# crashes; their text can carry internals and stays masked.
|
||||
if isinstance(exc.__cause__, ValueError):
|
||||
raise ToolError(f"{exc}: {exc.__cause__}") from exc.__cause__
|
||||
raise
|
||||
|
||||
|
||||
def build_mcp_server() -> MCPServer:
|
||||
|
||||
@@ -333,7 +333,10 @@ def document_title(
|
||||
# narrower wording would have read as "nothing to re-embed".
|
||||
#
|
||||
# 1 → 2: work logs joined the task document.
|
||||
CHUNKER_VERSION = 2
|
||||
# 2 → 3: no chunk is a heading alone (#4784). A content-free chunk embeds
|
||||
# near the centre of the space and outranked real records on any
|
||||
# vague query — "## Work log — 2026-08-21" took rank 1.
|
||||
CHUNKER_VERSION = 3
|
||||
|
||||
|
||||
# The public name of the space every score lives in, and the two facts that
|
||||
@@ -404,9 +407,45 @@ async def rows_off_the_live_model(table) -> int:
|
||||
# the exact data loss this exists to end.
|
||||
_CHUNK_CHAR_BUDGET = 1400
|
||||
|
||||
# The least a chunk may say beyond its headings (#4784). Below this a chunk is
|
||||
# a heading, a date or a one-line lead-in, and it embeds as text about nothing
|
||||
# in particular — which is close to everything, so it outranks real records on
|
||||
# any vague query. Such a piece is joined to its neighbour instead of being
|
||||
# given a vector of its own.
|
||||
_MIN_CHUNK_CONTENT = 160
|
||||
|
||||
_HEADING_RE = None # compiled lazily below to keep re import local
|
||||
|
||||
|
||||
def _heading_re():
|
||||
import re
|
||||
global _HEADING_RE
|
||||
if _HEADING_RE is None:
|
||||
_HEADING_RE = re.compile(r"^#{1,6}\s")
|
||||
return _HEADING_RE
|
||||
|
||||
|
||||
def _is_thin(text: str) -> bool:
|
||||
"""Says too little to be a chunk on its own: under `_MIN_CHUNK_CONTENT`
|
||||
once its heading lines are set aside."""
|
||||
rx = _heading_re()
|
||||
said = "\n".join(ln for ln in text.splitlines() if not rx.match(ln)).strip()
|
||||
return len(said) < _MIN_CHUNK_CONTENT
|
||||
|
||||
|
||||
def _last_heading(text: str, default: str = "") -> str:
|
||||
"""The last heading line outside a code fence, or `default`."""
|
||||
rx = _heading_re()
|
||||
in_fence = False
|
||||
found = default
|
||||
for line in text.splitlines():
|
||||
if line.lstrip().startswith(("```", "~~~")):
|
||||
in_fence = not in_fence
|
||||
elif not in_fence and rx.match(line):
|
||||
found = line
|
||||
return found
|
||||
|
||||
|
||||
def _split_sections(body: str) -> list[str]:
|
||||
"""Split a markdown body at heading lines, fence-aware.
|
||||
|
||||
@@ -415,17 +454,13 @@ def _split_sections(body: str) -> list[str]:
|
||||
code fences do not split — a commented `# step` in a recorded shell snippet
|
||||
is content, not structure.
|
||||
"""
|
||||
import re
|
||||
global _HEADING_RE
|
||||
if _HEADING_RE is None:
|
||||
_HEADING_RE = re.compile(r"^#{1,6}\s")
|
||||
|
||||
rx = _heading_re()
|
||||
sections: list[list[str]] = [[]]
|
||||
in_fence = False
|
||||
for line in body.splitlines():
|
||||
if line.lstrip().startswith(("```", "~~~")):
|
||||
in_fence = not in_fence
|
||||
if not in_fence and _HEADING_RE.match(line) and sections[-1]:
|
||||
if not in_fence and rx.match(line) and sections[-1]:
|
||||
sections.append([line])
|
||||
else:
|
||||
sections[-1].append(line)
|
||||
@@ -436,17 +471,27 @@ def _split_paragraphs(section: str, budget: int) -> list[str]:
|
||||
"""Break one oversize section into budget-sized pieces at paragraph
|
||||
boundaries, hard-splitting only a single paragraph that alone exceeds the
|
||||
budget (a monster table or code block — split at line boundaries so no
|
||||
content is dropped, which is the entire point of this module)."""
|
||||
content is dropped, which is the entire point of this module).
|
||||
|
||||
No piece is thin (#4784). A heading, or a lead-in line, that would be left
|
||||
on its own is carried into the paragraph after it; a thin tail joins the
|
||||
piece before it."""
|
||||
pieces: list[str] = []
|
||||
current = ""
|
||||
# A hard cut never lands in the first quarter of the budget: the newline
|
||||
# it would find there is the one closing a heading or a lead-in, and
|
||||
# cutting at it is how a heading became a chunk of its own.
|
||||
lo = budget // 4
|
||||
for para in section.split("\n\n"):
|
||||
if current and _is_thin(current):
|
||||
para, current = f"{current}\n\n{para}", ""
|
||||
while len(para) > budget:
|
||||
# Hard split: prefer the last newline inside the budget so lines
|
||||
# stay whole, then the last space so words do; a clean char cut is
|
||||
# the final resort for one enormous unbroken token.
|
||||
cut = para.rfind("\n", 0, budget)
|
||||
cut = para.rfind("\n", lo, budget)
|
||||
if cut <= 0:
|
||||
cut = para.rfind(" ", 0, budget)
|
||||
cut = para.rfind(" ", lo, budget)
|
||||
if cut <= 0:
|
||||
cut = budget
|
||||
head, para = para[:cut], para[cut:].lstrip("\n ")
|
||||
@@ -462,6 +507,19 @@ def _split_paragraphs(section: str, budget: int) -> list[str]:
|
||||
current = para
|
||||
else:
|
||||
current = candidate
|
||||
if current and pieces and _is_thin(current):
|
||||
# A thin tail joins the piece before it. If that piece is full, its
|
||||
# last paragraph moves down to keep the tail company instead.
|
||||
last = pieces.pop()
|
||||
if len(last) + 2 + len(current) <= budget:
|
||||
current = f"{last}\n\n{current}"
|
||||
else:
|
||||
head, sep, tail = last.rpartition("\n\n")
|
||||
if sep and len(tail) + 2 + len(current) <= budget:
|
||||
pieces.append(head)
|
||||
current = f"{tail}\n\n{current}"
|
||||
else:
|
||||
pieces.append(last)
|
||||
if current:
|
||||
pieces.append(current)
|
||||
return pieces
|
||||
@@ -499,12 +557,20 @@ def chunk_document(title: str | None, body: str | None) -> list[str]:
|
||||
|
||||
# Merge small adjacent sections upward so tiny sections don't each spend a
|
||||
# vector, then split anything still over budget at paragraph boundaries.
|
||||
# A THIN section takes the next one in whatever its size (#4784): a heading
|
||||
# with nothing under it ("## Findings" above its "### …" parts) or a lone
|
||||
# lead-in line would otherwise stand as a chunk, and the split below keeps
|
||||
# it attached to the first paragraph that follows.
|
||||
merged: list[str] = []
|
||||
for section in _split_sections(body or ""):
|
||||
if merged and len(merged[-1]) + 2 + len(section) <= budget:
|
||||
if merged and (_is_thin(merged[-1])
|
||||
or len(merged[-1]) + 2 + len(section) <= budget):
|
||||
merged[-1] = f"{merged[-1]}\n\n{section}"
|
||||
else:
|
||||
merged.append(section)
|
||||
if len(merged) > 1 and _is_thin(merged[-1]):
|
||||
tail = merged.pop()
|
||||
merged[-1] = f"{merged[-1]}\n\n{tail}"
|
||||
|
||||
chunks: list[str] = []
|
||||
for section in merged:
|
||||
@@ -512,13 +578,15 @@ def chunk_document(title: str | None, body: str | None) -> list[str]:
|
||||
chunks.append(embedding_text(title, section))
|
||||
continue
|
||||
pieces = _split_paragraphs(section, budget)
|
||||
first_line = section.split("\n", 1)[0]
|
||||
heading = first_line if first_line.lstrip().startswith("#") else ""
|
||||
heading = ""
|
||||
for i, piece in enumerate(pieces):
|
||||
# Repeat the section heading on continuation pieces so each vector
|
||||
# still knows what topic it is part of.
|
||||
# Repeat the heading a continuation piece sits under so each vector
|
||||
# still knows what topic it is part of. The LAST heading so far,
|
||||
# not the section's first: a thin heading merged forward (#4784)
|
||||
# means one section can hold several.
|
||||
if i > 0 and heading and not piece.startswith(heading):
|
||||
piece = f"{heading}\n{piece}"
|
||||
heading = _last_heading(piece, heading)
|
||||
chunks.append(embedding_text(title, piece))
|
||||
return chunks
|
||||
|
||||
|
||||
@@ -8,8 +8,11 @@ untouched), and long records lose NOTHING — every line of the body lands in
|
||||
some chunk, each chunk inside the window budget, each carrying the title as
|
||||
its topical anchor.
|
||||
"""
|
||||
import re
|
||||
|
||||
from scribe.services.embeddings import (
|
||||
_CHUNK_CHAR_BUDGET,
|
||||
_MIN_CHUNK_CONTENT,
|
||||
chunk_document,
|
||||
embedding_text,
|
||||
)
|
||||
@@ -136,6 +139,87 @@ def test_a_monster_single_paragraph_is_hard_split_not_dropped():
|
||||
assert total_words == 2000
|
||||
|
||||
|
||||
# --- no chunk is a heading alone (#4784) -------------------------------------
|
||||
#
|
||||
# Found by judging live auto-inject menus: "## Work log — 2026-08-21",
|
||||
# "## Findings worth carrying forward" and "Two dev→main PRs this session."
|
||||
# were each a whole chunk, and took ranks 1–3 on vague queries — text about
|
||||
# nothing embeds close to everything. Each case below is one of those shapes.
|
||||
|
||||
_HEADING = re.compile(r"^#{1,6}\s")
|
||||
|
||||
|
||||
def _said(chunk: str, title: str) -> str:
|
||||
"""A chunk's text once its title prefix and heading lines are set aside."""
|
||||
body = chunk[len(title) + 1:] if chunk.startswith(title + "\n") else chunk
|
||||
return "\n".join(ln for ln in body.splitlines() if not _HEADING.match(ln)).strip()
|
||||
|
||||
|
||||
def _assert_nothing_thin_and_nothing_lost(title: str, body: str) -> list[str]:
|
||||
chunks = chunk_document(title, body)
|
||||
assert len(chunks) > 1, "the case must be long enough to split"
|
||||
for chunk in chunks:
|
||||
assert len(_said(chunk, title)) >= _MIN_CHUNK_CONTENT, (
|
||||
f"a chunk says almost nothing: {chunk[:120]!r}")
|
||||
assert len(chunk) <= _CHUNK_CHAR_BUDGET + len(title) + 1 + 80
|
||||
joined = "\n".join(chunks)
|
||||
for line in (ln.strip() for ln in body.splitlines()):
|
||||
# A line longer than a chunk is cut across two, so only its ends can
|
||||
# be looked for whole.
|
||||
for part in ([line] if len(line) < 400 else [line[:60], line[-60:]]):
|
||||
assert part in joined, f"content dropped: {part[:60]!r}"
|
||||
return chunks
|
||||
|
||||
|
||||
def test_a_heading_over_one_long_paragraph_is_not_a_chunk_of_its_own():
|
||||
"""A work log whose entry is one long paragraph: the split used to flush
|
||||
the heading as its own piece before cutting the paragraph."""
|
||||
entry = " ".join(["The deploy was verified against the live instance."] * 40)
|
||||
_assert_nothing_thin_and_nothing_lost(
|
||||
"Step 6 — webhook drift-flagging",
|
||||
"The step body.\n\n## Work log — 2026-08-16\n\n" + entry,
|
||||
)
|
||||
|
||||
|
||||
def test_a_heading_with_nothing_under_it_joins_what_follows():
|
||||
"""'## Findings' directly above its '### …' parts is a section with no
|
||||
body, and stood alone whenever the section before it was full."""
|
||||
body = (
|
||||
# Five paragraphs fill the section before it, so the heading cannot
|
||||
# merge backwards and has to be carried forward.
|
||||
"## Context\n\n" + _long_section("context", paragraphs=5)
|
||||
+ "\n\n## Findings worth carrying forward\n\n"
|
||||
+ "### One\n\n" + _long_section("one", paragraphs=5)
|
||||
+ "\n\n### Two\n\n" + _long_section("two", paragraphs=5)
|
||||
)
|
||||
_assert_nothing_thin_and_nothing_lost("A milestone dev-log", body)
|
||||
|
||||
|
||||
def test_a_lone_lead_in_line_joins_the_section_after_it():
|
||||
_assert_nothing_thin_and_nothing_lost(
|
||||
"2026-06-10 — UI batch",
|
||||
"Two dev→main PRs this session.\n\n## PR 89\n\n"
|
||||
+ _long_section("pr89", paragraphs=8),
|
||||
)
|
||||
|
||||
|
||||
def test_a_thin_last_section_joins_the_one_before_it():
|
||||
_assert_nothing_thin_and_nothing_lost(
|
||||
"T",
|
||||
"## Body\n\n" + _long_section("body", paragraphs=8) + "\n\n## Next\n\nTBD.",
|
||||
)
|
||||
|
||||
|
||||
def test_a_continuation_piece_repeats_the_heading_it_sits_under():
|
||||
"""A thin heading merged forward puts two headings in one section; the
|
||||
pieces of the second must carry the second, not the first."""
|
||||
body = "## Parent\n\n## Child\n\n" + _long_section("child", paragraphs=40)
|
||||
chunks = chunk_document("T", body)
|
||||
assert len(chunks) > 2
|
||||
for chunk in chunks[1:]:
|
||||
assert "## Child" in chunk
|
||||
|
||||
|
||||
# --- the read path: best chunk wins (#280 step 4) ----------------------------
|
||||
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ call carrying arguments the tool does not declare, with a did-you-mean when
|
||||
one is close. An error the caller sees once beats data half-written forever.
|
||||
"""
|
||||
import pytest
|
||||
from mcp.server.mcpserver.exceptions import ToolError
|
||||
from mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError
|
||||
|
||||
from scribe.mcp.server import StrictArgsMCPServer
|
||||
|
||||
@@ -59,6 +59,58 @@ async def test_unknown_tool_keeps_the_upstream_error():
|
||||
assert "no_such_tool" in str(exc.value)
|
||||
|
||||
|
||||
# --- a refusal's text reaches the agent (#4794) -------------------------------
|
||||
#
|
||||
# SDK 2.x passes on only a ToolError's text; anything else is wrapped as
|
||||
# "Error executing tool X" and its message is kept on the server. ValueError is
|
||||
# how every Scribe tool refuses, so for weeks every refusal arrived bare.
|
||||
|
||||
|
||||
def _raising_server() -> StrictArgsMCPServer:
|
||||
mcp = StrictArgsMCPServer("refusal-test")
|
||||
|
||||
@mcp.tool()
|
||||
def refuse() -> str:
|
||||
raise ValueError("record 7 is not yours to change — open your own copy")
|
||||
|
||||
@mcp.tool()
|
||||
def crash() -> str:
|
||||
raise RuntimeError("password=hunter2 at db.internal:5432")
|
||||
|
||||
return mcp
|
||||
|
||||
|
||||
async def test_a_refusal_reaches_the_caller_with_its_reason():
|
||||
mcp = _raising_server()
|
||||
with pytest.raises(ToolError) as exc:
|
||||
await mcp.call_tool("refuse", {})
|
||||
msg = str(exc.value)
|
||||
assert "Error executing tool refuse" in msg
|
||||
assert "record 7 is not yours to change — open your own copy" in msg
|
||||
assert not isinstance(exc.value, UnexpectedToolError), (
|
||||
"still typed as a crash, so the SDK's handler would log it as one")
|
||||
|
||||
|
||||
async def test_a_crash_stays_masked():
|
||||
"""Only the refusal convention is unmasked: a crash's text can carry
|
||||
internals, and the SDK hides it for that reason."""
|
||||
mcp = _raising_server()
|
||||
with pytest.raises(UnexpectedToolError) as exc:
|
||||
await mcp.call_tool("crash", {})
|
||||
assert "hunter2" not in str(exc.value)
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("_bind_user")
|
||||
async def test_a_real_tools_refusal_reaches_the_caller():
|
||||
"""Against the real server, on a refusal that needs no database: the
|
||||
message judge_menu writes for an empty verdict list."""
|
||||
from scribe.mcp.server import build_mcp_server
|
||||
|
||||
with pytest.raises(ToolError) as exc:
|
||||
await build_mcp_server().call_tool("judge_menu", {"log_id": 1, "verdicts": []})
|
||||
assert "at least one verdict" in str(exc.value)
|
||||
|
||||
|
||||
async def test_the_original_regression_create_note_with_content():
|
||||
"""Pin #2709 itself against the REAL server: `content=` on create_note
|
||||
must raise before dispatch — naming the bad argument and listing `body`
|
||||
|
||||
@@ -52,6 +52,10 @@ SYNTHETIC = [
|
||||
"<local-command-stderr>Failed to reconnect</local-command-stderr>",
|
||||
"<local-command-caveat>Caveat: The messages below were generated…</local-command-caveat>",
|
||||
" \n<task-notification>\n<task-id>abc</task-id>", # whitespace must not walk the filter
|
||||
# A subagent's hand-back (#4785) — it carries attributes, so the tag is
|
||||
# matched up to its name, not to a closing `>`.
|
||||
'<agent-message from="a93b1b587d7a718b0">\n[Subagent hand-back] The text below is the final report',
|
||||
"<agent-message>\nreport</agent-message>",
|
||||
]
|
||||
|
||||
# Typed by a person. Every one of these must still be retrieved against.
|
||||
@@ -62,6 +66,7 @@ REAL = [
|
||||
"fix the hook<system-reminder>injected</system-reminder>", # tag, but trailing
|
||||
"This session is being continued from a previous conversation.", # see module docstring
|
||||
"<taskbar> is rendering wrong", # a tag, but not one of the client's
|
||||
"<agent-messages> list is empty", # shares the prefix, is not the tag
|
||||
"here is the transcript you asked for:\n<local-command-stdout>hi</local-command-stdout>",
|
||||
]
|
||||
|
||||
|
||||
Reference in New Issue
Block a user