Skip to content

Commit a6e95b2

Browse files
gsvigruhaclaude
andcommitted
fix(llm-obs): use raw HTTP for annotation schema and annotation commands
Integration testing against the real API showed three of the four new commands failing on ordinary responses. The generated client declares `annotation_schema`, `annotations` and `annotation_ids` as non-`Option`, but the API returns `null` for each in common cases: - `schema get` on a queue with no schema yet answers `"annotation_schema": null` → "invalid type: null, expected a mapping". Most queues have no schema, so this was the common case, not an edge. - `annotations upsert` reports per-item failures with HTTP 200 and `"annotations": null` beside a populated `errors` array → "invalid type: null, expected a sequence", turning a readable partial-failure report into an opaque serde error. Switch all four to the raw client, matching the existing precedent in this file (see `experiments_update`). Request shapes and paths are unchanged. - Route the four commands through raw_client (src/commands/llm_obs.rs) - Revert the now-unneeded UNSTABLE_OPS entries (src/client.rs) - Add regression tests pinning the null-response shapes, captured from real responses rather than hand-written (src/commands/llm_obs.rs) Verified against the real API: all four commands succeed, including the schema-less queue and the 200 partial-failure report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 296f3b4 commit a6e95b2

2 files changed

Lines changed: 132 additions & 28 deletions

File tree

