Skip to content

PR #62 follow-up 15: apply exact time bounds in get_trace_activity - #72

Merged
VibeSWE merged 5 commits into
mainfrom
fix/pr62-15-trace-activity-exact-range
Oct 24, 2026
Merged

VibeSWE merged 5 commits into
mainfrom
fix/pr62-15-trace-activity-exact-range

Conversation

@VibeSWE

@VibeSWE VibeSWE commented Oct 24, 2026 •

Copy link
Copy Markdown

Follow-up item 15 from the PR #62 review (upstream-inherited code from parseablehq/parseable 32979cb, "update: tool catalog (parseablehq#1831)").

Problem

get_trace_activity passed the caller's raw startTime/endTime to query_internal, which builds its scan range with TimeRange::parse_human_time and rounds both bounds down to the minute. The generated SQL had no time predicate of its own, so:

  • a sub-minute range such as 10:30:10Z–10:30:50Z collapsed to the empty window [10:30:00, 10:30:00) and returned no rows;
  • a range crossing a minute boundary scanned [floor(start), floor(end)), which included rows before startTime and dropped rows between floor(endTime) and endTime;
  • time_bucket_interval sized buckets from the rounded range (falling back to 3600s on a parse error), and DATE_BIN was aligned to the epoch, so the first and last buckets were partial.

Fix

This follows the fork's list_log_errors approach (85358df, log_error_query_request):

  • trace_activity_range resolves startTime/endTime once (relative expressions share one now) and rejects start >= end and unparsable bounds.
  • trace_activity_query adds exact half-open predicates (time >= start AND time < end) to the SQL and aligns DATE_BIN to the requested start.
  • trace_activity_request sends a minute-widened transport envelope, [floor(start), floor(end) + 1m), using checked arithmetic, so the generic query API's rounding can no longer drop rows. It validates the SQL with validate_tool_sql and keeps the query's own LIMIT 501, which item 14 (PR #62 follow-up 14: signal get_trace_activity truncation at 500 rows #71) uses to detect truncation.
  • time_bucket_interval sizes buckets from the exact range.

docs/tool-api.md now documents the exact half-open bounds, the start < end requirement, the start-aligned buckets, and the fact that the returned SQL embeds the exact bounds.

Tests

  • New trace_activity_subminute_range_returns_exact_rows runs the tool's request through DataFusion. It first applies the query API's minute-rounded transport filter to the fixture, then the generated SQL. It covers a sub-minute range, the inclusive start and exclusive end, and a range crossing a minute boundary, and checks bucket labels aligned to the start.
  • New trace_activity_request_validates_exact_ranges_and_widens_transport covers resolving relative bounds once, preserving microsecond precision in the SQL, the transport envelope, and rejecting empty, inverted and invalid ranges.
  • The existing trace activity fixture now goes through the same request path.
  • Both new tests fail when the upstream behaviour is restored (no SQL time predicates, no widening, epoch origin) and pass with the fix. cargo test --lib tool_ passes (40 tests), cargo clippy -- -D warnings is clean, and cargo clippy --all-targets adds no warnings in touched files.

Upstream fidelity

The diff is confined to the trace activity helpers in src/tool_executor.rs, their tests, and the docs section. trace_activity_query and time_bucket_interval gain a range parameter, and there are two new small helpers. The bug exists upstream too, and an upstream proposal is drafted separately; nothing has been filed upstream.

Stacking

This was opened stacked on #62. #62 has merged, so this PR now targets main and has main merged in, including item 14 (#71), item 17 (#73) and item 20 (#69). Item 14's truncation test now passes an explicit range.

🤖 Generated with Claude Code

VibeSWE and others added 3 commits October 24, 2026 14:45
The generic query API rounds startTime/endTime down to the minute, so a
sub-minute trace activity range collapsed to an empty window, and ranges
crossing a minute boundary included rows outside the requested bounds.

Resolve the bounds once, reject start >= end, put exact half-open
predicates in the SQL, widen the transport envelope to whole minutes
(as list_log_errors does), size buckets from the exact range and align
DATE_BIN to the requested start.

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

# 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 22:15
…ivity-exact-range

# Conflicts:
#	docs/tool-api.md
#	src/tool_executor.rs
@VibeSWE
VibeSWE merged commit 3c62307 into main Oct 24, 2026
9 checks passed
@VibeSWE
VibeSWE deleted the fix/pr62-15-trace-activity-exact-range branch October 24, 2026 22:43
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