Add new aggregation API#667
Conversation
hadarishav
left a comment
There was a problem hiding this comment.
Well-scoped PR and the content is solid — the conditional aggregated_score / value shapes by output_type are explained clearly. A few things to align with our API-docs conventions before merge (see inline comments). The nav/sidebar wiring and auth/param types all look correct.
| <ResponseField name="eval_aggregation" type="object"> | ||
| Per-eval rollup. Present only when `eval_aggregation=true`. Keys are `CustomEvalConfig` names; values are one rollup object per eval. | ||
| </ResponseField> | ||
| <ApiCollapsible title="Show eval rollup properties"> |
There was a problem hiding this comment.
<ApiCollapsible> is a sibling of the eval_aggregation ResponseField (which closed on line 78), so it renders as a standalone collapsible instead of nested under eval_aggregation. Move it inside the parent ResponseField:
<ResponseField name="eval_aggregation" type="object">
Per-eval rollup. Present only when `eval_aggregation=true`...
<ApiCollapsible title="Show eval rollup properties">
...
</ApiCollapsible>
</ResponseField>(Same pattern we standardized on the agent-versions pages.)
| <ResponseField name="span_aggregation" type="object"> | ||
| Per-span pivot. Present only when `span_aggregation=true`. Outer keys are `span_id` (one per span the task evaluated); inner keys are eval names; inner values are one entry per eval that touched the span. | ||
| </ResponseField> | ||
| <ApiCollapsible title="Show span entry properties"> |
There was a problem hiding this comment.
Same nesting issue here — this <ApiCollapsible> should be nested inside the span_aggregation ResponseField rather than sitting as a sibling.
| </ApiSection> | ||
|
|
||
| <ApiSection title="Behavior notes"> | ||
| <ParamField name="At least one flag required" type="Behavior"> |
There was a problem hiding this comment.
This "Behavior notes" section fakes a parameter type (<ParamField type="Behavior">) to render notes — no other API page does this. Use <Note> blocks instead, and fold the "at least one flag required" point into the query-param descriptions since it's a request constraint.
| </ParamField> | ||
| </ApiSection> | ||
|
|
||
| <ApiSection title="Errors"> |
There was a problem hiding this comment.
Errors section has 400 + 401 but no 500 — every other API page includes a 500. Add it unless this endpoint genuinely never 500s.
Pull Request
Description
Describe the changes in this pull request:
Checklist
Related Issues
Closes #<issue_number>
##Linear
Fixes TH-5466