Merge pull request #729 from gsvigruha/gergely.svigruha/annotation-raw-http-fix

fix(llm-obs): use raw HTTP for annotation schema and annotation commands

Author: platinummonkey

src/client.rs

@@ -301,7 +301,7 @@ static UNSTABLE_OPS: &[&str] = &[
     "v2.delete_aws_cloud_auth_persona_mapping",
     "v2.get_aws_cloud_auth_persona_mapping",
     "v2.list_aws_cloud_auth_persona_mappings",
-    // LLM Observability (25)
+    // LLM Observability (21)
     "v2.create_llm_obs_project",
     "v2.list_llm_obs_projects",
     "v2.create_llm_obs_experiment",
@@ -320,10 +320,6 @@ static UNSTABLE_OPS: &[&str] = &[
     "v2.create_llm_obs_annotation_queue_interactions",
     "v2.delete_llm_obs_annotation_queue_interactions",
     "v2.get_llm_obs_annotated_interactions",
-    "v2.get_llm_obs_annotation_queue_label_schema",
-    "v2.update_llm_obs_annotation_queue_label_schema",
-    "v2.upsert_llm_obs_annotations",
-    "v2.delete_llm_obs_annotations",
     "v2.get_llm_obs_custom_eval_config",
     "v2.update_llm_obs_custom_eval_config",
     "v2.delete_llm_obs_custom_eval_config",
@@ -520,7 +516,7 @@ mod tests {
 
     #[test]
     fn test_unstable_ops_count() {
-        assert_eq!(UNSTABLE_OPS.len(), 190);
+        assert_eq!(UNSTABLE_OPS.len(), 186);
     }
 
     #[test]

src/commands/llm_obs.rs

@@ -3,11 +3,10 @@ use datadog_api_client::datadogV2::api_llm_observability::{
     LLMObservabilityAPI, ListLLMObsAnnotationQueuesOptionalParams,
 };
 use datadog_api_client::datadogV2::model::{
-    LLMObsAnnotationQueueInteractionsRequest, LLMObsAnnotationQueueLabelSchemaUpdateRequest,
-    LLMObsAnnotationQueueRequest, LLMObsAnnotationQueueUpdateRequest, LLMObsAnnotationsRequest,
-    LLMObsCustomEvalConfigUpdateRequest, LLMObsDatasetBatchUpdateRequest,
-    LLMObsDatasetCloneRequest, LLMObsDatasetRequest, LLMObsDatasetRestoreVersionRequest,
-    LLMObsDeleteAnnotationQueueInteractionsRequest, LLMObsDeleteAnnotationsRequest,
+    LLMObsAnnotationQueueInteractionsRequest, LLMObsAnnotationQueueRequest,
+    LLMObsAnnotationQueueUpdateRequest, LLMObsCustomEvalConfigUpdateRequest,
+    LLMObsDatasetBatchUpdateRequest, LLMObsDatasetCloneRequest, LLMObsDatasetRequest,
+    LLMObsDatasetRestoreVersionRequest, LLMObsDeleteAnnotationQueueInteractionsRequest,
     LLMObsDeleteExperimentsRequest, LLMObsProjectRequest,
 };
 
@@ -597,52 +596,62 @@ pub async fn annotation_queue_interactions_list(cfg: &Config, queue_id: &str) ->
     formatter::output(cfg, &resp)
 }
 
+/// Uses the raw client rather than the typed one: a queue with no schema yet answers with
+/// `"annotation_schema": null`, but the generated client models it as a non-`Option`
+/// `LLMObsAnnotationSchema` and fails with "invalid type: null, expected a mapping". Most queues
+/// have no schema, so the typed path errors on the common case. The request shape is unchanged.
 pub async fn annotation_queue_schema_get(cfg: &Config, queue_id: &str) -> Result<()> {
-    let api = make_api(cfg);
-    let resp = api
-        .get_llm_obs_annotation_queue_label_schema(queue_id.to_string())
+    let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/label-schema");
+    let resp = raw_client::raw_get(cfg, &path, &[])
         .await
         .map_err(|e| anyhow::anyhow!("failed to get annotation queue label schema: {e:?}"))?;
     formatter::output(cfg, &resp)
 }
 
+/// Raw client for symmetry with [`annotation_queue_schema_get`], so a round-trip of get → edit →
+/// update never straddles two response representations.
 pub async fn annotation_queue_schema_update(
     cfg: &Config,
     queue_id: &str,
     file: &str,
 ) -> Result<()> {
-    let body: LLMObsAnnotationQueueLabelSchemaUpdateRequest = util::read_json_file(file)?;
-    let api = make_api(cfg);
-    let resp = api
-        .update_llm_obs_annotation_queue_label_schema(queue_id.to_string(), body)
+    let body: serde_json::Value = util::read_json_file(file)?;
+    let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/label-schema");
+    let resp = raw_client::raw_put(cfg, &path, body)
         .await
         .map_err(|e| anyhow::anyhow!("failed to update annotation queue label schema: {e:?}"))?;
     formatter::output(cfg, &resp)
 }
 
+/// Uses the raw client rather than the typed one: this endpoint reports per-item failures with
+/// **HTTP 200** and `"annotations": null` alongside a populated `errors` array. The generated
+/// client models `annotations` as a non-`Option` `Vec` and fails with "invalid type: null,
+/// expected a sequence", turning a readable partial-failure report into an opaque serde error.
+/// The request shape is unchanged.
 pub async fn annotation_queue_annotations_upsert(
     cfg: &Config,
     queue_id: &str,
     file: &str,
 ) -> Result<()> {
-    let body: LLMObsAnnotationsRequest = util::read_json_file(file)?;
-    let api = make_api(cfg);
-    let resp = api
-        .upsert_llm_obs_annotations(queue_id.to_string(), body)
+    let body: serde_json::Value = util::read_json_file(file)?;
+    let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/annotations");
+    let resp = raw_client::raw_post(cfg, &path, body)
         .await
         .map_err(|e| anyhow::anyhow!("failed to upsert annotations: {e:?}"))?;
     formatter::output(cfg, &resp)
 }
 
+/// Raw client for the same reason as [`annotation_queue_annotations_upsert`]: partial failures come
+/// back as HTTP 200, and both `annotation_ids` and `errors` are non-`Option` `Vec`s in the
+/// generated model, so a `null` in either field would surface as a serde error.
 pub async fn annotation_queue_annotations_delete(
     cfg: &Config,
     queue_id: &str,
     file: &str,
 ) -> Result<()> {
-    let body: LLMObsDeleteAnnotationsRequest = util::read_json_file(file)?;
-    let api = make_api(cfg);
-    let resp = api
-        .delete_llm_obs_annotations(queue_id.to_string(), body)
+    let body: serde_json::Value = util::read_json_file(file)?;
+    let path = format!("/api/v2/llm-obs/v1/annotation-queues/{queue_id}/annotations/delete");
+    let resp = raw_client::raw_post(cfg, &path, body)
         .await
         .map_err(|e| anyhow::anyhow!("failed to delete annotations: {e:?}"))?;
     formatter::output(cfg, &resp)
@@ -5016,8 +5025,33 @@ mod tests {
 
     // ---- Annotation queue label schemas ----
 
+    // Shapes below are captured from real API responses, not hand-written: the typed SDK models
+    // declare `annotation_schema`, `annotations` and `annotation_ids` as non-`Option`, but the API
+    // returns `null` for them in ordinary cases (queue with no schema; per-item write failures).
     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}]}}}}"#;
 
