Skip to content

Commit 8129bf4

Browse files
tillwfclaude
andcommitted
auto-experiment: name the root cause of pup's write exit codes, and the pending fix
The exit-code warning said pup's experiment writes "fail while deserializing the API's response" without saying why, which makes it impossible to tell whether an installed build is affected. Both causes are now stated, each confirmed against the live API: * `experiments update` — a successful PATCH answers HTTP 200 with a ZERO-BYTE body, which the generated typed client hands to serde_json::from_str, failing with "EOF while parsing a value". * `experiments create` — the 200 response omits `config`, a field the generated model requires, giving "missing field config". Neither is a request failure. In one run this fired four times and all four writes had applied. Also records that DataDog/pup#682 fixes both, by routing these two writes through pup's raw client (as every other llm-obs command already does) and making parse_response_json treat an empty successful body as JSON null. With that build update exits 0 and prints {"experiment_id": ..., "status": "updated"}, and create exits 0 returning the new id. That PR is OPEN, NOT MERGED, and the skill says so rather than describing unreleased behaviour as current. The detection advice is deliberately not "check the version": run the command and compare the exit code against a read-back, the same discipline the rest of this file uses — a version number would not have caught the --help probe that reported records-all present on a binary that lacked it. The setup gate keeps read-back unconditionally, since it is correct on both builds and avoids branching on which one is installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 88283c2 commit 8129bf4

1 file changed

Lines changed: 27 additions & 12 deletions

File tree

  • agent-observability/agent-observability-auto-experiment

agent-observability/agent-observability-auto-experiment/SKILL.md

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ backends are not strictly comparable.
290290
| span field inventory | `get_llmobs_span_details` | `spans get-details --trace-id T --span-ids S` ||
291291
| span content (`messages`) | `get_llmobs_span_content` | `spans get-content --trace-id T --span-id S --field messages` ||
292292
| expand a trace's spans | `expand_llmobs_spans` | `spans expand --trace-id T --span-ids S` ||
293-
| record run context / status | `update_llmobs_experiment` | `experiments update --file body.json <EXPERIMENT_ID>` | ⚠️ |
293+
| record run context / status | `update_llmobs_experiment` | `experiments update --file body.json <EXPERIMENT_ID>` | ⚠️ |
294294
| submit an iteration's score | `submit_llmobs_experiment_events` | `experiments events submit --metrics '[{…}]' <EXPERIMENT_ID>` | |
295295

296296
Every pup row is prefixed `pup llm-obs` and every one was **run successfully against pup 1.8.0**
@@ -299,8 +299,10 @@ there are no unsupported purposes. Two markers:
299299
-**use this to load the eval corpus.** Both backends must read the SAME records or the run's
300300
scores are not comparable to a run on the other backend; see **Loading the whole dataset** below.
301301
-**pass an explicit `--from`/`--to`.** These default to a 1-hour window; see below.
302-
- ⚠️ **exits non-zero even when the write succeeds.** Verify by reading state back, not by exit
303-
code; see the call mechanics below.
302+
- ⚠️† **on released pup, exits non-zero even when the write succeeds.** Verify by reading state
303+
back, not by exit code. Fixed by DataDog/pup#682**open, not merged at time of writing**, so
304+
assume the broken behaviour until you have confirmed otherwise on the installed build; see the
305+
call mechanics below.
304306

305307
### ★ Loading the whole dataset — same records on both backends
306308

