Skip to content

PR #62 follow-up 14: signal get_trace_activity truncation at 500 rows - #71

Merged
VibeSWE merged 4 commits into
mainfrom
fix/pr62-14-trace-activity-truncation
Oct 24, 2026
Merged

VibeSWE merged 4 commits into
mainfrom
fix/pr62-14-trace-activity-truncation

Conversation

@VibeSWE

@VibeSWE VibeSWE commented Oct 24, 2026 •

Copy link
Copy Markdown

Stacked on #62 (base upstream/tool-catalog-1831). Retarget to main after #62 merges.

Problem

get_trace_activity orders rows by bucket time, oldest first, and capped them at 500, both in the generated SQL (ORDER BY 1 LIMIT 500) and in the generic bounded_tool_sql wrapper. When groupBy cardinality is high (e.g. 20 services × 60 buckets ≈ 1200 rows), the newest buckets were dropped and nothing in the response said so. A caller, especially an LLM, reads that as traffic falling to zero.

Fix

  • The generated query now uses LIMIT MAX_TOOL_ROWS + 1 (501).
  • After validate_tool_sql, the query runs as-is. It already carries its own LIMIT, and the 500-row wrapper would hide the probe row. As a side effect, the returned query is now exactly the SQL that ran.
  • truncate_trace_activity_rows caps the rows at 500 and returns whether any were dropped. The response gains "truncated": bool.
  • docs/tool-api.md documents the ordering, LIMIT 501, truncated, and how to get the full series (reduce bucketCount, groupBy or the time range).

Tests

  • New trace_activity_signals_truncation_past_the_row_limit: a DataFusion fixture with 20 services × 60 one-minute buckets (1200 groups×buckets) runs the generated SQL the way the tool does. It asserts truncated == true, 500 rows, and that the last kept bucket is 00:24, i.e. the newest buckets are the ones dropped. It also checks that an ungrouped 60-row series is not flagged, plus the exactly-500 and records-object edge cases. With the old LIMIT 500 this test fails by construction: at most 500 rows come back, so truncate_trace_activity_rows returns false. (A local negative-control build was OOM-killed under machine load, so this was not observed empirically.)
  • The existing fixture tests now run the SQL through the same validated, unwrapped path the tool uses.
  • cargo test --lib tool_: 39 passed. cargo fmt --check is clean, and cargo clippy --all-targets adds no warnings in touched files.

Upstream fidelity

This is a small divergence from upstream 32979cb in src/tool_executor.rs (the query LIMIT, one helper, one response field) plus docs. It's additive for callers (a new truncated field), and the same fix is drafted for upstream on the review wiki page. Possible overlap: the item 04 docs wording, and items 15/21 in the same function.

🤖 Generated with Claude Code

VibeSWE and others added 3 commits October 24, 2026 14:30
…ilently

get_trace_activity ordered rows by bucket time and capped them at 500,
both in the generated SQL and in the generic tool wrapper. With high
groupBy cardinality (e.g. 20 services x 60 buckets) the newest buckets
were cut off with no indication, which reads as traffic dropping to zero.

The generated query now fetches MAX_TOOL_ROWS + 1 rows and runs as-is
after validation (it already carries its own LIMIT), so the tool can
return at most 500 rows plus a `truncated` flag. The returned `query`
is now exactly the SQL that ran.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… fix/pr62-14-trace-activity-truncation

# Conflicts:
#	docs/tool-api.md
#	src/tool_executor.rs
@VibeSWE
VibeSWE changed the base branch from upstream/tool-catalog-1831 to main October 24, 2026 21:53
@VibeSWE
VibeSWE merged commit 9d83d93 into main Oct 24, 2026
9 checks passed
@VibeSWE
VibeSWE deleted the fix/pr62-14-trace-activity-truncation branch October 24, 2026 22:15
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