mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-09 09:11:00 +08:00
fix(benchmark): separate default response cost from quality JSON
Route run_cli_tool_call() and run_mcp_tool_call() through a default-format request for elapsed_ms, response_bytes, and response_token_estimate, then issue format=json only for quality scoring. Record quality_probe_elapsed_ms and quality_response_bytes so the second request cannot inflate the user-facing latency or replace TOON payload sizing. This fixes the pilot JSONDecodeError: Expecting value at line 1 column 1 when search_graph returned its default TOON content. Document the two-request instrumentation contract in docs/BENCHMARK_CAMPAIGN.md. Validated with 23 focused pytest cases across tests/test_benchmark_incremental_speed.py, tests/test_summarize_benchmark_results.py, and tests/test_benchmark_campaign.py; Ruff and git diff --check pass. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
@@ -107,12 +107,15 @@ Every invocation also writes:
|
||||
run directories;
|
||||
- `reports/summary.md`, regenerated from validated completion records only.
|
||||
|
||||
The report lists exact canonical response bytes and a clearly labeled deterministic
|
||||
`ceil(UTF-8 bytes / 4)` token estimate. Pareto membership is restricted to candidates
|
||||
that pass every applicable quality/correctness gate and have query latency, response
|
||||
tokens, incremental latency, and peak RSS measurements. It maximizes quality while
|
||||
minimizing those cost axes. Exact bytes remain visible so the token estimate is never
|
||||
presented as tokenizer ground truth.
|
||||
The report lists exact bytes from each tool's default response encoding and a clearly
|
||||
labeled deterministic `ceil(UTF-8 bytes / 4)` token estimate. Each quality oracle makes
|
||||
a second request with `format=json`; its latency and canonical JSON size are recorded
|
||||
separately as `quality_probe_elapsed_ms` and `quality_response_bytes`, so parsing the
|
||||
oracle cannot silently replace or inflate the default user-facing measurement. Pareto
|
||||
membership is restricted to candidates that pass every applicable quality/correctness
|
||||
gate and have query latency, response tokens, incremental latency, and peak RSS
|
||||
measurements. It maximizes quality while minimizing those cost axes. Exact bytes remain
|
||||
visible so the token estimate is never presented as tokenizer ground truth.
|
||||
|
||||
Use `--audit-only` to scan and regenerate the report without running missing cells.
|
||||
Use `--minimum-free-gb` and `--stale-lock-hours` only when the recorded defaults are
|
||||
|
||||
@@ -269,6 +269,20 @@ def unwrap_mcp_result(response: dict[str, Any]) -> dict[str, Any]:
|
||||
return result
|
||||
|
||||
|
||||
def cli_result_text(stdout: str) -> str:
|
||||
outer = json.loads(stdout)
|
||||
if "content" in outer:
|
||||
return str(outer["content"][0]["text"])
|
||||
return json.dumps(outer, separators=(",", ":"), sort_keys=True)
|
||||
|
||||
|
||||
def mcp_result_text(response: dict[str, Any]) -> str:
|
||||
result = response.get("result", {})
|
||||
if "content" in result:
|
||||
return str(result["content"][0]["text"])
|
||||
return json.dumps(result, separators=(",", ":"), sort_keys=True)
|
||||
|
||||
|
||||
TOKEN_ESTIMATOR = "utf8_bytes_div_4_ceil"
|
||||
|
||||
|
||||
@@ -398,13 +412,19 @@ class McpClient:
|
||||
def call_tool(
|
||||
self, name: str, arguments: dict[str, Any]
|
||||
) -> tuple[dict[str, Any], str, int, float]:
|
||||
text, stderr, stdout_bytes, elapsed_ms = self.call_tool_text(name, arguments)
|
||||
return json.loads(text), stderr, stdout_bytes, elapsed_ms
|
||||
|
||||
def call_tool_text(
|
||||
self, name: str, arguments: dict[str, Any]
|
||||
) -> tuple[str, str, int, float]:
|
||||
mark = self._stderr_mark()
|
||||
start = now_ms()
|
||||
response = self._request("tools/call", {"name": name, "arguments": arguments})
|
||||
elapsed_ms = now_ms() - start
|
||||
stderr = self._stderr_since(mark)
|
||||
stdout_bytes = len(json.dumps(response, separators=(",", ":")).encode("utf-8"))
|
||||
return unwrap_mcp_result(response), stderr, stdout_bytes, elapsed_ms
|
||||
return mcp_result_text(response), stderr, stdout_bytes, elapsed_ms
|
||||
|
||||
|
||||
def log_tail(stderr: str) -> list[str]:
|
||||
@@ -725,8 +745,10 @@ def build_tool_call_result(
|
||||
stdout_bytes: int,
|
||||
elapsed_ms: float,
|
||||
include_logs: bool,
|
||||
response_payload: bytes | None = None,
|
||||
) -> dict[str, Any]:
|
||||
payload = canonical_response_bytes(data)
|
||||
quality_payload = canonical_response_bytes(data)
|
||||
payload = response_payload if response_payload is not None else quality_payload
|
||||
result: dict[str, Any] = {
|
||||
"elapsed_ms": round(elapsed_ms, 3),
|
||||
# Preserve the historical field while separating transport framing from
|
||||
@@ -736,6 +758,8 @@ def build_tool_call_result(
|
||||
"response_bytes": len(payload),
|
||||
"response_token_estimate": estimate_response_tokens(payload),
|
||||
"token_estimator": TOKEN_ESTIMATOR,
|
||||
"response_encoding": "tool_default" if response_payload is not None else "canonical_json",
|
||||
"quality_response_bytes": len(quality_payload),
|
||||
"response": data,
|
||||
"freshness_state": response_freshness_state(data) or None,
|
||||
"freshness": response_freshness(data),
|
||||
@@ -754,14 +778,29 @@ def run_cli_tool_call(
|
||||
timeout: int,
|
||||
include_logs: bool,
|
||||
) -> dict[str, Any]:
|
||||
cmd = [str(binary), "cli", "--json", tool_name, json.dumps(arguments, separators=(",", ":"))]
|
||||
encoded = json.dumps(arguments, separators=(",", ":"))
|
||||
cmd = [str(binary), "cli", "--json", tool_name, encoded]
|
||||
proc, elapsed_ms = command_result(cmd, env, timeout)
|
||||
if proc.returncode != 0:
|
||||
raise command_failure(f"{tool_name}_call", cmd, env, proc, elapsed_ms)
|
||||
data = unwrap_cli_json(proc.stdout)
|
||||
return build_tool_call_result(
|
||||
data, proc.stderr, len(proc.stdout.encode("utf-8")), elapsed_ms, include_logs
|
||||
raw_payload = cli_result_text(proc.stdout).encode("utf-8")
|
||||
quality_arguments = dict(arguments)
|
||||
quality_arguments["format"] = "json"
|
||||
quality_cmd = [
|
||||
str(binary), "cli", "--json", tool_name,
|
||||
json.dumps(quality_arguments, separators=(",", ":")),
|
||||
]
|
||||
quality_proc, quality_elapsed_ms = command_result(quality_cmd, env, timeout)
|
||||
if quality_proc.returncode != 0:
|
||||
raise command_failure(
|
||||
f"{tool_name}_quality_call", quality_cmd, env, quality_proc, quality_elapsed_ms
|
||||
)
|
||||
data = unwrap_cli_json(quality_proc.stdout)
|
||||
result = build_tool_call_result(
|
||||
data, proc.stderr, len(proc.stdout.encode("utf-8")), elapsed_ms, include_logs, raw_payload
|
||||
)
|
||||
result["quality_probe_elapsed_ms"] = round(quality_elapsed_ms, 3)
|
||||
return result
|
||||
|
||||
|
||||
def run_mcp_tool_call(
|
||||
@@ -770,8 +809,15 @@ def run_mcp_tool_call(
|
||||
arguments: dict[str, Any],
|
||||
include_logs: bool,
|
||||
) -> dict[str, Any]:
|
||||
data, stderr, stdout_bytes, elapsed_ms = client.call_tool(tool_name, arguments)
|
||||
return build_tool_call_result(data, stderr, stdout_bytes, elapsed_ms, include_logs)
|
||||
raw_text, stderr, stdout_bytes, elapsed_ms = client.call_tool_text(tool_name, arguments)
|
||||
quality_arguments = dict(arguments)
|
||||
quality_arguments["format"] = "json"
|
||||
data, _, _, quality_elapsed_ms = client.call_tool(tool_name, quality_arguments)
|
||||
result = build_tool_call_result(
|
||||
data, stderr, stdout_bytes, elapsed_ms, include_logs, raw_text.encode("utf-8")
|
||||
)
|
||||
result["quality_probe_elapsed_ms"] = round(quality_elapsed_ms, 3)
|
||||
return result
|
||||
|
||||
|
||||
def run_tool_call_for_transport(
|
||||
|
||||
@@ -12,17 +12,56 @@ SPEC.loader.exec_module(BENCHMARK)
|
||||
|
||||
|
||||
class BenchmarkIncrementalSpeedTest(unittest.TestCase):
|
||||
def test_tool_result_measures_canonical_payload_not_transport_envelope(self) -> None:
|
||||
def test_tool_result_separates_default_payload_quality_json_and_transport(self) -> None:
|
||||
default_payload = b"total: 1\nresults[1]{name}:\n alpha\n"
|
||||
result = BENCHMARK.build_tool_call_result(
|
||||
{"name": "alpha", "items": [1, 2]}, "", 999, 12.5, False
|
||||
{"name": "alpha", "items": [1, 2]}, "", 999, 12.5, False,
|
||||
default_payload,
|
||||
)
|
||||
canonical = b'{"items":[1,2],"name":"alpha"}'
|
||||
self.assertEqual(result["transport_response_bytes"], 999)
|
||||
self.assertEqual(result["response_bytes"], len(canonical))
|
||||
self.assertEqual(result["response_bytes"], len(default_payload))
|
||||
self.assertEqual(result["quality_response_bytes"], len(canonical))
|
||||
self.assertEqual(
|
||||
result["response_token_estimate"], BENCHMARK.estimate_response_tokens(canonical)
|
||||
result["response_token_estimate"], BENCHMARK.estimate_response_tokens(default_payload)
|
||||
)
|
||||
self.assertEqual(result["token_estimator"], "utf8_bytes_div_4_ceil")
|
||||
self.assertEqual(result["response_encoding"], "tool_default")
|
||||
|
||||
def test_result_text_extractors_preserve_default_toon(self) -> None:
|
||||
toon = "total: 1\nresults[1]{name}:\n alpha\n"
|
||||
cli_stdout = '{"content":[{"type":"text","text":"total: 1\\nresults[1]{name}:\\n alpha\\n"}]}'
|
||||
mcp_response = {"result": {"content": [{"type": "text", "text": toon}]}}
|
||||
self.assertEqual(BENCHMARK.cli_result_text(cli_stdout), toon)
|
||||
self.assertEqual(BENCHMARK.mcp_result_text(mcp_response), toon)
|
||||
|
||||
def test_mcp_tool_call_measures_default_payload_and_uses_json_for_quality(self) -> None:
|
||||
class FakeClient:
|
||||
def call_tool_text(self, name, arguments):
|
||||
self.default_call = (name, arguments)
|
||||
return "total: 1\nresults[1]{name}:\n alpha\n", "default log", 321, 7.25
|
||||
|
||||
def call_tool(self, name, arguments):
|
||||
self.quality_call = (name, arguments)
|
||||
return {"results": [{"name": "alpha"}]}, "quality log", 654, 2.5
|
||||
|
||||
client = FakeClient()
|
||||
result = BENCHMARK.run_mcp_tool_call(
|
||||
client, "search_graph", {"name_pattern": "alpha"}, False
|
||||
)
|
||||
|
||||
self.assertEqual(
|
||||
client.default_call, ("search_graph", {"name_pattern": "alpha"})
|
||||
)
|
||||
self.assertEqual(
|
||||
client.quality_call,
|
||||
("search_graph", {"name_pattern": "alpha", "format": "json"}),
|
||||
)
|
||||
self.assertEqual(result["elapsed_ms"], 7.25)
|
||||
self.assertEqual(result["quality_probe_elapsed_ms"], 2.5)
|
||||
self.assertEqual(result["transport_response_bytes"], 321)
|
||||
self.assertEqual(result["response_encoding"], "tool_default")
|
||||
self.assertEqual(result["response"]["results"][0]["name"], "alpha")
|
||||
|
||||
def test_quality_summary_requires_every_applicable_oracle(self) -> None:
|
||||
oracles = {
|
||||
|
||||
Reference in New Issue
Block a user