-
Notifications
You must be signed in to change notification settings - Fork 272
feat(relay): report routing outcome evidence #684
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,17 +202,25 @@ impl SwitchyardRuntime { | |
| }); | ||
| match route.execute(request, Some(observer)).await { | ||
| Ok(output) => { | ||
| self.emit_observations(&mut events, take_observations(&observations), &metadata); | ||
| let mut outcome_fields = self.emit_observations( | ||
| &mut events, | ||
| take_observations(&observations), | ||
| &metadata, | ||
| ); | ||
| let served_model = output.response.served_model().map(|model| model.as_str()); | ||
| let mut data = json!({ | ||
| "algorithm": route.algorithm_name(), | ||
| "selected_model": output.selected_model.as_str(), | ||
| "served_model": served_model, | ||
| "fallback_used": served_model | ||
| .map(|model| model != output.selected_model.as_str()), | ||
| }); | ||
| if let Json::Object(data) = &mut data { | ||
| data.append(&mut outcome_fields); | ||
| } | ||
| events.push(RoutingEvent::Mark(RoutingMark { | ||
| name: "switchyard.routing.decision".into(), | ||
| data: json!({ | ||
| "algorithm": route.algorithm_name(), | ||
| "selected_model": output.selected_model.as_str(), | ||
| "served_model": served_model, | ||
| "fallback_used": served_model | ||
| .map(|model| model != output.selected_model.as_str()), | ||
| }), | ||
| data, | ||
| metadata, | ||
| severity: Some(LogSeverity::Info), | ||
| })); | ||
|
|
@@ -222,11 +230,16 @@ impl SwitchyardRuntime { | |
| } | ||
| } | ||
| Err(error) => { | ||
| self.emit_observations(&mut events, take_observations(&observations), &metadata); | ||
| let outcome_fields = self.emit_observations( | ||
| &mut events, | ||
| take_observations(&observations), | ||
| &metadata, | ||
| ); | ||
| self.route_execution_error_mark( | ||
| &mut events, | ||
| &error.execution_error_summary(), | ||
| None, | ||
| outcome_fields, | ||
| ); | ||
| Execution { | ||
| result: Err("Switchyard route execution failed".into()), | ||
|
|
@@ -249,10 +262,20 @@ impl SwitchyardRuntime { | |
| events: &mut Vec<RoutingEvent>, | ||
| observations: Vec<RunObservation>, | ||
| metadata: &Json, | ||
| ) { | ||
| ) -> Map<String, Json> { | ||
| let mut call_index = 0; | ||
| let mut outcome_fields = Map::new(); | ||
| for observation in observations { | ||
| match observation { | ||
| RunObservation::Outcome(outcome) => { | ||
| outcome_fields.insert( | ||
| "outcome_id".into(), | ||
| Json::String(outcome.outcome_id().into()), | ||
| ); | ||
| if let Some(evidence) = evidence_for_mark(outcome.evidence) { | ||
| outcome_fields.insert("evidence".into(), evidence); | ||
| } | ||
| } | ||
| RunObservation::LlmCall(call) => { | ||
| call_index += 1; | ||
| self.routing_call_events(events, call, call_index, metadata); | ||
|
|
@@ -287,6 +310,7 @@ impl SwitchyardRuntime { | |
| } | ||
| } | ||
| } | ||
| outcome_fields | ||
| } | ||
|
|
||
| fn routing_call_events( | ||
|
|
@@ -338,14 +362,31 @@ impl SwitchyardRuntime { | |
| events: &mut Vec<RoutingEvent>, | ||
| summary: &RouteErrorSummary, | ||
| metadata: Option<&Json>, | ||
| outcome_fields: Map<String, Json>, | ||
| ) { | ||
| let metadata = metadata | ||
| .cloned() | ||
| .unwrap_or_else(|| event_metadata(events).unwrap_or_else(|| Json::Object(Map::new()))); | ||
| events.extend(route_execution_error_events(summary, metadata)); | ||
| events.extend(route_execution_error_events( | ||
| summary, | ||
| metadata, | ||
| outcome_fields, | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| fn evidence_for_mark(evidence: Option<Json>) -> Option<Json> { | ||
| let Some(Json::Object(mut evidence)) = evidence else { | ||
| return None; | ||
| }; | ||
| evidence.retain(|name, value| match name.as_str() { | ||
| "source" | "verdict" | "trigger" | "reason_code" => value.is_string(), | ||
| "score" | "confidence" | "threshold" => value.is_number(), | ||
| _ => false, | ||
| }); | ||
| (!evidence.is_empty()).then_some(Json::Object(evidence)) | ||
| } | ||
|
Comment on lines
+378
to
+388
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add the required Rust comments. The evidence filter has a telemetry safety contract. The new tests preserve important routing outcome behavior.
As per coding guidelines, Rust changes need concise comments for private helpers with non-obvious behavior and tests that encode important behavior. 📍 Affects 1 file
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| pub(crate) fn emit_events(runtime: &PluginRuntime, events: Vec<RoutingEvent>) { | ||
| for event in events { | ||
| emit_event(runtime, event); | ||
|
|
@@ -425,6 +466,7 @@ fn returned_events( | |
| for event in route_execution_error_events( | ||
| &stream_error_summary(error, served_model.as_ref()), | ||
| metadata.clone(), | ||
| Map::new(), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win Preserve routing outcome fields for late stream failures.
🤖 Prompt for AI Agents |
||
| ) { | ||
| emit_event(event); | ||
| } | ||
|
|
@@ -491,24 +533,40 @@ fn relay_stream_error(error: LlmStreamError) -> String { | |
| } | ||
| } | ||
|
|
||
| fn route_execution_error_mark(summary: &RouteErrorSummary, metadata: Json) -> RoutingMark { | ||
| fn route_execution_error_mark( | ||
| summary: &RouteErrorSummary, | ||
| metadata: Json, | ||
| mut outcome_fields: Map<String, Json>, | ||
| ) -> RoutingMark { | ||
| let mut data = json!({ | ||
| "failure_kind": "route_execution", | ||
| "category": summary.kind.as_str(), | ||
| "phase": summary.phase.as_str(), | ||
| "upstream_status": summary.upstream_status, | ||
| "target": summary.target.as_ref().map(|target| target.as_str()), | ||
| }); | ||
| if let Json::Object(data) = &mut data { | ||
| data.append(&mut outcome_fields); | ||
| } | ||
| RoutingMark { | ||
| name: "switchyard.routing.error".into(), | ||
| data: json!({ | ||
| "failure_kind": "route_execution", | ||
| "category": summary.kind.as_str(), | ||
| "phase": summary.phase.as_str(), | ||
| "upstream_status": summary.upstream_status, | ||
| "target": summary.target.as_ref().map(|target| target.as_str()), | ||
| }), | ||
| data, | ||
| metadata, | ||
| severity: Some(LogSeverity::Error), | ||
| } | ||
| } | ||
|
|
||
| fn route_execution_error_events(summary: &RouteErrorSummary, metadata: Json) -> Vec<RoutingEvent> { | ||
| fn route_execution_error_events( | ||
| summary: &RouteErrorSummary, | ||
| metadata: Json, | ||
| outcome_fields: Map<String, Json>, | ||
| ) -> Vec<RoutingEvent> { | ||
| vec![ | ||
| RoutingEvent::Mark(route_execution_error_mark(summary, metadata.clone())), | ||
| RoutingEvent::Mark(route_execution_error_mark( | ||
| summary, | ||
| metadata.clone(), | ||
| outcome_fields, | ||
| )), | ||
| failure_metric( | ||
| "route_execution", | ||
| Some(summary.kind.as_str()), | ||
|
|
@@ -894,12 +952,90 @@ mod tests { | |
| assert_eq!(decision["selected_model"], "target/model"); | ||
| assert_eq!(decision["served_model"], "target/model"); | ||
| assert_eq!(decision["fallback_used"], false); | ||
| assert!(decision["outcome_id"].is_string()); | ||
| let response = execution.result.expect("target call should succeed"); | ||
| assert_eq!(response["object"], "response"); | ||
| assert_eq!(response["model"], "target/model"); | ||
| } | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn failed_answer_keeps_routing_outcome_in_error_mark() { | ||
| let server = MockServer::start().await; | ||
| Mock::given(method("POST")) | ||
| .and(path("/v1/chat/completions")) | ||
| .respond_with(ResponseTemplate::new(503).set_body_string("unavailable")) | ||
| .expect(2) | ||
| .mount(&server) | ||
| .await; | ||
| let deployment = json!({ | ||
| "schema_version": 1, | ||
| "llm_clients": { | ||
| "local": { | ||
| "format": "openai_chat", | ||
| "base_url": format!("{}/v1", server.uri()), | ||
| "max_retries": 0, | ||
| } | ||
| }, | ||
| "targets": { | ||
| "strong": {"id": "poc/strong", "llm_client": "local"}, | ||
| "weak": {"id": "poc/weak", "llm_client": "local"}, | ||
| }, | ||
| "routes": { | ||
| "stage": { | ||
| "id": "switchyard/stage", | ||
| "type": "stage_router", | ||
| "capable_target": "strong", | ||
| "efficient_target": "weak", | ||
| "picker": "efficient_first", | ||
| "confidence_threshold": 0.5, | ||
| } | ||
| }, | ||
| }); | ||
| let runtime = SwitchyardRuntime::new(crate::config::SwitchyardConfig { | ||
| priority: 0, | ||
| switchyard_config_path: None, | ||
| switchyard_config: Some( | ||
| deployment | ||
| .as_object() | ||
| .expect("deployment should be an object") | ||
| .clone(), | ||
| ), | ||
| }) | ||
| .expect("stage runtime should load"); | ||
| let request = runtime | ||
| .decode_request( | ||
| WireFormat::OpenAiChat, | ||
| RelayRequest { | ||
| headers: Map::new(), | ||
| content: json!({ | ||
| "model": "switchyard/stage", | ||
| "messages": [{"role": "user", "content": "hello"}], | ||
| }), | ||
| }, | ||
| false, | ||
| ) | ||
| .expect("request should decode"); | ||
|
|
||
| let execution = runtime | ||
| .execute_buffered(WireFormat::OpenAiChat, request) | ||
| .await; | ||
|
|
||
| assert!(execution.result.is_err()); | ||
| let error = execution | ||
| .events | ||
| .iter() | ||
| .find_map(|event| match event { | ||
| RoutingEvent::Mark(mark) if mark.name == "switchyard.routing.error" => { | ||
| Some(&mark.data) | ||
| } | ||
| RoutingEvent::Mark(_) | RoutingEvent::Metric(_) => None, | ||
| }) | ||
| .expect("error mark should be emitted"); | ||
| assert!(error["outcome_id"].is_string()); | ||
| assert_eq!(error["evidence"], json!({"source": "fall_open"})); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn buffered_responses_restore_codex_tool_namespaces() { | ||
| let server = MockServer::start().await; | ||
|
|
@@ -1019,6 +1155,7 @@ mod tests { | |
| let mark = route_execution_error_mark( | ||
| &error.execution_error_summary(), | ||
| json!({"session_id": "session"}), | ||
| Map::new(), | ||
| ); | ||
|
|
||
| assert_eq!(mark.name, "switchyard.routing.error"); | ||
|
|
@@ -1033,6 +1170,33 @@ mod tests { | |
| assert!(!mark.data.to_string().contains(secret)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn decision_evidence_keeps_only_documented_typed_fields() { | ||
| let evidence = evidence_for_mark(Some(json!({ | ||
| "source": "llm-classifier", | ||
| "score": 0.9, | ||
| "confidence": "wrong type", | ||
| "threshold": 0.5, | ||
| "verdict": "continue", | ||
| "trigger": "turn", | ||
| "reason_code": "test", | ||
| "unknown": "patient name is Jane Doe", | ||
| "prompt": {"secret": "do not export"}, | ||
| }))); | ||
|
|
||
| assert_eq!( | ||
| evidence, | ||
| Some(json!({ | ||
| "source": "llm-classifier", | ||
| "score": 0.9, | ||
| "threshold": 0.5, | ||
| "verdict": "continue", | ||
| "trigger": "turn", | ||
| "reason_code": "test", | ||
| })) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn routing_observations_emit_debug_marks_and_metrics() { | ||
| let runtime = runtime_for("switchyard"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an enum-level doc comment for
RunObservation.This change expands a public enum. Document the enum intent and its observation-delivery invariant.
Proposed fix
+/// Events emitted by [`run`] for completed routing and model-call activity. pub enum RunObservation {As per coding guidelines, “add concise comments for ... public structs/enums.”
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Coding guidelines