Skip to content

fix: rilltime syntax in comparison time range in citation urls#9478

Merged
AdityaHegde merged 15 commits into
mainfrom
fix/rill-time-comparison-time-in-citation-urls
Jun 2, 2026
Merged

fix: rilltime syntax in comparison time range in citation urls#9478
AdityaHegde merged 15 commits into
mainfrom
fix/rill-time-comparison-time-in-citation-urls

Conversation

@AdityaHegde
Copy link
Copy Markdown
Collaborator

@AdityaHegde AdityaHegde commented May 21, 2026

LLM can use rill time for relative time ranges. Since backend supports it in comparison time range as well, it can use it in comparison as well. But UI doesnt support rill time, leading to removal of comparison all together.

Since watermark history at the time of query is not available, we are opting to resolve the time ranges and store it in the tool result.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@AdityaHegde AdityaHegde force-pushed the fix/rill-time-comparison-time-in-citation-urls branch 3 times, most recently from d8044db to 4124b7c Compare May 21, 2026 11:27
@AdityaHegde AdityaHegde marked this pull request as ready for review May 27, 2026 04:58
Comment on lines +282 to +286
// Resolve time ranges and store them in the result to record the exact resolved time ranges for this tool call
resolvedTimeRanges, err := t.resolveTimeRanges(ctx, session, args)
if err != nil {
return nil, err
}
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.

To avoid duplicate logic and execution (timestamps may not always be cached), can we return the time ranges from the metrics resolver (you can use ResolverResult.Meta() map for them), and in turn push it into the executor (which already has the necessary logic, and the timestamps binding ensures no duplicate queries)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok updated to return the resolved time ranges from metrics resolver.

Copy link
Copy Markdown
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

LGTM

@AdityaHegde AdityaHegde merged commit 6a86894 into main Jun 2, 2026
18 of 19 checks passed
@AdityaHegde AdityaHegde deleted the fix/rill-time-comparison-time-in-citation-urls branch June 2, 2026 05:24
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.

2 participants