fix(intake): stop publishing span filters that answer HTTP 500 - #1225
Merged
Conversation
Filtering spans by dataset_id, dataset_name, dataset_version, prompt_name, or prompt_version returned HTTP 500 with the body "An unexpected error occurred." ATTRIBUTE_EQ_FILTER_FIELDS routed all five to the span attribute catalog, and SpanAttributeField has no entry for any of them, so spec_for_field raised ValueError while the SQL was built. Nothing caught it, so a bad request read as a server fault. Remove them rather than add catalog entries. SpanSemanticAttributes has no dataset or prompt-name field, so Intake never ingests, stores, or returns this data. A catalog entry would return zero rows for every query, and silently empty results are harder to diagnose than a rejected field. Derive ATTRIBUTE_EQ_FILTER_FIELDS from the catalog instead of listing it by hand. The list and the catalog had no enforced relationship, which is what let them drift. Derived, a SpanFilter field with no catalog entry falls through to the existing 400 instead of raising. The published description advertised prompt_name and prompt_version, so it was sending callers at the broken fields. It now names only the fifteen served fields and says which operators they take. Add test_spans_filter_contract.py, which walks filter JSON to SQL for every published field. Verified it fails when any of the five is republished. Follow-up: the SDK and its generated CLI commands still expose the five. They regenerate from this spec through Stainless, which needs credentials I do not have here: run make update-sdk, then make generate-cli-commands. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (11)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughUpdated the span filter contract across OpenAPI schemas, Intake routing, CLI commands, analyst documentation, and tests. Removed dataset and prompt filters. Added contract tests for routing, catalog coverage, SQL generation, and unpublished fields. ChangesSpan filter contract
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openapi/ga/individual/platform.openapi.yaml (1)
17988-17990: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRegenerate the SDK and CLI filters for the removed
SpanFilterfields.
dataset_id,dataset_name,dataset_version,prompt_name, andprompt_versionare absent from all three OpenAPI artifacts but remain exposed by the Python SDK and CLI. Intake rejects these filters with HTTP 400. Runmake update-sdkand confirm that the fields are absent from all generated clients and CLI copies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/ga/individual/platform.openapi.yaml` around lines 17988 - 17990, Regenerate the SDK and CLI using make update-sdk based on the OpenAPI definitions in openapi/ga/individual/platform.openapi.yaml lines 17988-17990, openapi/ga/openapi.yaml lines 17990-17991, and openapi/openapi.yaml line 17991; ensure dataset_id, dataset_name, dataset_version, prompt_name, and prompt_version are removed from all generated Python SDK clients and CLI filter copies, while retaining the supported evaluation-id filter.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/intake/tests/test_spans_filter_contract.py`:
- Around line 26-30: Update the FILTER_VALUES fixture’s "status" entry to use
the valid SpanStatus value "ok" (or the corresponding enum member) instead of
"success", while leaving the other filter values unchanged.
---
Outside diff comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 17988-17990: Regenerate the SDK and CLI using make update-sdk
based on the OpenAPI definitions in openapi/ga/individual/platform.openapi.yaml
lines 17988-17990, openapi/ga/openapi.yaml lines 17990-17991, and
openapi/openapi.yaml line 17991; ensure dataset_id, dataset_name,
dataset_version, prompt_name, and prompt_version are removed from all generated
Python SDK clients and CLI filter copies, while retaining the supported
evaluation-id filter.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cff08208-f8e4-455c-9fbc-6add581f08e1
📒 Files selected for processing (8)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlplugins/nemo-insights/src/nemo_insights_plugin/analyst/agent.pyplugins/nemo-insights/src/nemo_insights_plugin/analyst/functions/spans.pyservices/intake/src/nmp/intake/spans/api/spans.pyservices/intake/src/nmp/intake/spans/api/spans_schemas.pyservices/intake/tests/test_spans_filter_contract.py
💤 Files with no reviewable changes (1)
- services/intake/src/nmp/intake/spans/api/spans_schemas.py
Contributor
|
BrianNewsom
reviewed
Aug 11, 2026
BrianNewsom
reviewed
Aug 11, 2026
6 tasks
Removing five fields from SpanFilter changed the published contract, but the checked-in Python SDK and CLI still offered them, so the repo shipped two different answers to the same question. Regenerating drops them from SpanFilterParam, the spans resource, both CLIs, and the reference docs. The SDK filter type now carries exactly the fifteen fields a live Intake serves. The Analyst docstrings advertised status values of "ok" and "error". There is no "ok": SpanStatus is success, error, cancelled, unknown, and a live Intake answers status=ok with "400 Filter field 'status' must be one of". An agent following those docs wrote a query that could not succeed. ATTRIBUTE_EQ_FILTER_FIELDS goes back to an explicit list. Deriving it by intersecting the schema with the storage catalog was hard to read and blurred a public contract into storage plumbing. Adding a name is now deliberate, and the contract tests supply the drift protection that deriving was standing in for, including a new test that no routed name has fallen out of the schema. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Contributor
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
BrianNewsom
approved these changes
Aug 11, 2026
aleckhoury
added a commit
that referenced
this pull request
Aug 12, 2026
Per review on #1212: the workaround should not need to exist, and it no longer does, so the notes describing it go too. The docstring no longer warns the agent off the five dataset and prompt span filters, and _explain no longer has a 500 branch naming them. #1225 unpublished those fields, so they read as unknown filter fields like any other typo, and the 400 branch already says where the real vocabulary lives. A 500 now falls through to the generic branch, which still names the status, since an Intake fault leaves the caller nothing to correct. The module docstring also still said span groups sort only by count, which #1242 changed and this branch already relies on. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five span filters returned HTTP 500 with the body
An unexpected error occurred.:ATTRIBUTE_EQ_FILTER_FIELDSinspans/api/spans.pyrouted all five to the span attribute catalog.SpanAttributeFieldhas no entry for any of them, sospec_for_fieldraisedValueErrorwhile the SQL was built, and nothing caught it. A bad request read as a server fault.Reproduced against a running platform before the fix:
This was reachable from the public API docs: the endpoint description named
prompt_nameandprompt_versionexplicitly.Why remove them instead of adding catalog entries
SpanSemanticAttributeshas no dataset or prompt-name field, so Intake never ingests, stores, or returns this data.SpanEvaluationContextexposes onlyevaluation_idandtest_case_id. A catalog entry would let the query run and return zero rows every time, and a silently empty result is harder to diagnose than a rejected field.Changes
spans_schemas.py: drop the five fields fromSpanFilter.spans.py:ATTRIBUTE_EQ_FILTER_FIELDSstays an explicit list of the eight.SpanFilteris the public contract and the catalog is storage plumbing, so adding a name should be a deliberate act rather than a set operation between the two. The contract tests below supply the drift protection.spans.py: the endpoint description now names only the fifteen served fields and says which operators they take.openapi/: regenerated. Pure deletion of the five properties plus the reworded description.make update-sdk.SpanFilterParam, the spans resource, both checked-in CLIs, anddocs/cli/reference.mdxno longer offer the five fields.SpanFilterParamnow carries exactly the fifteen fields a live Intake serves.plugins/nemo-insights: the Analyst told its model thatdataset_nameandevaluation_run_idwere valid span filters. The first returned 500 and the second is not a field at all. It also advertised a status value of"ok", which does not exist:SpanStatusissuccess,error,cancelled,unknown, and a live Intake answersstatus=okwith400 Filter field 'status' must be one of. All three corrected.Tests
services/intake/tests/test_spans_filter_contract.pywalks the chain that broke — filter JSON, then the API filter builder, then the SQL builder — for every fieldSpanFilterpublishes. It holds both directions of the contract: no published field is unroutable, and no routed name has fallen out of the schema.I confirmed the guard works by temporarily republishing
dataset_name, which fails three tests with:Test plan
uv run --frozen pytest services/intake/tests --ignore=services/intake/tests/integration— 277 passeduv run --frozen pytest services/intake/tests/integration/spans— 108 passeduv run ruff check— cleanuv run --frozen ty check services/intake— 7 diagnostics, none in changed filesmake update-sdk— five fields gone, diff scoped to intake spans with no unrelated generator driftsuccess200,error200,ok400nemo-insightstest failures confirmed identical onmainRelated
Found while adding Intake trace query tools to Eval Author in #1212, which had inherited the same wrong filter vocabulary in its docstrings.
Summary by CodeRabbit
Changes
started_atsupportsgteandltecomparisons.Tests