From a3dc2275dd0b250d585bcf4d8ac0d495a5bff80a Mon Sep 17 00:00:00 2001 From: Nishant Bangarwa Date: Tue, 1 Sep 2026 02:10:52 +0530 Subject: [PATCH] fix(runtime): return ai_instructions from the list_metrics_views tool The handler built a res map containing the instance's ai_instructions, but returned a typed result struct without it, so the map was discarded and project ai_instructions never reached MCP clients; the MCP server instructions tell clients to obey an ai_instructions field in tool responses, making this a silent no-op since the tool was introduced. Add the field to ListMetricsViewsResult and populate it for external MCP clients only: Rill's own agents already receive the project instructions directly in their prompts, so including it in their pre-invoked tool results would duplicate it in the conversation. Claude-Session: https://claude.ai/code/session_017udazBgdXmdTXMTq7sTh2L --- runtime/ai/metrics_view_list.go | 24 +++++----- runtime/ai/metrics_view_list_test.go | 70 ++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 runtime/ai/metrics_view_list_test.go diff --git a/runtime/ai/metrics_view_list.go b/runtime/ai/metrics_view_list.go index ddf208b0eef1..d7c6e6f8c5d7 100644 --- a/runtime/ai/metrics_view_list.go +++ b/runtime/ai/metrics_view_list.go @@ -22,7 +22,8 @@ var _ Tool[*ListMetricsViewsArgs, *ListMetricsViewsResult] = (*ListMetricsViews) type ListMetricsViewsArgs struct{} type ListMetricsViewsResult struct { - MetricsViews []map[string]any `json:"metrics_views"` + AIInstructions string `json:"ai_instructions,omitempty"` + MetricsViews []map[string]any `json:"metrics_views"` } func (t *ListMetricsViews) Spec() *mcp.Tool { @@ -91,16 +92,17 @@ func (t *ListMetricsViews) Handler(ctx context.Context, args *ListMetricsViewsAr i++ } - res := make(map[string]any) - // Find instance-wide AI context and add it to the response. // NOTE: These arguably belong in the top-level instructions or other metadata, but that doesn't currently support dynamic values. - instance, err := t.Runtime.Instance(ctx, session.InstanceID()) - if err != nil { - return nil, fmt.Errorf("failed to get instance %q: %w", session.InstanceID(), err) - } - if instance.AIInstructions != "" { - res["ai_instructions"] = instance.AIInstructions + // Rill's own agents receive the project instructions directly in their prompts, + // so this is only for external MCP clients (identified by a non-rill user agent). + var aiInstructions string + if !strings.HasPrefix(session.CatalogSession().UserAgent, "rill") { + instance, err := t.Runtime.Instance(ctx, session.InstanceID()) + if err != nil { + return nil, fmt.Errorf("failed to get instance %q: %w", session.InstanceID(), err) + } + aiInstructions = instance.AIInstructions } var metricsViews []map[string]any @@ -116,9 +118,9 @@ func (t *ListMetricsViews) Handler(ctx context.Context, args *ListMetricsViewsAr "description": mv.State.ValidSpec.Description, }) } - res["metrics_views"] = metricsViews return &ListMetricsViewsResult{ - MetricsViews: metricsViews, + AIInstructions: aiInstructions, + MetricsViews: metricsViews, }, nil } diff --git a/runtime/ai/metrics_view_list_test.go b/runtime/ai/metrics_view_list_test.go new file mode 100644 index 000000000000..2aeb6b985b8c --- /dev/null +++ b/runtime/ai/metrics_view_list_test.go @@ -0,0 +1,70 @@ +package ai_test + +import ( + "testing" + + "github.com/google/uuid" + "github.com/rilldata/rill/runtime" + "github.com/rilldata/rill/runtime/ai" + "github.com/rilldata/rill/runtime/pkg/activity" + "github.com/rilldata/rill/runtime/testruntime" + "github.com/stretchr/testify/require" +) + +// TestListMetricsViewsAIInstructions verifies that the project's ai_instructions are returned +// to external MCP clients, but not to Rill's own agents (which receive them directly in their prompts). +func TestListMetricsViewsAIInstructions(t *testing.T) { + rt, instanceID := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{ + Files: map[string]string{ + "rill.yaml": ` +ai_instructions: | + Revenue always refers to net revenue. +`, + "models/orders.yaml": ` +type: model +materialize: true +sql: SELECT '2025-01-01T00:00:00Z'::TIMESTAMP AS event_time, 100 AS revenue +`, + "metrics/orders.yaml": ` +type: metrics_view +model: orders +timeseries: event_time +measures: +- name: revenue + expression: SUM(revenue) +`, + }, + }) + testruntime.RequireReconcileState(t, rt, instanceID, 4, 0, 0) + + newSessionWithUserAgent := func(t *testing.T, userAgent string) *ai.Session { + claims := &runtime.SecurityClaims{UserID: uuid.NewString(), SkipChecks: true} + r := ai.NewRunner(rt, activity.NewNoopClient()) + s, err := r.Session(t.Context(), &ai.SessionOptions{ + InstanceID: instanceID, + Claims: claims, + UserAgent: userAgent, + }) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, s.Flush(t.Context())) + }) + return s + } + + // External MCP client: ai_instructions is included + s := newSessionWithUserAgent(t, "mcp-client") + var res *ai.ListMetricsViewsResult + _, err := s.CallTool(t.Context(), ai.RoleUser, ai.ListMetricsViewsName, &res, &ai.ListMetricsViewsArgs{}) + require.NoError(t, err) + require.Contains(t, res.AIInstructions, "Revenue always refers to net revenue.") + require.Len(t, res.MetricsViews, 1) + + // Rill's own agents: no ai_instructions (they are injected into agent prompts instead) + s = newSessionWithUserAgent(t, "rill-web") + res = nil + _, err = s.CallTool(t.Context(), ai.RoleUser, ai.ListMetricsViewsName, &res, &ai.ListMetricsViewsArgs{}) + require.NoError(t, err) + require.Empty(t, res.AIInstructions) + require.Len(t, res.MetricsViews, 1) +}