@@ -387,14 +389,27 @@ appears to fail while succeeding:
387389
`pup llm-obs experiments events submit --metrics '[{…}]' <EXPERIMENT_ID>` — the metrics array is
388390
passed inline and the `experiment_id` key the MCP tool wants is omitted. `experiments update` still
389391
takes `--file <path> <EXPERIMENT_ID>`.
390-
- ⚠️ **A non-zero pup exit does NOT mean the write failed.** `experiments create` and
391-
`experiments update` currently fail while *deserializing the API's response* (`missing field
392-
config`, and `EOF while parsing a value` for update's empty body) and exit non-zero **after the
393-
write has already landed** — both were confirmed applied by reading the experiment back. So for
394-
pup writes, **verify by reading state back, never by exit code**; treating exit 1 as failure will
395-
send you into a spurious retry loop that double-writes. `experiments events submit` is well behaved
396-
(exit 0, and it returns the same `{experiment_id, metrics_ingested, status}` shape as MCP), so the
397-
per-iteration score submission can still be confirmed the normal way.
392+
- ⚠️ **A non-zero pup exit does NOT mean the write failed (on released pup).**
393+
`experiments create` and `experiments update` fail while *deserializing the API's response* and
394+
exit non-zero **after the write has already landed**. Root causes, both confirmed against the live
395+
API: `update`'s successful PATCH answers **HTTP 200 with a zero-byte body**, which the generated
396+
typed client feeds to `serde_json::from_str` and fails on with `EOF while parsing a value`; and
397+
`create`'s 200 response **omits `config`**, a field the generated model requires, giving
398+
`missing field config`. Neither is a request failure. In one run this fired four times and all
399+
four writes had applied.
400+
401+
So for pup writes on released pup, **verify by reading state back, never by exit code** — treating
402+
exit 1 as failure sends you into a retry loop that double-writes. `experiments events submit` is
403+
unaffected (exit 0, same `{experiment_id, metrics_ingested, status}` shape as MCP), so the
404+
per-iteration score submission can be confirmed the normal way.
405+
406+
**DataDog/pup#682 fixes both** by routing these two writes through pup's raw client (as every other
407+
`llm-obs` command already does) and by making `raw_client::parse_response_json` treat an empty
408+
successful body as JSON `null` rather than an error. With that build, `update` exits 0 and prints
409+
`{"experiment_id": …, "status": "updated"}`, and `create` exits 0 returning the new id. **That PR is
410+
open, not merged, at time of writing** — so do not assume it is present. Determine which behaviour
411+
you have the same way you determine anything else about the installed build: run the command and
412+
look at the exit code against a read-back, rather than trusting a version number or this file.
398413
- `experiments create` additionally requires `data.attributes.project_id` (it uses the typed v2 route),
399414
which the `unstable` REST route does not. The skill never creates an experiment — the id is an
400415
input — so this only matters if you are provisioning one by hand.
@@ -481,7 +496,7 @@ ran because you intended it to.
481496
| 3 | `config.json` written | file exists with every required field populated (incl. the resolved `files_to_optimize` list, `evaluators` verbatim, data source) |
482497
| 4 | experiment id | `$experiment-id` validated as a UUID at the intake gate and persisted to `config.json` as `dd_auto_experiment_id` |
483498
| 5 | run context on experiment | confirm the `update_llmobs_experiment` call (or `pup llm-obs experiments update`) **actually returned a success response in hand** (not merely that you intended to call it). For the us5 MCP that response is `updated_fields` containing `"metadata"` — accept that, or any non-error response acknowledging the metadata write if the tool's shape differs. The check is "the call was made and acknowledged", so do not hard-block on one exact field name; if it errored or was never called, re-run it. |
484-
| 6 | backend reachable | with `datadog_backend: pup`, `pup auth status` (or `$PUP_BIN auth status`) returned `authenticated: true` for the expected site — run the check, don't assume the binary works. A missing or unauthenticated pup is a **STOP**, not a fallback (see **Datadog backend**). With `datadog_backend: mcp`, step 5's acknowledged response is itself the proof the backend is reachable. Record `backend_used` in `config.json` either way. **Under pup, satisfy step 5 by reading the experiment back** (`pup llm-obs experiments list --filter-project-id …` and confirm the metadata/status you just wrote), because `experiments update` exits non-zero on a response-parsing bug even when the write landed an exit-code check would fail a step that actually succeeded. |
499+
| 6 | backend reachable | with `datadog_backend: pup`, `pup auth status` (or `$PUP_BIN auth status`) returned `authenticated: true` for the expected site — run the check, don't assume the binary works. A missing or unauthenticated pup is a **STOP**, not a fallback (see **Datadog backend**). With `datadog_backend: mcp`, step 5's acknowledged response is itself the proof the backend is reachable. Record `backend_used` in `config.json` either way. **Under pup, satisfy step 5 by reading the experiment back** (`pup llm-obs experiments list --filter-project-id …` and confirm the metadata/status you just wrote). On released pup `experiments update` exits non-zero on a response-parsing bug even when the write landed, so an exit-code check would fail a step that actually succeeded; DataDog/pup#682 fixes that but is not merged yet. Read-back is correct either way, so use it unconditionally rather than branching on the build. |
485500

486501
State the gate result briefly (each step ✓ with its evidence) before Step 1. This same
487502
"external-effect step → verify against an artifact" discipline is why per-iteration score

0 commit comments

Comments
 (0)