Skip to content

Measure generic model-facing tool ergonomics #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yyjeqhc merged 3 commits into from
Aug 23, 2026
Merged

Measure generic model-facing tool ergonomics #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e90d0d4
Measure generic model-facing tool ergonomics
yyjeqhc Aug 23, 2026
ffed7ec
Close model ergonomics telemetry review gaps
yyjeqhc Aug 23, 2026
c9759ee
Merge remote-tracking branch 'origin/main' into feat/model-ergonomics…
yyjeqhc Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions docs/agent/session-model.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,75 @@ not “what is the coding task ledger for this repo work?”.
- Idle open-session reuse and explicit close for operator audit views
- Aggregate stats for read-only audit APIs, with explicit bounded-scan coverage

### Generic model-ergonomics telemetry

Model-visible **runtime** tool calls reuse the existing Action Audit event as the
durable/queryable sink for low-cardinality ergonomics telemetry. No second
telemetry table or recorder is created. The shared ToolRuntime kernel owns the
normal timer for a registered model-visible tool; the transport that already owns
the outer Action Audit row then finalizes one `summary.model_ergonomics` object
from the final model-facing ToolResult projection. A transport may use a bounded
fallback timer only after a runtime tool identity is established when MCP-only
validation rejects the call before kernel entry or the MCP hard dispatch timeout
prevents kernel completion. Batch items do not create generic invocation records,
and hidden/internal helpers do not start this telemetry.

The version-1 generic record contains only:

- `schema_version = 1`;
- registry-owned `tool_name` and closed/bounded `tool_category`;
- `success` and non-negative `duration_ms` for the outer invocation up to its
returned ToolResult/Job handoff;
- nullable `serialized_result_bytes`;
- nullable structured `error_kind`, `failure_kind`, `recovery_kind`, and
authoritative closed `execution_state` when present.

`serialized_result_bytes` is the UTF-8 byte length of the exact final
model-facing ToolResult JSON object (`success`, `output`, and `error` only when
present), serialized with the normal serde JSON representation. It is not a
character count, Rust memory size, stdout/stderr estimate, database-row size, or
HTTP/JSON-RPC/MCP framing size. MCP finalizes the count from the final
`structuredContent` ToolResult after MCP-only image/resource framing, so bytes
removed from the model-facing ToolResult are not charged. If a registered
model-visible tool identity has already been established but the call is rejected
before any ToolResult exists (for example invalid arguments, insufficient scope,
or MCP wrapper validation), the invocation is still counted but
`serialized_result_bytes` is `null`; the transport-specific error envelope is
never substituted for a ToolResult. The MCP hard dispatch timeout is recorded the
same way with structured `error_kind = dispatch_hard_timeout` so stalled calls do
not disappear from failure/latency aggregates.
There is currently no
authoritative generic final-result truncation fact, so generic
`result_truncated` is deliberately **not** recorded; tool-specific truncation
fields keep their existing meanings.

Classification consumes structured ToolResult fields only. It never derives a
kind from arbitrary English error prose. The generic record stores no tool
arguments, commands/argv/scripts/stdin/stdout/stderr, ToolResult body or error
prose, paths/cwd/project ids, query/file/clipboard/Computer contents, Session
message/prompt/answer bodies, credentials, native identities, or arbitrary user
text. Existing Action Audit correlation/attribution fields remain separate
pre-existing audit data; P1a does not copy them into `model_ergonomics`.

`edit_tool_telemetry` remains the edit-specific structured tracing enrichment.
An edit invocation therefore has one generic Action Audit invocation record plus
its existing edit-specific enrichment, not two generic counts. Workflow Session
`tool_call_started` / `tool_call_finished` events remain a separate workflow
ledger and are not the persistence source for this aggregate ergonomics data.

Telemetry is observation-only and failure-isolated. Failure to serialize the
bounded generic projection or to persist the Action Audit row is dropped/warned
without changing the tool's success, output, error, permission, execution state,
retry safety, Job lifecycle, or Computer authority.

For dogfood analysis, read bounded Action Audit events through
`/api/audit/session` or query SQLite `action_events.summary_json`, select rows
with `summary.model_ergonomics`, and aggregate by `tool_name`. The raw bounded
records are sufficient for invocation/success counts, duration p50/p95,
serialized-result mean/high percentiles, and structured error/recovery-kind
distributions in later SQL/Python analysis; no dashboard or analytics service is
part of this contract.

### Identity