+    /// A queue that has never had a schema set — the common case.
+    const LABEL_SCHEMA_NULL_BODY: &str =
+        r#"{"data":{"id":"queue-1","type":"queues","attributes":{"annotation_schema":null}}}"#;
+
+    #[tokio::test]
+    async fn test_annotation_queue_schema_get_null_schema() {
+        let _lock = lock_env().await;
+        let mut server = mockito::Server::new_async().await;
+        let cfg = test_config(&server.url());
+        let _mock = mock_any(&mut server, "GET", LABEL_SCHEMA_NULL_BODY).await;
+
+        // Regression: the typed client rejected this with
+        // "invalid type: null, expected a mapping".
+        let result = super::annotation_queue_schema_get(&cfg, "queue-1").await;
+        assert!(
+            result.is_ok(),
+            "schema_get must tolerate a null annotation_schema: {:?}",
+            result.err()
+        );
+        cleanup_env();
+    }
+
     #[tokio::test]
     async fn test_annotation_queue_schema_get() {
         let _lock = lock_env().await;
@@ -5089,8 +5123,7 @@ mod tests {
         let server = mockito::Server::new_async().await;
         let cfg = test_config(&server.url());
 
-        // Valid JSON but missing the required `data` member.
-        let path = write_temp_json("pup_aq_schema_bad.json", r#"{"nope":true}"#);
+        let path = write_temp_json("pup_aq_schema_bad.json", r#"{"data": }"#);
         let result =
             super::annotation_queue_schema_update(&cfg, "queue-1", path.to_str().unwrap()).await;
         let err = result.expect_err("should reject bad body").to_string();
@@ -5099,6 +5132,30 @@ mod tests {
         cleanup_env();
     }
 
+    /// A body the API rejects (rather than one serde rejects): the raw path forwards it, so the
+    /// error must come back from the server instead of being caught locally.
+    #[tokio::test]
+    async fn test_annotation_queue_schema_update_rejected_body() {
+        let _lock = lock_env().await;
+        let mut server = mockito::Server::new_async().await;
+        let cfg = test_config(&server.url());
+        let _mock = server
+            .mock("PUT", mockito::Matcher::Any)
+            .match_query(mockito::Matcher::Any)
+            .with_status(400)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"errors":[{"detail":"data is required"}]}"#)
+            .create_async()
+            .await;
+
+        let path = write_temp_json("pup_aq_schema_nodata.json", r#"{"nope":true}"#);
+        let result =
+            super::annotation_queue_schema_update(&cfg, "queue-1", path.to_str().unwrap()).await;
+        assert!(result.is_err(), "should surface the API's 400");
+        let _ = std::fs::remove_file(&path);
+        cleanup_env();
+    }
+
     // ---- Annotations on queue interactions ----
 
     #[tokio::test]
@@ -5121,6 +5178,57 @@ mod tests {
         cleanup_env();
     }
 
+    #[tokio::test]
+    async fn test_annotation_queue_annotations_upsert_null_annotations() {
+        let _lock = lock_env().await;
+        let mut server = mockito::Server::new_async().await;
+        let cfg = test_config(&server.url());
+        // Real partial-failure shape: HTTP 200, `annotations` null, `errors` populated.
+        let body = r#"{"data":{"id":"queue-1","type":"annotations","attributes":{"annotations":null,"errors":[{"interaction_id":"i-9","error":"interaction not found: i-9"}]}}}"#;
+        let _mock = mock_any(&mut server, "POST", body).await;
+
+        let path = write_temp_json(
+            "pup_aq_annotations_upsert_null.json",
+            r#"{"data":{"type":"annotations","attributes":{"annotations":[{"interaction_id":"i-9","label_values":[{"label_schema_id":"ls-1","value":3.0}]}]}}}"#,
+        );
+        // Regression: the typed client rejected this with
+        // "invalid type: null, expected a sequence", hiding the per-item error report.
+        let result =
+            super::annotation_queue_annotations_upsert(&cfg, "queue-1", path.to_str().unwrap())
+                .await;
+        assert!(
+            result.is_ok(),
+            "upsert must surface a 200 partial-failure report: {:?}",
+            result.err()
+        );
+        let _ = std::fs::remove_file(&path);
+        cleanup_env();
+    }
+
+    #[tokio::test]
+    async fn test_annotation_queue_annotations_delete_null_ids() {
+        let _lock = lock_env().await;
+        let mut server = mockito::Server::new_async().await;
+        let cfg = test_config(&server.url());
+        let body = r#"{"data":{"id":"queue-1","type":"annotations","attributes":{"annotation_ids":null,"errors":[{"annotation_id":"a-9","error":"annotation not found: a-9"}]}}}"#;
+        let _mock = mock_any(&mut server, "POST", body).await;
+
+        let path = write_temp_json(
+            "pup_aq_annotations_delete_null.json",
+            r#"{"data":{"type":"annotations","attributes":{"annotation_ids":["a-9"]}}}"#,
+        );
+        let result =
+            super::annotation_queue_annotations_delete(&cfg, "queue-1", path.to_str().unwrap())
+                .await;
+        assert!(
+            result.is_ok(),
+            "delete must tolerate null annotation_ids: {:?}",
+            result.err()
+        );
+        let _ = std::fs::remove_file(&path);
+        cleanup_env();
+    }
+
     #[tokio::test]
     async fn test_annotation_queue_annotations_upsert_400() {
         let _lock = lock_env().await;