Generate chart yaml using AI #4323

AdityaHegde · 2024-03-12T16:09:18Z

Endpoint to send prompt to chatgpt.
Parse chatgpt response and generate the yaml (happens in the UI right now).
Hook up a menu item to chart to generate the vega lite chart spec.
Support generating a chart directly from a table/model.
Support generating a chart directly from a metrics view.

These will have a follow up PR

Prompt tuning.

runtime/resolver.go

runtime/server/generate_chart.go

proto/rill/runtime/v1/api.proto

begelundmuller · 2024-03-14T08:44:34Z

runtime/server/generate_chart.go

+Replace the data field in vega lite json with,
+{ "name": "table" }


With metrics view generation, it seemed that less fixed rule were better. So maybe consider saying "don't add a 'data' field". And then just add this programmatically afterwards.

One thing I am seeing is chatgpt does a good job of giving proper indentation. Parsing, updating and reformatting will lose this. So probably it might be good to keep it as is.

Okay, you can keep if you want. But note that Go's JSON encoder is able to output indented JSON.

While true, I dont think any library can do partial indentation like,

{ "$schema": "https://vega.github.io/schema/vega-lite/v5.json", "data": { "name": "table" }, "mark": "arc", "encoding": { "theta": {"field": "total_records", "type": "quantitative"}, "color": {"field": "publisher", "type": "nominal"} }, "width": 300, "height": 300 }

That's probably true :) Makes sense

runtime/server/generate_chart.go

begelundmuller · 2024-03-16T11:11:22Z

proto/rill/runtime/v1/api.proto

+  rpc GenerateResolver(GenerateResolverRequest) returns (GenerateResolverResponse) {
+    option (google.api.http) = {
+      post: "/v1/instances/{instance_id}/files/generate-resolver",
+      body: "*"
+    };
+  }
+
+  rpc GenerateChartSpec(GenerateChartSpecRequest) returns (GenerateChartSpecResponse) {
+    option (google.api.http) = {
+      post: "/v1/instances/{instance_id}/files/generate-chart",
+      body: "*"
+    };
+  }


Add a docstring for each API (can be short, like for the other APIs) :)

Since these endpoints don't generate files, they shouldn't use the /files/ path. For consistency between all the generator APIs, you might also consider refactoring GenerateMetricsViewFile to GenerateMetricsView and have the frontend write the file.

for 2 will raise a separate PR to refactor GenerateMetricsViewFile. This PR is already getting large.

begelundmuller · 2024-03-16T11:12:53Z

proto/rill/runtime/v1/api.proto

+  string table = 3;
+  string connector = 4;


nit: define connector before table – that's the order in all other protos with these two props

begelundmuller · 2024-03-16T11:13:31Z

proto/rill/runtime/v1/api.proto

+  string prompt = 2;
+  string table = 3;
+  string connector = 4;
+  string metrics_view = 5;


Maybe add a comment that if this is set, both connector and table should be empty

begelundmuller · 2024-03-16T11:15:17Z

runtime/resolver.go

@@ -86,7 +95,7 @@ type ResolveOptions struct {
 }

 // Resolve resolves a query using the given options.
