Skip to content

refactor(traces): type the ClickHouse query help response - #44285

Merged
yujonglee-berri merged 3 commits into
mainfrom
litellm_query_help_typed_response
Oct 3, 2026
Merged

yujonglee-berri merged 3 commits into
mainfrom
litellm_query_help_typed_response

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Query help was assembled with json!, so key typos and wrong types compiled
  • Metadata kinds and attribute types were free strings on both sides of the bridge
  • A failed discovery could carry an error next to populated fields
  • Reader limits were hard-coded in three places, including guide prose

How it solves it:

  • query_help returns a QueryHelp struct; the bridge serializes it to a Python value
  • JsonKind, MapValueType and TraceTable enums replace string literals
  • Discovery::{Observed, Unavailable} makes the error and sample fields mutually exclusive
  • READ_LIMITS and READER_LIMITS feed the HTTP settings, the reader user and the Askama guide
  • Python models use Literal types and extra="forbid" so contract drift fails loudly

The serialized JSON shape is unchanged: discovery still flattens fields, truncated and the optional error into the catalog object, and Askama still renders the guide, examples and gotchas

User Flow

Before: an agent asking the proxy how to query traces gets a guide whose limits text could drift from what the reader enforces

  1. The agent calls GET https://litellm-domain/v1/traces/query/help with its key
  2. The response lists tables, metadata paths and a guide stating "1000 result rows, 4 MiB response bytes, 256 MiB memory and a 10 second query limit"
  3. If a limit changed in the reader, the guide kept the old numbers and the response still validated

After: the same call returns the same shape, and the guide numbers come from the limits the reader enforces

  1. The agent calls GET https://litellm-domain/v1/traces/query/help with its key
  2. The response is identical in shape, with the limits sentence rendered from the shared constants
  3. A malformed field from the native layer is rejected with a 5xx instead of reaching the agent

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

No LLM call is involved; this is a contract refactor of the trace query help response. Proof is the real ClickHouse integration run plus validating its output against the Python models

After (8c7e348)

  1. cargo test -p litellm-traces-clickhouse -p litellm-storage-clickhouse against a live ClickHouse container: all suites pass, including query_help_discovers_live_schema_and_runs_its_examples and query_help_preserves_schema_and_guide_when_discovery_hits_reader_limits
  2. The help payloads from both tests (discovery observed and discovery failing on reader limits) were dumped and fed to TypeAdapter(TraceQueryHelp).validate_python, and both validated with extra="forbid":
observed None [None, None] False
['The reader enforces 1000 result rows, 4 MiB response bytes, 256 MiB memory and a 10 second query limit; exceeding limits fails instead of returning partial results']
unavailable ClickHouse query failed with HTTP status 500 ['ClickHouse query failed with HTTP status 500', 'ClickHouse query failed with HTTP status 500'] True
['Metadata discovery unavailable: ClickHouse query failed with HTTP status 500', 'Attribute discovery unavailable: ClickHouse query failed with HTTP status 500', 'Attribute discovery unavailable: ClickHouse query failed with HTTP status 500', 'The reader enforces 1000 result rows, 4 MiB response bytes, 256 MiB memory and a 10 second query limit; exceeding limits fails instead of returning partial results']
  1. uv run pytest tests/unit/proxy/test_tracing_endpoints.py tests/unit/rust_bridge: 1229 passed
  2. c583c50 adds storage adapter tests that feed a native help value straight into ClickHouseStorage.query_help: a valid payload comes back as TraceQueryHelp, while a boolen metadata kind, a traces table name or an unknown top-level key each raise RuntimeError (46 passed in tests/unit/proxy/test_tracing_endpoints.py)

Type

🧹 Refactoring

Caveats (if any)

Medium

  • The guide drops the sampled-rows line when metadata discovery fails
    • The JSON still carries zeroed counts and truncated: true in that case
  • extra="forbid" means a new Rust field needs a matching Python field
  • main currently fails rust-test in crates/traces/tests/query/named.rs because the round-trip fixtures predate the agent_names, frameworks and framework fields; 54d1ce6 adds them so this branch is green

Link to Devin session: https://app.devin.ai/sessions/4c9710dc56ac43a99ec98515b219636d
Open in Devin Desktop: https://app.devin.ai/desktop/session/4c9710dc56ac43a99ec98515b219636d?variant=devin
Requested by: @yujonglee-berri

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Refactors query help response types and structure.

This PR appears safe to merge, with a non-blocking request for direct tests of the new Python response check

Findings

  1. P2 Response checks lack direct tests ▶

Summary

This PR replaces the query-help JSON string with a typed Rust response, adds matching Python literals, and shares reader limits with the guide

  • The response fields and current limits remain unchanged
  • Add direct tests for the new Python response check
  • devin-ai-integration[bot] explicitly acknowledged that failed discovery drops the sampled-rows guide line and that new Rust fields require matching Python fields. Neither is reported as a defect

Reviews (1) · Last reviewed commit: "refactor(traces): type the ClickHouse qu..."

Comment thread litellm/rust_bridge/traces.py
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_query_help_typed_response (c583c50) with main (9e31afb)

Open in CodSpeed

yujonglee-berri and others added 2 commits October 3, 2026 00:13
…und trips

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant