Skip to content

Commit

Permalink
Provide error-enriched access to both FnResult fields
Browse files Browse the repository at this point in the history
  • Loading branch information
imalsogreg committed Oct 23, 2024
1 parent a1ade48 commit 0f18524
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 50 deletions.
6 changes: 3 additions & 3 deletions engine/baml-runtime/src/cli/serve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ Tip: test that the server is up using `curl http://localhost:{}/_debug/ping`

match result {
Ok(function_result) => match function_result.llm_response() {
LLMResponse::Success(_) => match function_result.parsed_content() {
LLMResponse::Success(_) => match function_result.result_with_constraints_content() {
// Just because the LLM returned 2xx doesn't mean that it returned parse-able content!
Ok(parsed) => {
(StatusCode::OK, Json::<ResponseBamlValue>(parsed.clone())).into_response()
Expand Down Expand Up @@ -478,7 +478,7 @@ Tip: test that the server is up using `curl http://localhost:{}/_debug/ping`

match result {
Ok(function_result) => match function_result.llm_response() {
LLMResponse::Success(_) => match function_result.parsed_content() {
LLMResponse::Success(_) => match function_result.result_with_constraints_content() {
// Just because the LLM returned 2xx doesn't mean that it returned parse-able content!
Ok(parsed) => {
(StatusCode::OK, Json::<ResponseBamlValue>(parsed.clone()))
Expand Down Expand Up @@ -645,7 +645,7 @@ impl Stream for EventStream {
cx: &mut std::task::Context<'_>,
) -> Poll<Option<Self::Item>> {
match self.receiver.poll_recv(cx) {
Poll::Ready(Some(item)) => match item.parsed_content() {
Poll::Ready(Some(item)) => match item.result_with_constraints_content() {
// TODO: not sure if this is the correct way to implement this.
Ok(parsed) => Poll::Ready(Some(parsed.into())),
Err(_) => Poll::Pending,
Expand Down
10 changes: 5 additions & 5 deletions engine/baml-runtime/src/tracing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<'a> Visualize for FunctionResult {
));
}
s.push(self.llm_response().visualize(max_chunk_size));
match self.parsed() {
match self.result_with_constraints() {
Some(Ok(val)) => {
let val: BamlValue = val.into();
s.push(format!(
Expand Down Expand Up @@ -292,7 +292,7 @@ impl BamlTracer {

if let Ok(response) = &response {
let name = event_chain.last().map(|s| s.name.as_str());
let is_ok = response.parsed().as_ref().is_some_and(|r| r.is_ok());
let is_ok = response.result_with_constraints().as_ref().is_some_and(|r| r.is_ok());
log::log!(
target: "baml_events",
if is_ok { log::Level::Info } else { log::Level::Warn },
Expand Down Expand Up @@ -339,7 +339,7 @@ impl BamlTracer {

if let Ok(response) = &response {
let name = event_chain.last().map(|s| s.name.as_str());
let is_ok = response.parsed().as_ref().is_some_and(|r| r.is_ok());
let is_ok = response.result_with_constraints().as_ref().is_some_and(|r| r.is_ok());
log::log!(
target: "baml_events",
if is_ok { log::Level::Info } else { log::Level::Warn },
Expand Down Expand Up @@ -461,7 +461,7 @@ impl From<&BamlValue> for IOValue {
}

fn error_from_result(result: &FunctionResult) -> Option<api_wrapper::core_types::Error> {
match result.parsed() {
match result.result_with_constraints() {
Some(Ok(_)) => None,
Some(Err(e)) => Some(api_wrapper::core_types::Error {
code: 2,
Expand Down Expand Up @@ -607,7 +607,7 @@ impl ToLogSchema for FunctionResult {
io: IO {
input: Some((&span.params).into()),
output: self
.parsed()
.result_with_constraints()
.as_ref()
.map(|r| r.as_ref().ok())
.flatten()
Expand Down
107 changes: 71 additions & 36 deletions engine/baml-runtime/src/types/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl std::fmt::Display for FunctionResult {
)?;
}
writeln!(f, "{}", self.llm_response())?;
match &self.parsed() {
match &self.result_with_constraints() {
Some(Ok(val)) => {
writeln!(
f,
Expand Down Expand Up @@ -95,51 +95,86 @@ impl FunctionResult {
&self.event_chain.last().unwrap().0
}

pub fn parsed(&self) -> &Option<Result<ResponseBamlValue>> {
pub fn parsed(&self) -> &Option<Result<BamlValueWithFlags>> {
&self.event_chain.last().unwrap().2
}

/// Get the parsed result. This logic is strange because parsing errors can
/// be forwarded to a different field in the orchestrator.
/// TODO: (Greg) Fix the strange logic.
/// Historical note: Most of the consumers of the orchestrator use a final
/// `ResponseBamlValue`, a type designed to hold only the information needed
/// in those responses. But one consumer, the wasm client, requires extra info
/// from the parsing stage. Therefore we preserve both the parsing stage data
/// and the `ResponseValue` side by side. And because `anyhow::Error` is not
/// `Clone`, errors from the parsing stage are handled the most easily by
/// migrating them to the `ResponseValue` in cases where parsing failed.
/// The proper solution is to create a `RuntimeBamlValue` that contains
/// enough information for all clients, and then types like
/// `SDKClientResponseBamlValue` and `WasmResponseBamlValue` which derive
/// from `RuntimeBamlValue` where needed.
pub fn parsed_content(&self) -> Result<&BamlValueWithFlags> {
match (self.parsed(), self.result_with_constraints()) {
// Error at parse time was forwarded to later result.
(None, Some(Err(e))) => Err(self.format_err(e)),
// Parsing succeeded.
(Some(Ok(v)), _) => Ok(v),
// Error at parse time was not forwarded to later results.
(Some(Err(e)), _) => Err(self.format_err(e)),
(None, None) => Err(anyhow::anyhow!(self.llm_response().clone())),
(None, Some(_)) => unreachable!("A response could not have been created without a successful parse")
}
}

pub fn result_with_constraints(&self) -> &Option<Result<ResponseBamlValue>> {
&self.event_chain.last().unwrap().3
}

pub fn parsed_content(&self) -> Result<&ResponseBamlValue> {
self.parsed()
pub fn result_with_constraints_content(&self) -> Result<&ResponseBamlValue> {
self.result_with_constraints()
.as_ref()
.map(|res| {
if let Ok(val) = res {
Ok(val)
} else {
// Capture the actual error to preserve its details
let actual_error = res.as_ref().err().unwrap().to_string();
Err(anyhow::anyhow!(ExposedError::ValidationError {
prompt: match self.llm_response() {
LLMResponse::Success(resp) => resp.prompt.to_string(),
LLMResponse::LLMFailure(err) => err.prompt.to_string(),
_ => "N/A".to_string(),
},
raw_output: self
.llm_response()
.content()
.unwrap_or_default()
.to_string(),
// The only branch that should be hit is LLMResponse::Success(_) since we
// only call this function when we have a successful response.
message: match self.llm_response() {
LLMResponse::Success(_) =>
format!("Failed to parse LLM response: {}", actual_error),
LLMResponse::LLMFailure(err) => format!(
"LLM Failure: {} ({}) - {}",
err.message,
err.code.to_string(),
actual_error
),
LLMResponse::UserFailure(err) =>
format!("User Failure: {} - {}", err, actual_error),
LLMResponse::InternalFailure(err) =>
format!("Internal Failure: {} - {}", err, actual_error),
},
}))
Err(self.format_err( res.as_ref().err().unwrap() ))
}
})
.unwrap_or_else(|| Err(anyhow::anyhow!(self.llm_response().clone())))
}

fn format_err(&self, err: &anyhow::Error) -> anyhow::Error {
// Capture the actual error to preserve its details
let actual_error = err.to_string();
anyhow::anyhow!(ExposedError::ValidationError {
prompt: match self.llm_response() {
LLMResponse::Success(resp) => resp.prompt.to_string(),
LLMResponse::LLMFailure(err) => err.prompt.to_string(),
_ => "N/A".to_string(),
},
raw_output: self
.llm_response()
.content()
.unwrap_or_default()
.to_string(),
// The only branch that should be hit is LLMResponse::Success(_) since we
// only call this function when we have a successful response.
message: match self.llm_response() {
LLMResponse::Success(_) =>
format!("Failed to parse LLM response: {}", actual_error),
LLMResponse::LLMFailure(err) => format!(
"LLM Failure: {} ({}) - {}",
err.message,
err.code.to_string(),
actual_error
),
LLMResponse::UserFailure(err) =>
format!("User Failure: {} - {}", err, actual_error),
LLMResponse::InternalFailure(err) =>
format!("Internal Failure: {} - {}", err, actual_error),
},
})
}
}

pub struct TestResponse {
Expand Down Expand Up @@ -193,7 +228,7 @@ impl Eq for TestFailReason<'_> {}
impl TestResponse {
pub fn status(&self) -> TestStatus {
let func_res = &self.function_response;
if let Some(parsed) = func_res.parsed() {
if let Some(parsed) = func_res.result_with_constraints() {
if parsed.is_ok() {
TestStatus::Pass
} else {
Expand All @@ -214,7 +249,7 @@ use std::process::Termination;
#[cfg(test)]
impl Termination for FunctionResult {
fn report(self) -> std::process::ExitCode {
if self.parsed_content().is_ok() {
if self.result_with_constraints_content().is_ok() {
std::process::ExitCode::SUCCESS
} else {
std::process::ExitCode::FAILURE
Expand Down
2 changes: 1 addition & 1 deletion engine/baml-schema-wasm/src/runtime_wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ impl WasmLLMResponse {
impl WasmFunctionResponse {
pub fn parsed_response(&self) -> Option<String> {
self.function_response
.parsed_content()
.result_with_constraints_content()
.map(|p| serde_json::to_string(&BamlValue::from(p)))
.map_or_else(|_| None, |s| s.ok())
}
Expand Down
4 changes: 2 additions & 2 deletions engine/language_client_python/src/types/function_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl FunctionResult {
}

fn is_ok(&self) -> bool {
self.inner.parsed_content().is_ok()
self.inner.result_with_constraints_content().is_ok()
}

/// This is a debug function that returns the internal representation of the response
Expand All @@ -35,7 +35,7 @@ impl FunctionResult {
) -> PyResult<PyObject> {
let parsed = self
.inner
.parsed_content()
.result_with_constraints_content()
.map_err(BamlError::from_anyhow)?;

let parsed = pythonize_strict(py, parsed.clone(), &enum_module, &cls_module)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl FunctionResult {
rb_self: &FunctionResult,
types: RModule,
) -> Result<Value> {
match rb_self.inner.parsed_content() {
match rb_self.inner.result_with_constraints_content() {
Ok(parsed) => {
ruby_to_json::RubyToJson::serialize_baml(ruby, types, parsed.clone())
.map_err(|e| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ impl FunctionResult {

#[napi]
pub fn is_ok(&self) -> bool {
self.inner.parsed_content().is_ok()
self.inner.result_with_constraints_content().is_ok()
}

#[napi]
pub fn parsed(&self) -> napi::Result<serde_json::Value> {
let parsed = self
.inner
.parsed_content()
.result_with_constraints_content()
.map_err(|e| from_anyhow_error(e))?;

Ok(serde_json::to_value(parsed)?)
Expand Down
1 change: 1 addition & 0 deletions shell.nix
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ in pkgs.mkShell {
nixfmt-classic
swc
lld_18
wasm-pack
] ++ (if pkgs.stdenv.isDarwin then appleDeps else [ ]);

LIBCLANG_PATH = pkgs.libclang.lib + "/lib/";
Expand Down

0 comments on commit 0f18524

Please sign in to comment.