src/client.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ static UNSTABLE_OPS: &[&str] = &[
301301
"v2.delete_aws_cloud_auth_persona_mapping",
302302
"v2.get_aws_cloud_auth_persona_mapping",
303303
"v2.list_aws_cloud_auth_persona_mappings",
304-
// LLM Observability (25)
304+
// LLM Observability (21)
305305
"v2.create_llm_obs_project",
306306
"v2.list_llm_obs_projects",
307307
"v2.create_llm_obs_experiment",
@@ -320,10 +320,6 @@ static UNSTABLE_OPS: &[&str] = &[
320320
"v2.create_llm_obs_annotation_queue_interactions",
321321
"v2.delete_llm_obs_annotation_queue_interactions",
322322
"v2.get_llm_obs_annotated_interactions",
323-
"v2.get_llm_obs_annotation_queue_label_schema",
324-
"v2.update_llm_obs_annotation_queue_label_schema",
325-
"v2.upsert_llm_obs_annotations",
326-
"v2.delete_llm_obs_annotations",
327323
"v2.get_llm_obs_custom_eval_config",
328324
"v2.update_llm_obs_custom_eval_config",
329325
"v2.delete_llm_obs_custom_eval_config",
@@ -520,7 +516,7 @@ mod tests {
520516

521517
#[test]
522518
fn test_unstable_ops_count() {
523-
assert_eq!(UNSTABLE_OPS.len(), 190);
519+
assert_eq!(UNSTABLE_OPS.len(), 186);
524520
}
525521

526522
#[test]

src/commands/llm_obs.rs

Lines changed: 130 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@ use datadog_api_client::datadogV2::api_llm_observability::{
33
LLMObservabilityAPI, ListLLMObsAnnotationQueuesOptionalParams,
44
};
55
use datadog_api_client::datadogV2::model::{
6-
LLMObsAnnotationQueueInteractionsRequest, LLMObsAnnotationQueueLabelSchemaUpdateRequest,
7-
LLMObsAnnotationQueueRequest, LLMObsAnnotationQueueUpdateRequest, LLMObsAnnotationsRequest,
8-
LLMObsCustomEvalConfigUpdateRequest, LLMObsDatasetBatchUpdateRequest,
9-
LLMObsDatasetCloneRequest, LLMObsDatasetRequest, LLMObsDatasetRestoreVersionRequest,
10-
LLMObsDeleteAnnotationQueueInteractionsRequest, LLMObsDeleteAnnotationsRequest,
6+
LLMObsAnnotationQueueInteractionsRequest, LLMObsAnnotationQueueRequest,
7+
LLMObsAnnotationQueueUpdateRequest, LLMObsCustomEvalConfigUpdateRequest,
8+
LLMObsDatasetBatchUpdateRequest, LLMObsDatasetCloneRequest, LLMObsDatasetRequest,
9+
LLMObsDatasetRestoreVersionRequest, LLMObsDeleteAnnotationQueueInteractionsRequest,
1110
LLMObsDeleteExperimentsRequest, LLMObsProjectRequest,
1211
};
1312

@@ -597,52 +596,62 @@ pub async fn annotation_queue_interactions_list(cfg: &Config, queue_id: &str) ->
597596
formatter::output(cfg, &resp)
598597
}
599598

599+
/// Uses the raw client rather than the typed one: a queue with no schema yet answers with
600+
/// `"annotation_schema": null`, but the generated client models it as a non-`Option`
601+
/// `LLMObsAnnotationSchema` and fails with "invalid type: null, expected a mapping". Most queues
602+
/// have no schema, so the typed path errors on the common case. The request shape is unchanged.
600603
pub async fn annotation_queue_schema_get(cfg: &Config, queue_id: &str) -> Result<()> {
601-
let api = make_api(cfg);
602-
let resp = api
603-
.get_llm_obs_annotation_queue_label_schema(queue_id.to_string())
604+
let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/label-schema");
605+
let resp = raw_client::raw_get(cfg, &path, &[])
604606
.await
605607
.map_err(|e| anyhow::anyhow!("failed to get annotation queue label schema: {e:?}"))?;
606608
formatter::output(cfg, &resp)
607609
}
608610

611+
/// Raw client for symmetry with [`annotation_queue_schema_get`], so a round-trip of get → edit →
612+
/// update never straddles two response representations.
609613
pub async fn annotation_queue_schema_update(
610614
cfg: &Config,
611615
queue_id: &str,
612616
file: &str,
613617
) -> Result<()> {
614-
let body: LLMObsAnnotationQueueLabelSchemaUpdateRequest = util::read_json_file(file)?;
615-
let api = make_api(cfg);
616-
let resp = api
617-
.update_llm_obs_annotation_queue_label_schema(queue_id.to_string(), body)
618+
let body: serde_json::Value = util::read_json_file(file)?;
619+
let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/label-schema");
620+
let resp = raw_client::raw_put(cfg, &path, body)
618621
.await
619622
.map_err(|e| anyhow::anyhow!("failed to update annotation queue label schema: {e:?}"))?;
620623
formatter::output(cfg, &resp)
621624
}
622625

626+
/// Uses the raw client rather than the typed one: this endpoint reports per-item failures with
627+
/// **HTTP 200** and `"annotations": null` alongside a populated `errors` array. The generated
628+
/// client models `annotations` as a non-`Option` `Vec` and fails with "invalid type: null,
629+
/// expected a sequence", turning a readable partial-failure report into an opaque serde error.
630+
/// The request shape is unchanged.
623631
pub async fn annotation_queue_annotations_upsert(
624632
cfg: &Config,
625633
queue_id: &str,
626634
file: &str,
627635
) -> Result<()> {
628-
let body: LLMObsAnnotationsRequest = util::read_json_file(file)?;
629-
let api = make_api(cfg);
630-
let resp = api
631-
.upsert_llm_obs_annotations(queue_id.to_string(), body)
636+
let body: serde_json::Value = util::read_json_file(file)?;
637+
let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/annotations");
638+
let resp = raw_client::raw_post(cfg, &path, body)
632639
.await
633640
.map_err(|e| anyhow::anyhow!("failed to upsert annotations: {e:?}"))?;
634641
formatter::output(cfg, &resp)
635642
}
636643

644+
/// Raw client for the same reason as [`annotation_queue_annotations_upsert`]: partial failures come
645+
/// back as HTTP 200, and both `annotation_ids` and `errors` are non-`Option` `Vec`s in the
646+
/// generated model, so a `null` in either field would surface as a serde error.
637647
pub async fn annotation_queue_annotations_delete(
638648
cfg: &Config,
639649
queue_id: &str,
640650
file: &str,
641651
) -> Result<()> {
642-
let body: LLMObsDeleteAnnotationsRequest = util::read_json_file(file)?;
643-
let api = make_api(cfg);
644-
let resp = api
645-
.delete_llm_obs_annotations(queue_id.to_string(), body)
652+
let body: serde_json::Value = util::read_json_file(file)?;
653+
let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/annotations/delete");
654+
let resp = raw_client::raw_post(cfg, &path, body)
646655
.await
647656
.map_err(|e| anyhow::anyhow!("failed to delete annotations: {e:?}"))?;
648657
formatter::output(cfg, &resp)
@@ -5016,8 +5025,33 @@ mod tests {
50165025

50175026
// ---- Annotation queue label schemas ----
50185027

5028+
// Shapes below are captured from real API responses, not hand-written: the typed SDK models
5029+
// declare `annotation_schema`, `annotations` and `annotation_ids` as non-`Option`, but the API
5030+
// returns `null` for them in ordinary cases (queue with no schema; per-item write failures).
50195031
const LABEL_SCHEMA_BODY: &str = r#"{"data":{"id":"queue-1","type":"queues","attributes":{"annotation_schema":{"label_schemas":[{"id":"ls-1","name":"quality","type":"score","min":0.0,"max":5.0,"is_required":true}]}}}}"#;
50205032

5033+
/// A queue that has never had a schema set — the common case.
5034+
const LABEL_SCHEMA_NULL_BODY: &str =
5035+
r#"{"data":{"id":"queue-1","type":"queues","attributes":{"annotation_schema":null}}}"#;
5036+
5037+
#[tokio::test]
5038+
async fn test_annotation_queue_schema_get_null_schema() {
5039+
let _lock = lock_env().await;
5040+
let mut server = mockito::Server::new_async().await;
5041+
let cfg = test_config(&server.url());
5042+
let _mock = mock_any(&mut server, "GET", LABEL_SCHEMA_NULL_BODY).await;
5043+
5044+
// Regression: the typed client rejected this with
5045+
// "invalid type: null, expected a mapping".
5046+
let result = super::annotation_queue_schema_get(&cfg, "queue-1").await;
5047+
assert!(
5048+
result.is_ok(),
5049+
"schema_get must tolerate a null annotation_schema: {:?}",
5050+
result.err()
5051+
);
5052+
cleanup_env();
5053+
}
5054+
50215055
#[tokio::test]
50225056
async fn test_annotation_queue_schema_get() {
50235057
let _lock = lock_env().await;
@@ -5089,8 +5123,7 @@ mod tests {
50895123
let server = mockito::Server::new_async().await;
50905124
let cfg = test_config(&server.url());
50915125

5092-
// Valid JSON but missing the required `data` member.
5093-
let path = write_temp_json("pup_aq_schema_bad.json", r#"{"nope":true}"#);
5126+
let path = write_temp_json("pup_aq_schema_bad.json", r#"{"data": }"#);
50945127
let result =
50955128
super::annotation_queue_schema_update(&cfg, "queue-1", path.to_str().unwrap()).await;
50965129
let err = result.expect_err("should reject bad body").to_string();
@@ -5099,6 +5132,30 @@ mod tests {
50995132
cleanup_env();
51005133
}
51015134

5135+
/// A body the API rejects (rather than one serde rejects): the raw path forwards it, so the
5136+
/// error must come back from the server instead of being caught locally.
5137+
#[tokio::test]
5138+
async fn test_annotation_queue_schema_update_rejected_body() {
5139+
let _lock = lock_env().await;
5140+
let mut server = mockito::Server::new_async().await;
5141+
let cfg = test_config(&server.url());
5142+
let _mock = server
5143+
.mock("PUT", mockito::Matcher::Any)
5144+
.match_query(mockito::Matcher::Any)
5145+
.with_status(400)
5146+
.with_header("content-type", "application/json")
5147+
.with_body(r#"{"errors":[{"detail":"data is required"}]}"#)
5148+
.create_async()
5149+
.await;
5150+
5151+
let path = write_temp_json("pup_aq_schema_nodata.json", r#"{"nope":true}"#);
5152+
let result =
5153+
super::annotation_queue_schema_update(&cfg, "queue-1", path.to_str().unwrap()).await;
5154+
assert!(result.is_err(), "should surface the API's 400");
5155+
let _ = std::fs::remove_file(&path);
5156+
cleanup_env();
5157+
}
5158+
51025159
// ---- Annotations on queue interactions ----
51035160

51045161
#[tokio::test]
@@ -5121,6 +5178,57 @@ mod tests {
51215178
cleanup_env();
51225179
}
51235180

5181+
#[tokio::test]
5182+
async fn test_annotation_queue_annotations_upsert_null_annotations() {
5183+
let _lock = lock_env().await;
5184+
let mut server = mockito::Server::new_async().await;
5185+
let cfg = test_config(&server.url());
5186+
// Real partial-failure shape: HTTP 200, `annotations` null, `errors` populated.
5187+
let body = r#"{"data":{"id":"queue-1","type":"annotations","attributes":{"annotations":null,"errors":[{"interaction_id":"i-9","error":"interaction not found: i-9"}]}}}"#;
5188+
let _mock = mock_any(&mut server, "POST", body).await;
5189+
5190+
let path = write_temp_json(
5191+
"pup_aq_annotations_upsert_null.json",
5192+
r#"{"data":{"type":"annotations","attributes":{"annotations":[{"interaction_id":"i-9","label_values":[{"label_schema_id":"ls-1","value":3.0}]}]}}}"#,
5193+
);
5194+
// Regression: the typed client rejected this with
5195+
// "invalid type: null, expected a sequence", hiding the per-item error report.
5196+
let result =
5197+
super::annotation_queue_annotations_upsert(&cfg, "queue-1", path.to_str().unwrap())
5198+
.await;
5199+
assert!(
5200+
result.is_ok(),
5201+
"upsert must surface a 200 partial-failure report: {:?}",
5202+
result.err()
5203+
);
5204+
let _ = std::fs::remove_file(&path);
5205+
cleanup_env();
5206+
}
5207+
5208+
#[tokio::test]
5209+
async fn test_annotation_queue_annotations_delete_null_ids() {
5210+
let _lock = lock_env().await;
5211+
let mut server = mockito::Server::new_async().await;
5212+
let cfg = test_config(&server.url());
5213+
let body = r#"{"data":{"id":"queue-1","type":"annotations","attributes":{"annotation_ids":null,"errors":[{"annotation_id":"a-9","error":"annotation not found: a-9"}]}}}"#;
5214+
let _mock = mock_any(&mut server, "POST", body).await;
5215+
5216+
let path = write_temp_json(
5217+
"pup_aq_annotations_delete_null.json",
5218+
r#"{"data":{"type":"annotations","attributes":{"annotation_ids":["a-9"]}}}"#,
5219+
);
5220+
let result =
5221+
super::annotation_queue_annotations_delete(&cfg, "queue-1", path.to_str().unwrap())
5222+
.await;
5223+
assert!(
5224+
result.is_ok(),
5225+
"delete must tolerate null annotation_ids: {:?}",
5226+
result.err()
5227+
);
5228+
let _ = std::fs::remove_file(&path);
5229+
cleanup_env();
5230+
}
5231+
51245232
#[tokio::test]
51255233
async fn test_annotation_queue_annotations_upsert_400() {
51265234
let _lock = lock_env().await;

0 commit comments

Comments
 (0)