-func (r *Runtime) Resolve(ctx context.Context, opts *ResolveOptions) ([]byte, error) {
+func (r *Runtime) Resolve(ctx context.Context, opts *ResolveOptions) (*CachedResolverResult, error) {


Would suggest not using a pointer, this is a very light struct

begelundmuller · 2024-03-16T11:17:06Z

runtime/resolver.go

+// CachedResolverResult is subset of ResolverResult that is cached
+type CachedResolverResult struct {


Would suggest calling this ResolveResult because it is a result returned from Resolve (note: not "resolve", not "resolver"). And then move the definition to after the ResolveOptions` struct.

"Cached" is an implementation detail, and it will actually not always have been served from cache :)

begelundmuller · 2024-03-16T11:24:38Z

runtime/server/generate_chart.go

+Replace the data field in vega lite json with,
+{ "name": "table" }


Okay, you can keep if you want. But note that Go's JSON encoder is able to output indented JSON.

begelundmuller · 2024-03-16T11:27:18Z

runtime/server/generate_chart.go

+func marshalResolverProperties(resolverProperties map[string]interface{}) string {
+	if sql, ok := resolverProperties["sql"]; ok {
+		if sqlStr, ok := sql.(string); ok {
+			return sqlStr
+		}
+	}
+
+	if api, ok := resolverProperties["api"]; ok {
+		if apiStr, ok := api.(string); ok {
+			return apiStr
+		}
+	}
+
+	return ""
+}


The goal of the resolver abstraction is to avoid resolver-specific switches littered across the codebase. So can we change this to a normal JSON serialization?

begelundmuller · 2024-03-16T11:27:29Z

runtime/server/generate_resolver.go

+		attribute.String("args.connector", req.Connector),
+		attribute.String("args.table", req.Table),
+		attribute.String("args.metrics_view", req.MetricsView),
+		// attribute.String("args.prompt", req.Prompt), // Adding this might be a privacy issue


Same comment as in the other API

begelundmuller · 2024-03-16T11:29:30Z

runtime/server/generate_resolver.go

+
+		start := time.Now()
+
+		sql, err := s.generateResolverForTable(ctx, req.InstanceId, req.Prompt, tbl.Name, dialect, tbl.Schema)


Would suggest returning resolver, resolverProps, err here instead of sql (otherwise the function should be generateSQLResolverForTable)

begelundmuller · 2024-03-16T11:39:20Z

runtime/server/generate_resolver.go

+		sql, err := s.generateResolverForMetricsView(ctx, req.InstanceId, req.Prompt, req.MetricsView, dialect, q.Result.Schema)
+		if err != nil {
+			return nil, err
+		}


Same comment – mismatch between func name and return value

AdityaHegde added 4 commits March 12, 2024 21:37

Add an endpoint to generate a chart yaml

c9276b6

Merge branch 'main' into adityahegde/custom-chart-using-ai

def17cb

Adding simple UI to add prompts

c0eab5e

Add support for generating from table/model

e32e9a3

AdityaHegde force-pushed the adityahegde/custom-chart-using-ai branch from 74af5f5 to e32e9a3 Compare March 13, 2024 07:56

AdityaHegde added 2 commits March 13, 2024 20:19

Rename a few things

fffbda4

Improve the yaml generation

d64fc31

AdityaHegde marked this pull request as ready for review March 13, 2024 15:31

AdityaHegde requested a review from begelundmuller March 13, 2024 15:31

begelundmuller requested changes Mar 14, 2024

View reviewed changes

AdityaHegde added 2 commits March 14, 2024 17:16

Split generate into 2 APIs

1d4fb3b

Update modals to be async

a4d5fdc

AdityaHegde force-pushed the adityahegde/custom-chart-using-ai branch from b2bb265 to a4d5fdc Compare March 14, 2024 12:51

AdityaHegde added 2 commits March 14, 2024 18:34

Add telemetry to APIs

063abdb

Fix lint

024e3a0

AdityaHegde force-pushed the adityahegde/custom-chart-using-ai branch from d0e92c6 to 024e3a0 Compare March 15, 2024 05:47

AdityaHegde added 2 commits March 15, 2024 15:48

Merge branch 'main' into adityahegde/custom-chart-using-ai

c6c6572

Main merge fixes

8a0f5a8

AdityaHegde force-pushed the adityahegde/custom-chart-using-ai branch from d3fdec9 to 8a0f5a8 Compare March 15, 2024 10:36

begelundmuller requested changes Mar 16, 2024

View reviewed changes

AdityaHegde added 2 commits March 18, 2024 11:02

Merge branch 'main' into adityahegde/custom-chart-using-ai

0e9c194

PR comments

ca7fd31

AdityaHegde force-pushed the adityahegde/custom-chart-using-ai branch from 07263f8 to ca7fd31 Compare March 18, 2024 06:30

Adding prompt history

18d4276

AdityaHegde requested a review from djbarnwal March 18, 2024 09:51

AdityaHegde added 5 commits March 18, 2024 15:43

Fix job

de2cd5b

Fix trailing semicolon

897de31

Merge branch 'main' into adityahegde/custom-chart-using-ai

e4e5567

Update label

00be544

Show prompt errors

61c03ea

AdityaHegde force-pushed the adityahegde/custom-chart-using-ai branch from 46a9209 to 61c03ea Compare March 21, 2024 06:02

djbarnwal approved these changes Mar 21, 2024

View reviewed changes

AdityaHegde added 2 commits March 25, 2024 13:30

Merge branch 'main' into adityahegde/custom-chart-using-ai

cc30c9c

Merge branch 'main' into adityahegde/custom-chart-using-ai

b2e05bd

begelundmuller approved these changes Mar 25, 2024

View reviewed changes

AdityaHegde merged commit 8315aec into main Mar 25, 2024
7 checks passed

AdityaHegde deleted the adityahegde/custom-chart-using-ai branch March 25, 2024 11:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate chart yaml using AI #4323

Generate chart yaml using AI #4323

AdityaHegde commented Mar 12, 2024 •

edited

Loading

begelundmuller Mar 14, 2024

AdityaHegde Mar 14, 2024

begelundmuller Mar 16, 2024

AdityaHegde Mar 18, 2024

begelundmuller Mar 25, 2024

begelundmuller Mar 16, 2024

AdityaHegde Mar 18, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

begelundmuller Mar 16, 2024

		Replace the data field in vega lite json with,
		{ "name": "table" }

		// CachedResolverResult is subset of ResolverResult that is cached
		type CachedResolverResult struct {


		start := time.Now()

		sql, err := s.generateResolverForTable(ctx, req.InstanceId, req.Prompt, tbl.Name, dialect, tbl.Schema)

Generate chart yaml using AI #4323

Generate chart yaml using AI #4323

Conversation

AdityaHegde commented Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdityaHegde commented Mar 12, 2024 •

edited

Loading