Skip to content

fix(intake): performance improvements for trace list - #1002

Merged
BrianNewsom merged 2 commits into
mainfrom
brnewsom/fix-ch-traces-query
Jul 30, 2026
Merged

fix(intake): performance improvements for trace list#1002
BrianNewsom merged 2 commits into
mainfrom
brnewsom/fix-ch-traces-query

Conversation

@BrianNewsom

@BrianNewsom BrianNewsom commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Use a two phase query to hydrate the trace list view instead of the current single query approach, which requires reading all input/output data across spans.

Summary by CodeRabbit

  • Bug Fixes

    • Improved trace listing pagination and multi-step loading to keep results consistent across modes.
    • Added reconciliation when hydration omits items, including warning logs and corrected total counts.
    • Refined preview payload handling to ensure proper truncation without impacting detailed views.
    • Empty preview pages now avoid unnecessary follow-up loading.
  • Tests

    • Expanded assertions for preview and detailed query behavior, including aggregation expressions and payload formatting.
    • Added cases for empty pages and reconciliation when hydrated results miss requested refs.

Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
@BrianNewsom
BrianNewsom requested review from a team as code owners July 30, 2026 21:00
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2918b72a-a4a1-4d41-8ee5-d8a1fb846169

📥 Commits

Reviewing files that changed from the base of the PR and between 4b541a5 and 27ea1b3.

📒 Files selected for processing (2)
  • services/intake/src/nmp/intake/repository/clickhouse/trace.py
  • services/intake/tests/test_traces_clickhouse_repository.py

📝 Walkthrough

Walkthrough

Non-summary trace listing now retrieves page roots first, hydrates selected traces through page-scoped ClickHouse queries, reconciles missing hydrated rows, logs dropped references, and adjusts totals. Tests cover preview, detailed, empty-page, ordering, and query-shape behavior.

Changes

Trace page hydration

Layer / File(s) Summary
Staged page retrieval and reconciliation
services/intake/src/nmp/intake/repository/clickhouse/trace.py, services/intake/tests/test_traces_clickhouse_repository.py
Non-summary listing creates page references, skips hydration for empty pages, restores page order after hydration, logs missing references, and adjusts pagination totals.
Mode-specific root projections
services/intake/src/nmp/intake/repository/clickhouse/trace.py, services/intake/tests/test_traces_clickhouse_repository.py
Root SELECT columns and mode-specific parameters are centralized; preview truncation and detailed span reconstruction are validated.
Page-scoped span aggregation
services/intake/src/nmp/intake/repository/clickhouse/trace.py
Hydration composes page-root and aggregate subqueries, reconstructs span fields, counts spans, and selects unique model/provider attributes.

Sequence Diagram(s)

sequenceDiagram
  participant ClickHouseTraceRepository
  participant ClickHouseClient
  participant PageRootQuery
  ClickHouseTraceRepository->>ClickHouseClient: Execute page-root query
  ClickHouseClient->>PageRootQuery: Select page trace roots
  PageRootQuery-->>ClickHouseClient: Return page references
  ClickHouseClient-->>ClickHouseTraceRepository: Return page rows
  ClickHouseTraceRepository->>ClickHouseClient: Execute page-scoped hydration query
  ClickHouseClient-->>ClickHouseTraceRepository: Return hydrated rows
  ClickHouseTraceRepository->>ClickHouseTraceRepository: Reconcile order and dropped refs
Loading

Possibly related PRs

Suggested labels: refactor

Suggested reviewers: shanaiabuggy, a2bondar, aahunt-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: speeding up trace list retrieval in intake.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch brnewsom/fix-ch-traces-query

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/src/nmp/intake/repository/clickhouse/trace.py`:
- Around line 135-165: Reconcile hydration results with the page references in
the trace listing flow: after the `traces.list.hydrate` query, detect page refs
that were not returned because concurrent filtering or deletion excluded them,
and explicitly track or log those dropped refs rather than silently shrinking
the page. Keep `total_results` and pagination behavior consistent with the
chosen reconciliation policy.
🪄 Autofix (Beta)

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: 2fab7cf1-b5b9-4294-a510-d69eb6cb9940

📥 Commits

Reviewing files that changed from the base of the PR and between 9c36aac and 4b541a5.

📒 Files selected for processing (2)
  • services/intake/src/nmp/intake/repository/clickhouse/trace.py
  • services/intake/tests/test_traces_clickhouse_repository.py

Comment thread services/intake/src/nmp/intake/repository/clickhouse/trace.py
@github-actions github-actions Bot added the fix label Jul 30, 2026
Comment thread services/intake/src/nmp/intake/repository/clickhouse/trace.py
Comment thread services/intake/src/nmp/intake/repository/clickhouse/trace.py
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 28406/36247 78.4% 62.8%
Integration Tests 16988/34965 48.6% 21.1%

@BrianNewsom
BrianNewsom added this pull request to the merge queue Jul 30, 2026
Merged via the queue into main with commit 8a47285 Jul 30, 2026
60 checks passed
@BrianNewsom
BrianNewsom deleted the brnewsom/fix-ch-traces-query branch July 30, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants