[PLAT-416] ensure bounded time range for mv query tool call by pjain1 · Pull Request #8887 · rilldata/rill

pjain1 · 2026-02-19T16:00:16Z

To prevent LLMs to run unbounded queries

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!

begelundmuller

Two thoughts:

It might be nice to be able to disable this for projects that run into problems. So maybe add it as a rill.ai.name instance-level flag? So it can be overridden at runtime if someone has problems.
I'm wondering if the AI will just get the full time range using the summary tool call and then use that, which will have similar bad performance. Do you think it would make sense to instead support a bounding by a number of days, e.g. rill.ai.max_time_range_days? So e.g. users with large clusters could bound AI queries to max 30 days of data.

pjain1 · 2026-02-21T09:49:19Z

IMO we should definitely ensure that a valid time range is set, absence of it just means LLM is not able to some how use it, may be this can be also behind a flag and default is ensure time range, what do you think? I agree we can have rill.ai.max_time_range_days and have a default of 0 means no restriction as some projects having less data might want to actually use full time range and in those cases LLM can specify the full time range retrieved from summary tool call.

pjain1 · 2026-02-23T14:04:41Z

runtime/metricsview/executor/executor_enforce_ai_limits.go

+		return nil
+	}
+
+	if qry.TimeRange.IsZero() {


put this behind feature flag?

I'm okay with keeping it required for AI initially – but try running the evals in analyst_agent_test.go and check they still pass

They pass (has to disable enforcing of time range for tests directly doing query_metrics_view tool call) except from CanvasContext test which fails here and I think its a genuine failure as I am not sure why app_or_site = 'App' this filter would be present in where clause.

Then it's flaky. The filter is because the contextual canvas component has it:

rill/runtime/testruntime/projects.go

Line 215 in dae94e6

dimension_filters: app_or_site IN ('App')

runtime/ai/metrics_view_query.go

runtime/metricsview/executor/executor.go

begelundmuller · 2026-02-26T12:09:46Z

runtime/ai/metrics_view_query.go

- Prefer using absolute 'start' and 'end' times in 'time_range' and 'comparison_time_range' if available. 
-  Otherwise, use 'expression' for relative time ranges, when specifying 'expression' make sure no other time range fields should be set as its not supported. 
+- Prefer using absolute 'start' and 'end' time range parameters in 'time_range' and 'comparison_time_range' if available. 
+  Otherwise, use 'time_range.expression' for relative time ranges, when specifying 'time_range.expression' make sure no other time range fields other than 'time_range.time_dimension' is set as its not supported.


The problem with this is that there is also comparison_time_range.expression (and expressions are usually most useful for comparisons)

runtime/drivers/registry.go

runtime/metricsview/executor/executor_enforce_query_limits.go

begelundmuller · 2026-02-26T12:14:09Z

runtime/metricsview/query.go

+
+	QueryLimit *QueryLimits `json:"-" mapstructure:"query_limits"`


Is there a reason not to expose this?

Suggested change

QueryLimit *QueryLimits `json:"-" mapstructure:"query_limits"`

QueryLimit *QueryLimits `json:"limits,omitempty" mapstructure:"query_limits"`

Yes, this will be used only during AI tool calls and resolvers use mapstructure to decode the args, also query limits are not exposed through any proto requests so didn't felt need to expose it.

I think we should just expose it. I'm not sure, but I think we may have some places that use JSON instead of mapstructure for serialization/deserialization, or if not, we might add that in the future. So since it would not be harmful to expose, I think it might just be simpler/safer.

ok why use limits as key instead of query_limits, seems confusing with already present limit key.

added json tags

Was just typing quickly, query_limits also seems fine by me

begelundmuller · 2026-02-26T12:15:44Z

runtime/metricsview/query.go

+// QueryLimits represents limits that should be applied to a query, such as requiring a time range or limiting the maximum time range for interactive queries. These are not part of the Query json itself because they are not intrinsic to the query, but rather are constraints that may be applied to the query before execution.
+type QueryLimits struct {
+	RequireTimeRange bool `json:"-" mapstructure:"require_time_range"`
+	MaxTimeRangeDays int  `json:"-" mapstructure:"max_time_range_days"`
+}


Same question about exposing it.

Is it a problem to consider it as part of the query? From the metricsview package perspective, I guess it is part of the query? It is only from the API/tool perspective that it may overwrite it (i.e. not allow the user to set it).

runtime/ai/metrics_view_query.go

begelundmuller

LGTM!

ensure bounded time range for mv query tool call

1c5df84

pjain1 requested a review from begelundmuller February 19, 2026 16:00

begelundmuller reviewed Feb 19, 2026

View reviewed changes

pjain1 changed the title ~~ensure bounded time range for mv query tool call~~ [PLAT-416] ensure bounded time range for mv query tool call Feb 20, 2026

enforce max days

058b9f7

pjain1 requested a review from begelundmuller February 23, 2026 04:53

pjain1 commented Feb 23, 2026

View reviewed changes

begelundmuller requested changes Feb 23, 2026

View reviewed changes

runtime/ai/metrics_view_query.go Outdated Show resolved Hide resolved

runtime/metricsview/executor/executor.go Outdated Show resolved Hide resolved

pjain1 added 3 commits February 24, 2026 13:06

review comment

d6c076a

Merge branch 'main' into defensive_tool_call

7f1c8c8

review comment

b250817

pjain1 requested a review from begelundmuller February 24, 2026 11:06

comment

35fef4a

begelundmuller requested changes Feb 26, 2026

View reviewed changes

pjain1 added 2 commits February 26, 2026 18:49

review comments

624f9a8

name

f5bb22a

pjain1 requested a review from begelundmuller February 26, 2026 13:30

support json serde for QueryLimits

2f9a486

begelundmuller approved these changes Feb 27, 2026

View reviewed changes

begelundmuller merged commit 5f88a72 into main Feb 27, 2026
18 of 19 checks passed

begelundmuller deleted the defensive_tool_call branch February 27, 2026 16:16


		QueryLimit *QueryLimits `json:"-" mapstructure:"query_limits"`


	QueryLimit *QueryLimits `json:"-" mapstructure:"query_limits"`

	QueryLimit *QueryLimits `json:"limits,omitempty" mapstructure:"query_limits"`

Conversation

pjain1 commented Feb 19, 2026

Uh oh!

begelundmuller left a comment

Choose a reason for hiding this comment

Uh oh!

pjain1 commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjain1 Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjain1 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjain1 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

begelundmuller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pjain1 commented Feb 21, 2026 •

edited

Loading

pjain1 Feb 24, 2026 •

edited

Loading

pjain1 Feb 26, 2026 •

edited

Loading

pjain1 Feb 26, 2026 •

edited

Loading