From e5e31329d1073cd5c1d5ea7e4ab9074f9cd53b27 Mon Sep 17 00:00:00 2001 From: Nishant Bangarwa Date: Wed, 2 Sep 2026 17:28:34 +0530 Subject: [PATCH 01/15] feat(ai): support project-defined skills for AI agents Skills are markdown files at skills/.md or skills//SKILL.md with YAML front matter (description required; optional name, metrics_views, agents, always_apply) that teach Rill's AI agents project-specific practices such as analysis playbooks and business glossaries. Runtime: - New session-scoped loader in runtime/ai reads skills from the repo; malformed files are reported as issues and logged, never failing the session. - New list_skills and load_skill tools gated on UseAI only, so cloud viewers without repo access can use them; they are exposed on the MCP server automatically for external clients. - The analyst agent's prompt gains an index of relevant skills (scope-filtered by the dashboard's metrics views) and inlines always_apply skill bodies after ai_instructions, capped at 32kb. - The MCP server instructions tell clients to discover and load skills. - instructions.ParseFrontMatter is exported and shared with the embedded instructions. Frontend: - load_skill/list_skills render in the chat thinking trace with a skill icon. - Add > More gains an "AI Skill" entry that creates skills/my_skill.md from a starter template; skill files get a dedicated icon in the file explorer. Claude-Session: https://claude.ai/code/session_017udazBgdXmdTXMTq7sTh2L --- .../docs/developers/build/ai-configuration.md | 44 ++++ docs/docs/guide/ai/mcp.md | 4 + runtime/ai/ai.go | 7 + runtime/ai/analyst_agent.go | 70 +++++-- runtime/ai/analyst_agent_test.go | 67 ++++++ runtime/ai/instructions/data/development.md | 14 ++ runtime/ai/instructions/instructions.go | 21 +- runtime/ai/instructions/instructions_test.go | 3 +- runtime/ai/mcp.go | 6 + runtime/ai/skill_list.go | 80 +++++++ runtime/ai/skill_load.go | 77 +++++++ runtime/ai/skills.go | 197 ++++++++++++++++++ runtime/ai/skills_internal_test.go | 55 +++++ runtime/ai/skills_test.go | 140 +++++++++++++ .../chat/core/messages/tools/tool-icons.ts | 3 + web-common/src/features/chat/core/types.ts | 2 + .../add/AddAssetButton.svelte | 29 ++- .../entity-management/add/new-files.ts | 18 ++ .../resource-icon-mapping.ts | 11 +- 19 files changed, 820 insertions(+), 28 deletions(-) create mode 100644 runtime/ai/skill_list.go create mode 100644 runtime/ai/skill_load.go create mode 100644 runtime/ai/skills.go create mode 100644 runtime/ai/skills_internal_test.go create mode 100644 runtime/ai/skills_test.go diff --git a/docs/docs/developers/build/ai-configuration.md b/docs/docs/developers/build/ai-configuration.md index aaea0721fa61..d4881f2061d4 100644 --- a/docs/docs/developers/build/ai-configuration.md +++ b/docs/docs/developers/build/ai-configuration.md @@ -16,6 +16,8 @@ There are two places to add `ai_instructions`: 1. **`rill.yaml`**: Project-wide instructions that apply to all queries across your entire project. 2. **`.yaml`**: Metrics view-specific instructions for individual dashboards. +For longer, structured guidance — such as step-by-step analysis playbooks — use [skills](#skills) instead, which the AI loads on demand. + ## Automatic Context Inclusion In addition to `ai_instructions`, Rill automatically includes the following in the AI context: @@ -95,6 +97,48 @@ ai_instructions: | - Weekend traffic patterns are anomalous due to our B2B focus. ``` +## Skills + +Skills are markdown files that teach Rill's AI project-specific practices, such as analysis playbooks (e.g. how to do root-cause analysis for a revenue drop) or business glossaries. Where `ai_instructions` is best for short guidance that always applies, skills hold longer, structured instructions that the AI loads only when they are relevant to the question at hand. Skills apply both in [AI Chat](/guide/ai/ai-chat) and to external AI clients connected via the [MCP Server](/guide/ai/mcp). + +A skill lives at `skills/.md` (or `skills//SKILL.md`) and consists of YAML front matter followed by markdown instructions: + +```markdown +--- +description: Playbook for diagnosing revenue drops. Use when asked why revenue or bookings declined. +--- + +# Revenue root-cause analysis + +When asked why revenue declined: +1. Establish the comparison window and compute the total change. +2. Break the change down by `channel`, then `region`, then `plan_type`. +3. Account for known seasonality: B2B traffic drops on weekends. +4. State your confidence and call out data quirks that may affect the result. +``` + +The `description` is required: the AI sees an index of skill names and descriptions, and uses the description to decide when to load a skill. Phrase it as "what it does + when to use it". + +The front matter supports these additional properties: + +```markdown +--- +description: Business glossary for our e-commerce metrics. +name: glossary # Optional: overrides the name derived from the file path +metrics_views: [orders] # Optional: only offer this skill for analyses involving these metrics views +always_apply: true # Optional: always include the full skill instead of loading it on demand +--- +``` + +- **`metrics_views`** scopes a skill to specific metrics views, so for example a marketing playbook is not offered during a finance analysis. It is a relevance filter, not access control. +- **`always_apply`** injects the skill's full contents into every conversation, like `ai_instructions`. Use it for short, broadly applicable guidance such as glossaries; keep always-apply skills small since they are included in every request. + +When the AI uses a skill, the chat response's activity trace shows a "Loaded skill" step, so you can verify a skill was applied and iterate on it: edit the file, ask a test question, and check the trace. + +:::warning Skills are visible to all AI users +Skill contents are provided to every user who can use AI features in the project, including viewers. Never put secrets or sensitive data in a skill. Access to the underlying data is still governed by your metrics view security policies. +::: + ## Visualization Tips When using the [Rill MCP Server](/guide/ai/mcp) with external AI clients like Claude, you can provide specific instructions on how to visualize data. Since the MCP server returns structured data, the AI client is responsible for rendering it. diff --git a/docs/docs/guide/ai/mcp.md b/docs/docs/guide/ai/mcp.md index ee1b47b7d26a..1b3c15d01e06 100644 --- a/docs/docs/guide/ai/mcp.md +++ b/docs/docs/guide/ai/mcp.md @@ -243,6 +243,8 @@ There are two places to add `ai_instructions`: 1. `rill.yaml` for project-wide context, such as instructions on how to use Rill MCP Server 2. Every metrics view YAML (`.yaml`), with examples of Explore URLs for that metrics view +For longer, structured guidance — such as analysis playbooks and business glossaries — you can also define [skills](/developers/build/ai-configuration#skills) in your project. External MCP clients discover them with the `list_skills` tool and load them on demand with `load_skill`. + For detailed examples and best practices on writing effective AI instructions, see the [AI Configuration guide](/developers/build/ai-configuration). You can look at one of our [example projects](https://github.com/rilldata/rill-examples/tree/main/rill-openrtb-prog-ads) to see how these are used. Experiment with the instructions and see what works best for your requirements. @@ -258,6 +260,8 @@ You can look at one of our [example projects](https://github.com/rilldata/rill-e - __*Get metrics view spec*__ – Use `get_metrics_view` to fetch a metrics view's specification. This is important to understand all the dimensions and measures in a metrics view. - __*Query the time range*__ – Use `query_metrics_view_summary` to obtain the available time range for a metrics view. This is important to understand what time range the data spans. - __*Query the metrics*__ – Use `query_metrics_view` to run queries to get aggregated results. +- __*List skills*__ – Use `list_skills` to discover the [skills](/developers/build/ai-configuration#skills) defined in the project. +- __*Load a skill*__ – Use `load_skill` to fetch a skill's full instructions before doing work its description covers. ### Usage Examples diff --git a/runtime/ai/ai.go b/runtime/ai/ai.go index 85b6a4dcdb17..1a8df5e52b16 100644 --- a/runtime/ai/ai.go +++ b/runtime/ai/ai.go @@ -61,6 +61,8 @@ func NewRunner(rt *runtime.Runtime, activity *activity.Client) *Runner { RegisterTool(r, &QueryMetricsViewSummary{Runtime: rt}) RegisterTool(r, &QueryMetricsView{Runtime: rt}) RegisterTool(r, &CreateChart{Runtime: rt}) + RegisterTool(r, &ListSkills{Runtime: rt}) + RegisterTool(r, &LoadSkill{Runtime: rt}) RegisterTool(r, &DevelopFile{Runtime: rt}) RegisterTool(r, &ListFiles{Runtime: rt}) @@ -526,6 +528,11 @@ type BaseSession struct { messages []*Message messagesDirty bool subscribers map[chan *Message]struct{} + + skillsMu sync.Mutex + skillsLoaded bool + skills []*Skill + skillIssues []SkillIssue } func (s *BaseSession) Flush(ctx context.Context) error { diff --git a/runtime/ai/analyst_agent.go b/runtime/ai/analyst_agent.go index 40bfab552f14..fd8ceec363dd 100644 --- a/runtime/ai/analyst_agent.go +++ b/runtime/ai/analyst_agent.go @@ -14,6 +14,7 @@ import ( "github.com/rilldata/rill/runtime" "github.com/rilldata/rill/runtime/ai/instructions" "github.com/rilldata/rill/runtime/metricsview" + "go.uber.org/zap" ) const AnalystAgentName = "analyst_agent" @@ -160,6 +161,15 @@ func (t *AnalystAgent) Handler(ctx context.Context, args *AnalystAgentArgs) (*An } } + // Load project-defined skills relevant to this analysis. + // Skill loading failures should degrade the analysis, not fail it. + skills, _, err := s.Skills(ctx) + if err != nil { + s.logger.Warn("failed to load project skills", zap.Error(err)) + skills = nil + } + skills = filterSkills(skills, skillAgentAnalyst, metricsViewNames) + // Determine tools that can be used tools := []string{} if args.Explore == "" { @@ -169,13 +179,16 @@ func (t *AnalystAgent) Handler(ctx context.Context, args *AnalystAgentArgs) (*An if !args.DisableCharts { tools = append(tools, CreateChartName) } + if len(skills) > 0 { + tools = append(tools, LoadSkillName) + } // Build completion messages systemPrompt, err := t.systemPrompt() if err != nil { return nil, err } - userPrompt, err := t.userPrompt(ctx, metricsViewNames, args) + userPrompt, err := t.userPrompt(ctx, metricsViewNames, skills, args) if err != nil { return nil, err } @@ -213,7 +226,7 @@ func (t *AnalystAgent) systemPrompt() (string, error) { return instr.Body, nil } -func (t *AnalystAgent) userPrompt(ctx context.Context, metricsViewNames []string, args *AnalystAgentArgs) (string, error) { +func (t *AnalystAgent) userPrompt(ctx context.Context, metricsViewNames []string, skills []*Skill, args *AnalystAgentArgs) (string, error) { // Prepare template data. // NOTE: All the template properties are optional and may be empty. session := GetSession(ctx) @@ -242,20 +255,37 @@ func (t *AnalystAgent) userPrompt(ctx context.Context, metricsViewNames []string measuresQuoted[i] = fmt.Sprintf("`%s`", measure) } + // Split skills into always-apply bodies (injected wholesale) and an index of on-demand skills (loaded via load_skill). + // Always-apply bodies that would exceed the size cap fall back to the on-demand index. + var alwaysApplySkills strings.Builder + var skillsIndex strings.Builder + for _, sk := range skills { + if sk.AlwaysApply && alwaysApplySkills.Len()+len(sk.Body) <= skillsMaxAlwaysApplyBytes { + fmt.Fprintf(&alwaysApplySkills, "## Skill: %s\n\n%s\n\n", sk.Name, sk.Body) + } else { + if sk.AlwaysApply { + session.logger.Warn("always-apply skill exceeds the prompt size cap; falling back to on-demand loading", zap.String("skill", sk.Name)) + } + fmt.Fprintf(&skillsIndex, "- %s: %s\n", sk.Name, sk.Description) + } + } + data := map[string]any{ - "prompt": args.Prompt, - "ai_instructions": session.ProjectInstructions(), - "is_prompt": args.Prompt != "", - "metrics_views": strings.Join(metricsViewsQuoted, ", "), - "explore": args.Explore, - "canvas": args.Canvas, - "canvas_component": args.CanvasComponent, - "dimensions": strings.Join(dimensionsQuoted, ", "), - "measures": strings.Join(measuresQuoted, ", "), - "forked": session.Forked(), - "is_report": args.IsReport, - "now": time.Now(), - "max_query_limit": instanceCfg.AIMaxQueryLimit, + "prompt": args.Prompt, + "ai_instructions": session.ProjectInstructions(), + "always_apply_skills": strings.TrimSpace(alwaysApplySkills.String()), + "skills_index": strings.TrimSpace(skillsIndex.String()), + "is_prompt": args.Prompt != "", + "metrics_views": strings.Join(metricsViewsQuoted, ", "), + "explore": args.Explore, + "canvas": args.Canvas, + "canvas_component": args.CanvasComponent, + "dimensions": strings.Join(dimensionsQuoted, ", "), + "measures": strings.Join(measuresQuoted, ", "), + "forked": session.Forked(), + "is_report": args.IsReport, + "now": time.Now(), + "max_query_limit": instanceCfg.AIMaxQueryLimit, } if !args.TimeStart.IsZero() && !args.TimeEnd.IsZero() { @@ -380,6 +410,16 @@ The administrator has provided the following project-wide instructions, which ma {{ .ai_instructions }} {{ end }} +{{ if .always_apply_skills }} +The administrator has defined the following skills that always apply to this analysis. Follow their guidance: +{{ .always_apply_skills }} +{{ end }} + +{{ if .skills_index }} +The administrator has defined the following analysis skills. Before starting the analysis, check whether any skill matches the user's request; if one does, call the "load_skill" tool with its name and follow its instructions: +{{ .skills_index }} +{{ end }} + {{ if .is_prompt }} The user's request: {{ .prompt }} diff --git a/runtime/ai/analyst_agent_test.go b/runtime/ai/analyst_agent_test.go index cb21e17caded..70029f09579c 100644 --- a/runtime/ai/analyst_agent_test.go +++ b/runtime/ai/analyst_agent_test.go @@ -384,6 +384,73 @@ func requireValidChartSpec(t *testing.T, s *ai.Session, chartCall *ai.Message, v return spec } +func TestAnalystSkills(t *testing.T) { + // Setup the basic orders metrics view plus two project skills: + // an on-demand RCA playbook (loaded via load_skill) and an always-apply glossary. + rt, instanceID := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{ + AIConnector: "openai", + Files: map[string]string{ + "models/orders.yaml": ` +type: model +materialize: true +sql: | + SELECT '2025-01-01T00:00:00Z'::TIMESTAMP AS event_time, 'United States' AS country, 100 AS revenue + UNION ALL + SELECT '2025-01-01T00:00:00Z'::TIMESTAMP AS event_time, 'Denmark' AS country, 10 AS revenue + UNION ALL + SELECT '2025-01-02T00:00:00Z'::TIMESTAMP AS event_time, 'United States' AS country, 100 AS revenue + UNION ALL + SELECT '2025-01-02T00:00:00Z'::TIMESTAMP AS event_time, 'Denmark' AS country, 10 AS revenue +`, + "metrics/orders.yaml": ` +type: metrics_view +model: orders +timeseries: event_time +dimensions: +- column: country +measures: +- name: count + expression: COUNT(*) +- name: revenue + expression: SUM(revenue) +`, + "skills/revenue-rca.md": `--- +description: Playbook for analyzing revenue. Use whenever asked about revenue amounts or changes. +metrics_views: [orders] +--- + +When answering any question about revenue, you MUST end your final answer with the exact phrase "(via revenue-rca)". +`, + "skills/glossary.md": `--- +description: Business glossary. +always_apply: true +--- + +"Nordics revenue" refers to revenue where the country is Denmark. +`, + }, + }) + testruntime.RequireReconcileState(t, rt, instanceID, 4, 0, 0) + + // Initialize eval + s := newEval(t, rt, instanceID) + + // Ask a revenue question that exercises both skills + var res *ai.RouterAgentResult + _, err := s.CallTool(t.Context(), ai.RoleUser, ai.RouterAgentName, &res, ai.RouterAgentArgs{ + Prompt: "What is the total Nordics revenue? Answer with the number and any required attribution, nothing else.", + }) + require.NoError(t, err) + require.Equal(t, ai.AnalystAgentName, res.Agent) + + // The on-demand RCA skill must have been loaded and its instruction followed + require.NotEmpty(t, s.Messages(ai.FilterByType(ai.MessageTypeCall), ai.FilterByTool(ai.LoadSkillName))) + require.Contains(t, res.Response, "(via revenue-rca)") + + // The always-apply glossary skill defines "Nordics revenue" as Denmark's revenue + require.Contains(t, res.Response, "20") +} + func parseTestTime(tst *testing.T, t string) time.Time { ts, err := time.Parse(time.RFC3339, t) require.NoError(tst, err) diff --git a/runtime/ai/instructions/data/development.md b/runtime/ai/instructions/data/development.md index 8f7f9ec23889..39d05532ee74 100644 --- a/runtime/ai/instructions/data/development.md +++ b/runtime/ai/instructions/data/development.md @@ -189,6 +189,20 @@ Since they repeatedly run a query, they are slightly expensive resources. They are usually found downstream of a metrics view in the DAG. Most projects don't define reports directly as files; instead, users can define reports using a UI in Rill Cloud. +### Skills + +Skills are markdown files that teach Rill's AI agents project-specific practices, such as analysis playbooks (e.g. how to do root-cause analysis for a revenue drop) and business glossaries. +Unlike the resource types above, they are not parsed into resources; they are plain markdown files read directly by the AI agents. +They live at `skills/.md` or `skills//SKILL.md` and consist of YAML front matter followed by a markdown body with the instructions. +The front matter supports these properties: +- `description:` (required) a short summary used to decide when the skill applies; write it as "what it does + when to use it" +- `name:` (optional) overrides the name derived from the file path +- `metrics_views:` (optional) list of metrics view names; the skill is only offered when the analysis involves one of them +- `agents:` (optional) list of agents the skill applies to, `analyst` and/or `developer`; defaults to `[analyst]` +- `always_apply:` (optional) if `true`, the skill's full body is always injected into the agent's context instead of being loaded on demand; use for short, broadly applicable guidance such as glossaries + +Skill contents are visible to every user who can use AI features in the project, so they must never contain secrets. + ### `rill.yaml` `rill.yaml` is a required file for project-wide config found at the root directory of a Rill project. diff --git a/runtime/ai/instructions/instructions.go b/runtime/ai/instructions/instructions.go index 09a154827f4d..2eec5334f4cd 100644 --- a/runtime/ai/instructions/instructions.go +++ b/runtime/ai/instructions/instructions.go @@ -96,7 +96,8 @@ func parseInstruction(path string, content []byte, opts Options) (*Instruction, name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) // Parse front matter - fm, body, err := parseFrontMatter(content) + var fm frontMatter + body, err := ParseFrontMatter(content, &fm) if err != nil { return nil, err } @@ -114,15 +115,16 @@ func parseInstruction(path string, content []byte, opts Options) (*Instruction, }, nil } -// parseFrontMatter extracts YAML front matter from markdown content. +// ParseFrontMatter extracts YAML front matter from markdown content, decoding it into the provided struct. // Front matter is expected to be delimited by "---" at the start and end. -func parseFrontMatter(content []byte) (*frontMatter, string, error) { +// If the content has no front matter, the struct is left untouched and the full content is returned as the body. +func ParseFrontMatter(content []byte, into any) (string, error) { contentStr := strings.TrimSpace(string(content)) // Check for front matter delimiter at the start if !strings.HasPrefix(contentStr, "---\n") && !strings.HasPrefix(contentStr, "---\r\n") { - // No front matter, return empty front matter and full content as body - return &frontMatter{}, contentStr, nil + // No front matter, return full content as body + return contentStr, nil } // Find the closing delimiter @@ -134,7 +136,7 @@ func parseFrontMatter(content []byte) (*frontMatter, string, error) { endIdx := strings.Index(rest, "\n---") if endIdx == -1 { - return nil, "", fmt.Errorf("unclosed front matter: missing closing ---") + return "", fmt.Errorf("unclosed front matter: missing closing ---") } frontMatterContent := rest[:endIdx] @@ -144,12 +146,11 @@ func parseFrontMatter(content []byte) (*frontMatter, string, error) { body = strings.TrimSpace(body) // Parse the front matter YAML - var fm frontMatter - if err := yaml.Unmarshal([]byte(frontMatterContent), &fm); err != nil { - return nil, "", fmt.Errorf("failed to parse front matter YAML: %w", err) + if err := yaml.Unmarshal([]byte(frontMatterContent), into); err != nil { + return "", fmt.Errorf("failed to parse front matter YAML: %w", err) } - return &fm, body, nil + return body, nil } // executeTemplate applies Go's template engine to the instruction body. diff --git a/runtime/ai/instructions/instructions_test.go b/runtime/ai/instructions/instructions_test.go index 6fb005dcf5ac..4445232eaac9 100644 --- a/runtime/ai/instructions/instructions_test.go +++ b/runtime/ai/instructions/instructions_test.go @@ -114,7 +114,8 @@ description: Test`, for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fm, body, err := parseFrontMatter([]byte(tt.content)) + var fm frontMatter + body, err := ParseFrontMatter([]byte(tt.content), &fm) if tt.wantErr { require.Error(t, err) return diff --git a/runtime/ai/mcp.go b/runtime/ai/mcp.go index cb6610e0ec2a..33858e4b3df9 100644 --- a/runtime/ai/mcp.go +++ b/runtime/ai/mcp.go @@ -28,6 +28,12 @@ This server exposes APIs for querying **metrics views**, which represent Rill's In the workflow, do not proceed with the next step until the previous step has been completed. If the information from the previous step is already known (let's say for subsequent queries), you can skip it. If a response contains an "ai_instructions" field, you should interpret it as additional instructions for how to behave in subsequent responses that relate to that tool call. +## Skills +Projects may define **skills**: instruction files that teach agents project-specific analysis or development practices, such as analysis playbooks and business glossaries. +- Use "list_skills" early in a session to discover the project's skills. +- Before doing work that a skill's description covers, use "load_skill" to fetch its full instructions and follow them. +- Load any skill marked "always_apply" up front and treat its instructions as always in effect. + ## Project Development If you have edit access, the server also exposes tools for inspecting and editing the project's source code, which consists of YAML and SQL files: - **List files:** Use "list_files" to browse the files in the project. diff --git a/runtime/ai/skill_list.go b/runtime/ai/skill_list.go new file mode 100644 index 000000000000..4d5e42a8b2e3 --- /dev/null +++ b/runtime/ai/skill_list.go @@ -0,0 +1,80 @@ +package ai + +import ( + "context" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/rilldata/rill/runtime" +) + +const ListSkillsName = "list_skills" + +type ListSkills struct { + Runtime *runtime.Runtime +} + +var _ Tool[*ListSkillsArgs, *ListSkillsResult] = (*ListSkills)(nil) + +type ListSkillsArgs struct{} + +type ListSkillsResult struct { + Skills []*SkillInfo `json:"skills"` + Invalid []SkillIssue `json:"invalid,omitempty"` +} + +// SkillInfo describes a skill without its body. Use load_skill to fetch the full instructions. +type SkillInfo struct { + Name string `json:"name"` + Description string `json:"description"` + MetricsViews []string `json:"metrics_views,omitempty"` + Agents []string `json:"agents"` + AlwaysApply bool `json:"always_apply"` +} + +func (t *ListSkills) Spec() *mcp.Tool { + return &mcp.Tool{ + Name: ListSkillsName, + Title: "List skills", + Description: "Lists the skills defined in the project. Skills are instruction files that teach AI agents project-specific analysis or development practices. Load a skill with the load_skill tool before doing work its description covers. Treat skills marked always_apply as standing instructions and load them up front.", + Annotations: &mcp.ToolAnnotations{ + DestructiveHint: boolPtr(false), + IdempotentHint: true, + OpenWorldHint: boolPtr(false), + ReadOnlyHint: true, + }, + Meta: map[string]any{ + "openai/toolInvocation/invoking": "Listing skills...", + "openai/toolInvocation/invoked": "Listed skills", + }, + } +} + +func (t *ListSkills) CheckAccess(ctx context.Context) (bool, error) { + s := GetSession(ctx) + return s.Claims().Can(runtime.UseAI), nil +} + +func (t *ListSkills) Handler(ctx context.Context, args *ListSkillsArgs) (*ListSkillsResult, error) { + s := GetSession(ctx) + + skills, issues, err := s.Skills(ctx) + if err != nil { + return nil, err + } + + infos := make([]*SkillInfo, len(skills)) + for i, sk := range skills { + infos[i] = &SkillInfo{ + Name: sk.Name, + Description: sk.Description, + MetricsViews: sk.MetricsViews, + Agents: sk.Agents, + AlwaysApply: sk.AlwaysApply, + } + } + + return &ListSkillsResult{ + Skills: infos, + Invalid: issues, + }, nil +} diff --git a/runtime/ai/skill_load.go b/runtime/ai/skill_load.go new file mode 100644 index 000000000000..e139899ee1f6 --- /dev/null +++ b/runtime/ai/skill_load.go @@ -0,0 +1,77 @@ +package ai + +import ( + "context" + "fmt" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/rilldata/rill/runtime" +) + +const LoadSkillName = "load_skill" + +type LoadSkill struct { + Runtime *runtime.Runtime +} + +var _ Tool[*LoadSkillArgs, *LoadSkillResult] = (*LoadSkill)(nil) + +type LoadSkillArgs struct { + Name string `json:"name" jsonschema:"Name of the skill to load"` +} + +type LoadSkillResult struct { + Name string `json:"name"` + Description string `json:"description"` + Body string `json:"body"` +} + +func (t *LoadSkill) Spec() *mcp.Tool { + return &mcp.Tool{ + Name: LoadSkillName, + Title: "Using skill", + Description: "Loads the full instructions for a skill defined in the project. Skills teach project-specific analysis or development practices. Call this before doing work that a skill's description covers, then follow the returned instructions.", + Annotations: &mcp.ToolAnnotations{ + DestructiveHint: boolPtr(false), + IdempotentHint: true, + OpenWorldHint: boolPtr(false), + ReadOnlyHint: true, + }, + Meta: map[string]any{ + "openai/toolInvocation/invoking": "Loading skill...", + "openai/toolInvocation/invoked": "Loaded skill", + }, + } +} + +func (t *LoadSkill) CheckAccess(ctx context.Context) (bool, error) { + s := GetSession(ctx) + return s.Claims().Can(runtime.UseAI), nil +} + +func (t *LoadSkill) Handler(ctx context.Context, args *LoadSkillArgs) (*LoadSkillResult, error) { + s := GetSession(ctx) + + skills, _, err := s.Skills(ctx) + if err != nil { + return nil, err + } + + names := make([]string, len(skills)) + for i, sk := range skills { + if sk.Name == args.Name { + return &LoadSkillResult{ + Name: sk.Name, + Description: sk.Description, + Body: sk.Body, + }, nil + } + names[i] = sk.Name + } + + if len(names) == 0 { + return nil, fmt.Errorf("skill %q not found: the project does not define any skills", args.Name) + } + return nil, fmt.Errorf("skill %q not found: available skills are %s", args.Name, strings.Join(names, ", ")) +} diff --git a/runtime/ai/skills.go b/runtime/ai/skills.go new file mode 100644 index 000000000000..bc498065bb69 --- /dev/null +++ b/runtime/ai/skills.go @@ -0,0 +1,197 @@ +package ai + +import ( + "context" + "fmt" + "path" + "slices" + "strings" + + "github.com/rilldata/rill/runtime" + "github.com/rilldata/rill/runtime/ai/instructions" + "github.com/rilldata/rill/runtime/drivers" + "go.uber.org/zap" +) + +// skillsGlob matches skill files in the project repo. +// Skills are markdown files with YAML front matter, located at `skills/.md` or `skills//SKILL.md`. +const skillsGlob = "skills/**/*.md" + +// skillMaxFileSize is the maximum size of a skill file. +// It matches the parser's per-file limit so skill files remain valid if they become parsed resources in the future. +const skillMaxFileSize = 1 << 17 // 128kb + +// skillsMaxAlwaysApplyBytes caps the total size of always-apply skill bodies injected into a prompt. +// Skills that exceed the cap fall back to on-demand loading via the load_skill tool. +const skillsMaxAlwaysApplyBytes = 1 << 15 // 32kb + +// Agents that a skill can target via the `agents` front matter field. +const ( + skillAgentAnalyst = "analyst" + skillAgentDeveloper = "developer" +) + +// Skill is a user-defined instruction file that teaches Rill's AI agents project-specific practices, +// such as analysis playbooks, business glossaries, or development conventions. +type Skill struct { + Name string `json:"name"` + Path string `json:"path"` + Description string `json:"description"` + MetricsViews []string `json:"metrics_views,omitempty"` + Agents []string `json:"agents"` + AlwaysApply bool `json:"always_apply"` + Body string `json:"body"` +} + +// SkillIssue describes a skill file that could not be loaded. +type SkillIssue struct { + Path string `json:"path"` + Error string `json:"error"` +} + +// skillFrontMatter is the YAML front matter of a skill file. +type skillFrontMatter struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + MetricsViews []string `yaml:"metrics_views"` + Agents []string `yaml:"agents"` + AlwaysApply bool `yaml:"always_apply"` +} + +// Skills lazily loads the project's skill files, memoizing the result for the lifetime of the session. +// Malformed skill files are reported as issues and logged; they never fail the load as a whole. +func (s *BaseSession) Skills(ctx context.Context) ([]*Skill, []SkillIssue, error) { + s.skillsMu.Lock() + defer s.skillsMu.Unlock() + if s.skillsLoaded { + return s.skills, s.skillIssues, nil + } + + skills, issues, err := loadSkills(ctx, s.runner.Runtime, s.instanceID) + if err != nil { + return nil, nil, err + } + for _, issue := range issues { + s.logger.Warn("skipping invalid skill file", zap.String("path", issue.Path), zap.String("error", issue.Error)) + } + + s.skills = skills + s.skillIssues = issues + s.skillsLoaded = true + return s.skills, s.skillIssues, nil +} + +// loadSkills reads and parses all skill files in the project repo. +func loadSkills(ctx context.Context, rt *runtime.Runtime, instanceID string) ([]*Skill, []SkillIssue, error) { + repo, release, err := rt.Repo(ctx, instanceID) + if err != nil { + return nil, nil, fmt.Errorf("failed to open repo: %w", err) + } + defer release() + + entries, err := repo.ListGlob(ctx, skillsGlob, true) + if err != nil { + return nil, nil, fmt.Errorf("failed to list skill files: %w", err) + } + + // Sort by path so duplicate name resolution is deterministic (first path wins). + slices.SortFunc(entries, func(a, b drivers.DirEntry) int { return strings.Compare(a.Path, b.Path) }) + + var skills []*Skill + var issues []SkillIssue + seen := map[string]string{} // skill name to the path that claimed it + for _, entry := range entries { + name, ok := skillNameForPath(entry.Path) + if !ok { + // E.g. an auxiliary markdown file inside a skill directory + continue + } + + content, err := repo.Get(ctx, entry.Path) + if err != nil { + issues = append(issues, SkillIssue{Path: entry.Path, Error: fmt.Sprintf("failed to read file: %s", err)}) + continue + } + if len(content) > skillMaxFileSize { + issues = append(issues, SkillIssue{Path: entry.Path, Error: fmt.Sprintf("file exceeds the maximum skill size of %d bytes", skillMaxFileSize)}) + continue + } + + var fm skillFrontMatter + body, err := instructions.ParseFrontMatter([]byte(content), &fm) + if err != nil { + issues = append(issues, SkillIssue{Path: entry.Path, Error: err.Error()}) + continue + } + if fm.Name != "" { + name = fm.Name + } + if fm.Description == "" { + issues = append(issues, SkillIssue{Path: entry.Path, Error: "missing required \"description\" property in front matter"}) + continue + } + if prev, ok := seen[name]; ok { + issues = append(issues, SkillIssue{Path: entry.Path, Error: fmt.Sprintf("duplicate skill name %q (already defined in %q)", name, prev)}) + continue + } + seen[name] = entry.Path + + agents := fm.Agents + if len(agents) == 0 { + agents = []string{skillAgentAnalyst} + } + + skills = append(skills, &Skill{ + Name: name, + Path: entry.Path, + Description: fm.Description, + MetricsViews: fm.MetricsViews, + Agents: agents, + AlwaysApply: fm.AlwaysApply, + Body: body, + }) + } + + return skills, issues, nil +} + +// skillNameForPath derives a skill's default name from its file path. +// Valid skill paths are `/skills/.md` and `/skills//SKILL.md`; +// other markdown files under `/skills/` (such as auxiliary files in a skill directory) return false. +func skillNameForPath(p string) (string, bool) { + parts := strings.Split(strings.TrimPrefix(path.Clean(p), "/"), "/") + if len(parts) < 2 || parts[0] != "skills" { + return "", false + } + switch len(parts) { + case 2: + return strings.TrimSuffix(parts[1], ".md"), true + case 3: + if parts[2] == "SKILL.md" { + return parts[1], true + } + } + return "", false +} + +// filterSkills returns the skills relevant to the given agent and metrics view context. +// A skill scoped to specific metrics views is included only if the context references one of them. +// An empty context includes all of the agent's skills: scoping is a relevance filter, not access control. +func filterSkills(skills []*Skill, agent string, metricsViewNames []string) []*Skill { + var res []*Skill + for _, sk := range skills { + if !slices.Contains(sk.Agents, agent) { + continue + } + if len(sk.MetricsViews) > 0 && len(metricsViewNames) > 0 { + relevant := slices.ContainsFunc(sk.MetricsViews, func(mv string) bool { + return slices.Contains(metricsViewNames, mv) + }) + if !relevant { + continue + } + } + res = append(res, sk) + } + return res +} diff --git a/runtime/ai/skills_internal_test.go b/runtime/ai/skills_internal_test.go new file mode 100644 index 000000000000..205e1932e79d --- /dev/null +++ b/runtime/ai/skills_internal_test.go @@ -0,0 +1,55 @@ +package ai + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSkillNameForPath(t *testing.T) { + tests := []struct { + path string + wantName string + wantOK bool + }{ + {"/skills/revenue-rca.md", "revenue-rca", true}, + {"/skills/glossary/SKILL.md", "glossary", true}, + {"/skills/glossary/notes.md", "", false}, + {"/skills/a/b/SKILL.md", "", false}, + {"/models/orders.md", "", false}, + {"/skills.md", "", false}, + } + for _, tt := range tests { + name, ok := skillNameForPath(tt.path) + require.Equal(t, tt.wantOK, ok, "path %q", tt.path) + require.Equal(t, tt.wantName, name, "path %q", tt.path) + } +} + +func TestFilterSkills(t *testing.T) { + skills := []*Skill{ + {Name: "rca", Agents: []string{skillAgentAnalyst}, MetricsViews: []string{"orders"}}, + {Name: "glossary", Agents: []string{skillAgentAnalyst, skillAgentDeveloper}}, + {Name: "modeling", Agents: []string{skillAgentDeveloper}}, + } + + names := func(skills []*Skill) []string { + res := make([]string, len(skills)) + for i, sk := range skills { + res[i] = sk.Name + } + return res + } + + // No metrics view context: all analyst skills are included (scoping is relevance, not security) + require.Equal(t, []string{"rca", "glossary"}, names(filterSkills(skills, skillAgentAnalyst, nil))) + + // Matching metrics view context + require.Equal(t, []string{"rca", "glossary"}, names(filterSkills(skills, skillAgentAnalyst, []string{"orders"}))) + + // Non-matching metrics view context: scoped skills are excluded, unscoped ones remain + require.Equal(t, []string{"glossary"}, names(filterSkills(skills, skillAgentAnalyst, []string{"bids"}))) + + // Developer agent + require.Equal(t, []string{"glossary", "modeling"}, names(filterSkills(skills, skillAgentDeveloper, nil))) +} diff --git a/runtime/ai/skills_test.go b/runtime/ai/skills_test.go new file mode 100644 index 000000000000..bfd07e37413e --- /dev/null +++ b/runtime/ai/skills_test.go @@ -0,0 +1,140 @@ +package ai_test + +import ( + "strings" + "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" +) + +func TestSkills(t *testing.T) { + rt, instanceID := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{ + Files: map[string]string{ + "skills/revenue-rca.md": `--- +description: Playbook for diagnosing revenue drops. +metrics_views: [orders] +--- + +# Revenue RCA playbook +Always break revenue down by country first.`, + "skills/glossary/SKILL.md": `--- +description: Business glossary. +always_apply: true +agents: [analyst, developer] +--- + +ARPU excludes trial users.`, + // Auxiliary file in a skill directory: not a skill. + "skills/glossary/notes.md": "Internal notes, not a skill.", + // Missing required description. + "skills/broken.md": "# No front matter here", + // Duplicate name (sorted after revenue-rca.md, so it loses). + "skills/zzz-dupe.md": `--- +name: revenue-rca +description: Duplicate of the RCA skill. +--- + +Body.`, + // Exceeds the per-file size cap. + "skills/oversized.md": "---\ndescription: Too big.\n---\n\n" + strings.Repeat("x", 1<<17), + }, + }) + s := newSession(t, rt, instanceID) + + // List skills: valid skills in path order, invalid files reported with errors + var listRes *ai.ListSkillsResult + _, err := s.CallTool(t.Context(), ai.RoleUser, ai.ListSkillsName, &listRes, &ai.ListSkillsArgs{}) + require.NoError(t, err) + require.Len(t, listRes.Skills, 2) + require.Equal(t, "glossary", listRes.Skills[0].Name) + require.True(t, listRes.Skills[0].AlwaysApply) + require.Equal(t, []string{"analyst", "developer"}, listRes.Skills[0].Agents) + require.Equal(t, "revenue-rca", listRes.Skills[1].Name) + require.Equal(t, []string{"orders"}, listRes.Skills[1].MetricsViews) + require.Equal(t, []string{"analyst"}, listRes.Skills[1].Agents) + + require.Len(t, listRes.Invalid, 3) + issues := map[string]string{} + for _, issue := range listRes.Invalid { + issues[issue.Path] = issue.Error + } + require.Contains(t, issues["/skills/broken.md"], "description") + require.Contains(t, issues["/skills/oversized.md"], "maximum skill size") + require.Contains(t, issues["/skills/zzz-dupe.md"], "duplicate skill name") + + // Load a skill by name + var loadRes *ai.LoadSkillResult + _, err = s.CallTool(t.Context(), ai.RoleUser, ai.LoadSkillName, &loadRes, &ai.LoadSkillArgs{Name: "revenue-rca"}) + require.NoError(t, err) + require.Equal(t, "Playbook for diagnosing revenue drops.", loadRes.Description) + require.Contains(t, loadRes.Body, "Always break revenue down by country first.") + require.NotContains(t, loadRes.Body, "---") + + // Load an unknown skill: the error lists the available names + _, err = s.CallTool(t.Context(), ai.RoleUser, ai.LoadSkillName, &loadRes, &ai.LoadSkillArgs{Name: "nope"}) + require.ErrorContains(t, err, "glossary, revenue-rca") +} + +func TestSkillsEmptyProject(t *testing.T) { + rt, instanceID := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{}) + s := newSession(t, rt, instanceID) + + var listRes *ai.ListSkillsResult + _, err := s.CallTool(t.Context(), ai.RoleUser, ai.ListSkillsName, &listRes, &ai.ListSkillsArgs{}) + require.NoError(t, err) + require.Empty(t, listRes.Skills) + require.Empty(t, listRes.Invalid) + + var loadRes *ai.LoadSkillResult + _, err = s.CallTool(t.Context(), ai.RoleUser, ai.LoadSkillName, &loadRes, &ai.LoadSkillArgs{Name: "anything"}) + require.ErrorContains(t, err, "does not define any skills") +} + +// TestSkillsMCPAccess verifies that the skill tools are exposed to any principal with UseAI, +// including viewers without repo access. +func TestSkillsMCPAccess(t *testing.T) { + rt, instanceID := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{}) + + newMCPSession := func(t *testing.T, permissions ...runtime.Permission) *ai.Session { + claims := &runtime.SecurityClaims{ + UserID: uuid.NewString(), + SkipChecks: false, + Permissions: permissions, + } + r := ai.NewRunner(rt, activity.NewNoopClient()) + s, err := r.Session(t.Context(), &ai.SessionOptions{ + InstanceID: instanceID, + Claims: claims, + UserAgent: "mcp-client", + }) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, s.Flush(t.Context())) + }) + return s + } + + assertAccess := func(t *testing.T, s *ai.Session, name string, want bool) { + t.Helper() + tool, ok := s.Tool(name) + require.True(t, ok, "tool %q should be registered", name) + allowed, err := tool.CheckAccess(ai.WithSession(t.Context(), s)) + require.NoError(t, err) + require.Equal(t, want, allowed, "tool %q access", name) + } + + // A viewer-like claim set (no ReadRepo/EditRepo) can use the skill tools + s := newMCPSession(t, runtime.UseAI, runtime.ReadMetrics, runtime.ReadObjects) + assertAccess(t, s, ai.ListSkillsName, true) + assertAccess(t, s, ai.LoadSkillName, true) + + // Without UseAI, the skill tools are not accessible + s = newMCPSession(t, runtime.ReadMetrics, runtime.ReadObjects) + assertAccess(t, s, ai.ListSkillsName, false) + assertAccess(t, s, ai.LoadSkillName, false) +} diff --git a/web-common/src/features/chat/core/messages/tools/tool-icons.ts b/web-common/src/features/chat/core/messages/tools/tool-icons.ts index b52803f894b4..294631a4fa35 100644 --- a/web-common/src/features/chat/core/messages/tools/tool-icons.ts +++ b/web-common/src/features/chat/core/messages/tools/tool-icons.ts @@ -9,6 +9,7 @@ import { FolderTree, Pencil, CornerDownRight, + GraduationCap, } from "lucide-svelte"; import type { ComponentType } from "svelte"; import Chart from "../../../../../components/icons/Chart.svelte"; @@ -37,6 +38,8 @@ export const TOOL_ICONS: Record = { // Common tools [ToolName.NAVIGATE]: CornerDownRight, + [ToolName.LIST_SKILLS]: GraduationCap, + [ToolName.LOAD_SKILL]: GraduationCap, }; /** diff --git a/web-common/src/features/chat/core/types.ts b/web-common/src/features/chat/core/types.ts index 17bfe399ce02..bd9e96fe5dcb 100644 --- a/web-common/src/features/chat/core/types.ts +++ b/web-common/src/features/chat/core/types.ts @@ -58,6 +58,8 @@ export const ToolName = { // Common tools NAVIGATE: "navigate", + LIST_SKILLS: "list_skills", + LOAD_SKILL: "load_skill", } as const; // ============================================================================= diff --git a/web-common/src/features/entity-management/add/AddAssetButton.svelte b/web-common/src/features/entity-management/add/AddAssetButton.svelte index cb13eab6e678..729422c5c4de 100644 --- a/web-common/src/features/entity-management/add/AddAssetButton.svelte +++ b/web-common/src/features/entity-management/add/AddAssetButton.svelte @@ -5,6 +5,7 @@ Database, File, Folder, + GraduationCap, PlusCircleIcon, Wand, } from "lucide-svelte"; @@ -27,7 +28,7 @@ import { useRuntimeClient } from "../../../runtime-client/v2"; import { useIsModelingSupportedForDefaultOlapDriverOLAP as useIsModelingSupportedForDefaultOlapDriver } from "../../connectors/selectors.ts"; import { directoryState } from "../../file-explorer/directory-store.ts"; - import { createResourceAndNavigate } from "./new-files.ts"; + import { createResourceAndNavigate, skillFileTemplate } from "./new-files.ts"; import AddAiConnectorDialog from "../../connectors/ai/AddAiConnectorDialog.svelte"; import CreateExploreDialog from "./CreateExploreDialog.svelte"; import { removeLeadingSlash } from "../entity-mappers.ts"; @@ -73,6 +74,8 @@ currentDirectory, ); + $: skillsFileNamesQuery = useFileNamesInDirectory(runtimeClient, "skills"); + $: isModelingSupportedForDefaultOlapDriver = useIsModelingSupportedForDefaultOlapDriver(runtimeClient); $: isModelingSupported = $isModelingSupportedForDefaultOlapDriver.data; @@ -139,6 +142,25 @@ await navigateToFile(`/${path}`); } + + /** + * Put a skill file (markdown instructions for the AI agents) in the skills directory + */ + async function handleAddSkill() { + const existingNames = ($skillsFileNamesQuery?.data ?? []).map((name) => + name.replace(/\.md$/, ""), + ); + const path = `skills/${getName("my_skill", existingNames)}.md`; + + await $createFile.mutateAsync({ + path, + blob: skillFileTemplate, + create: true, + createOnly: true, + }); + + await navigateToFile(`/${path}`); + } @@ -296,6 +318,11 @@ /> Theme + + + + AI Skill +