| Aspect | Contract |
Expand Down
94 changes: 88 additions & 6 deletions src/mcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ use crate::tool_runtime::kernel::{
check_runtime_tool_scope, HostFileImportTrust, ToolCallContext, ToolCallErrorStatus,
ToolCallRequest as KernelToolCallRequest, ToolTransport,
};
use crate::tool_runtime::model_ergonomics_telemetry::{
ModelErgonomicsRecord, ModelErgonomicsTimer,
};
use crate::tool_runtime::tool_definition::LOCAL_CODING_TOOL_NAMES;
#[cfg(test)]
use crate::tool_runtime::MAX_PROJECT_ARTIFACT_BYTES;
Expand Down Expand Up @@ -2266,11 +2269,20 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
} else {
None
};
let record_audit = |success: bool, status: StatusCode, error: Option<String>| {
let record_audit = |success: bool,
status: StatusCode,
error: Option<String>,
model_ergonomics: Option<&ModelErgonomicsRecord>| {
if let Some((audit, tool, project)) = audit.as_ref() {
let mut summary = json!({ "transport": "mcp" });
if let Some(telemetry) =
model_ergonomics.and_then(|record| serde_json::to_value(record).ok())
{
summary["model_ergonomics"] = telemetry;
}
let mut event = ActionAuditRecord::new(tool.clone(), success, status)
.error(error)
.summary(json!({ "transport": "mcp" }));
.summary(summary);
event.project = project.clone();
audit.record(event);
}
Expand All @@ -2291,6 +2303,16 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
// failure mode behind "MCP request never gets a reply"), converting a
// silently dead HTTP request into an observable JSON-RPC error.
let request_id = request.id.clone();
// The shared kernel timer is authoritative for completed runtime calls. Keep
// one outer emergency timer only so the MCP hard-timeout path does not erase
// an otherwise established runtime invocation from ergonomics telemetry.
let mut hard_timeout_model_ergonomics =
if runtime.model_surface() == ModelSurface::CanonicalConnector {
None
} else {
tool_name.as_deref().and_then(ModelErgonomicsTimer::start)
};
let mut model_ergonomics = None;
let outcome = match tokio::time::timeout(
MCP_DISPATCH_HARD_TIMEOUT,
handle_mcp_request_with_lifecycle(
Expand All @@ -2302,6 +2324,7 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
host_file_import_trust,
window.identity.as_ref(),
Some(&mut guard),
Some(&mut model_ergonomics),
),
)
.await
Expand Down Expand Up @@ -2330,10 +2353,16 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
Some(-32000),
);
}
let timeout_model_ergonomics = hard_timeout_model_ergonomics.take().map(|timer| {
timer
.finish()
.record_for_pre_result_failure("dispatch_hard_timeout")
});
record_audit(
false,
StatusCode::INTERNAL_SERVER_ERROR,
Some("mcp dispatch hard timeout".to_string()),
timeout_model_ergonomics.as_ref(),
);
let estimated = estimate_json_bytes(&body);
guard.response_serialized(500, estimated, Some(false), None, "dispatch_hard_timeout");
Expand Down Expand Up @@ -2396,14 +2425,15 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
.as_str()
.map(str::to_string)
},
model_ergonomics.as_ref(),
);
let estimated = estimate_json_bytes(&body);
guard.response_serialized(200, estimated, Some(true), tool_success, "ok");
res.render(Json(body));
guard.handler_returned(200, estimated, Some(true), tool_success, "ok");
}
McpOutcome::ArtifactExportStream { id, plan } => {
record_audit(true, StatusCode::OK, None);
record_audit(true, StatusCode::OK, None, None);
guard.response_serialized(200, None, Some(true), None, "artifact_export_stream");
res.status_code(StatusCode::OK);
let _ = res.add_header("content-type", "application/json", true);
Expand Down Expand Up @@ -2450,6 +2480,7 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
false,
StatusCode::BAD_REQUEST,
body["error"]["message"].as_str().map(str::to_string),
model_ergonomics.as_ref(),
);
let estimated = estimate_json_bytes(&body);
guard.response_serialized(400, estimated, Some(false), None, "bad_request");
Expand All @@ -2462,6 +2493,7 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
false,
StatusCode::NOT_FOUND,
body["error"]["message"].as_str().map(str::to_string),
model_ergonomics.as_ref(),
);
let estimated = estimate_json_bytes(&body);
guard.response_serialized(404, estimated, Some(false), None, "not_found");
Expand All @@ -2480,6 +2512,7 @@ pub async fn mcp_post(req: &mut Request, depot: &mut Depot, res: &mut Response)
"insufficient scope: {}",
required_scope.unwrap_or("unknown")
)),
model_ergonomics.as_ref(),
);
let estimated = estimate_json_bytes(&body);
guard.response_serialized(403, estimated, Some(false), None, "forbidden");
Expand Down Expand Up @@ -2525,6 +2558,7 @@ async fn handle_mcp_request(
HostFileImportTrust::Untrusted,
None,
None,
None,
)
.await;
match outcome {
Expand All @@ -2547,6 +2581,7 @@ async fn handle_mcp_request_with_lifecycle(
host_file_import_trust: HostFileImportTrust,
window: Option<&crate::client_window::ClientWindow>,
mut lifecycle: Option<&mut ToolRequestLifecycle>,
mut model_ergonomics_out: Option<&mut Option<ModelErgonomicsRecord>>,
) -> McpOutcome {
let stateless_2026 = protocol_era == McpProtocolEra::Stateless2026;
let artifact_export_resource_read = stateless_2026
Expand Down Expand Up @@ -2888,6 +2923,11 @@ async fn handle_mcp_request_with_lifecycle(
),
));
}
// From here on, the MCP boundary has established a model-visible runtime
// tool identity. A few MCP-only validations still happen before the
// shared ToolRuntime kernel; preserve those failed attempts in generic
// telemetry without creating a second record for normal kernel calls.
let mut pre_kernel_model_ergonomics = ModelErgonomicsTimer::start(&params.name);
let artifact_export_caller = if params.name == "export_project_artifact" {
if !stateless_2026 || runtime.model_surface() != ModelSurface::FullOperatorRuntime {
return McpOutcome::BadRequest(rpc_error(
Expand All @@ -2899,11 +2939,21 @@ async fn handle_mcp_request_with_lifecycle(
match mcp_artifact_export_caller_binding(auth) {
Ok(caller) => Some(caller),
Err(error) => {
if let (Some(slot), Some(timer)) = (
model_ergonomics_out.as_deref_mut(),
pre_kernel_model_ergonomics.take(),
) {
*slot = Some(
timer
.finish()
.record_for_pre_result_failure("invalid_arguments"),
);
}
return McpOutcome::BadRequest(rpc_error(
id,
-32602,
format!("export_project_artifact cannot bind this caller: {error}"),
))
));
}
}
} else {
Expand Down Expand Up @@ -2994,6 +3044,16 @@ async fn handle_mcp_request_with_lifecycle(
lc.dispatch_failed("invalid_arguments");
lc.dispatch_finished(false, Some(false), "invalid_arguments");
}
if let (Some(slot), Some(timer)) = (
model_ergonomics_out.as_deref_mut(),
pre_kernel_model_ergonomics.take(),
) {
*slot = Some(
timer
.finish()
.record_for_pre_result_failure("invalid_arguments"),
);
}
return McpOutcome::BadRequest(rpc_error(id, -32602, message));
}
};
Expand All @@ -3015,6 +3075,7 @@ async fn handle_mcp_request_with_lifecycle(
},
)
.await;
let model_ergonomics_completion = outcome.model_ergonomics;
let result = match outcome.error_status {
Some(ToolCallErrorStatus::InsufficientScope {
required_scope,
Expand All @@ -3024,13 +3085,26 @@ async fn handle_mcp_request_with_lifecycle(
lc.dispatch_failed("forbidden");
lc.dispatch_finished(false, Some(false), "forbidden");
}
if let (Some(slot), Some(completion)) = (
model_ergonomics_out.as_deref_mut(),
model_ergonomics_completion.as_ref(),
) {
*slot =
Some(completion.record_for_pre_result_failure("insufficient_scope"));
}
return scope_forbidden(auth, required_scope, description);
}
Some(ToolCallErrorStatus::InvalidArguments { message }) => {
if let Some(lc) = lifecycle.as_deref() {
lc.dispatch_failed("invalid_arguments");
lc.dispatch_finished(false, Some(false), "invalid_arguments");
}
if let (Some(slot), Some(completion)) = (
model_ergonomics_out.as_deref_mut(),
model_ergonomics_completion.as_ref(),
) {
*slot = Some(completion.record_for_pre_result_failure("invalid_arguments"));
}
return McpOutcome::BadRequest(rpc_error(id, -32602, message));
}
None => outcome
Expand Down Expand Up @@ -3065,14 +3139,22 @@ async fn handle_mcp_request_with_lifecycle(
snapshot_resource_caller,
)
};
rpc_result(
let model_ergonomics = model_ergonomics_completion.as_ref().and_then(|completion| {
result
.get("structuredContent")
.and_then(|structured| completion.record_for_structured_content(structured))
});
if let Some(slot) = model_ergonomics_out.as_deref_mut() {
*slot = model_ergonomics;
}
return McpOutcome::Ok(rpc_result(
id,
if stateless_2026 {
mcp_stateless_result(result, false)
} else {
result
},
)
));
}
"notifications/initialized" if !stateless_2026 => rpc_result(id, json!({})),
_ => {
Expand Down
Loading
Loading