diff --git a/internal/gaby/actionlog.go b/internal/gaby/actionlog.go index 430846b..487b8f7 100644 --- a/internal/gaby/actionlog.go +++ b/internal/gaby/actionlog.go @@ -5,7 +5,6 @@ package main import ( - "bytes" "encoding/json" "errors" "fmt" @@ -19,10 +18,10 @@ import ( "golang.org/x/oscar/internal/storage" ) -var _ page = actionLogPage{} - // actionLogPage is the data for the action log HTML template. type actionLogPage struct { + CommonPage + Start, End endpoint StartTime, EndTime string // formatted times that the endpoints describe Entries []*actions.Entry @@ -79,11 +78,30 @@ func (g *Gaby) doActionLog(r *http.Request) (content []byte, status int, err err page.End.Radio = "fixed" } - var buf bytes.Buffer - if err := actionLogPageTmpl.Execute(&buf, page); err != nil { + page.setCommonPage() + + b, err := Exec(actionLogPageTmpl, &page) + if err != nil { return nil, http.StatusInternalServerError, err } - return buf.Bytes(), http.StatusOK, nil + return b, http.StatusOK, nil +} + +func (p *actionLogPage) setCommonPage() { + p.CommonPage = *p.toCommonPage() +} + +func (p *actionLogPage) toCommonPage() *CommonPage { + return &CommonPage{ + ID: actionlogID, + Description: "Browse actions taken by Oscar.", + Form: Form{ + // Unset because the action log page defines its form inputs + // directly in an HTML template. + Inputs: nil, + SubmitText: "display", + }, + } } // formValues populates an endpoint from the values in the form. diff --git a/internal/gaby/actionlog_test.go b/internal/gaby/actionlog_test.go index d6a5585..a4833bf 100644 --- a/internal/gaby/actionlog_test.go +++ b/internal/gaby/actionlog_test.go @@ -5,7 +5,6 @@ package main import ( - "bytes" "context" "strings" "testing" @@ -95,7 +94,6 @@ func TestTimes(t *testing.T) { } func TestActionTemplate(t *testing.T) { - var buf bytes.Buffer page := actionLogPage{ Start: endpoint{DurNum: "3", DurUnit: "days"}, StartTime: "whatevs", @@ -107,10 +105,12 @@ func TestActionTemplate(t *testing.T) { }, }, } - if err := actionLogPageTmpl.Execute(&buf, page); err != nil { + page.setCommonPage() + b, err := Exec(actionLogPageTmpl, &page) + if err != nil { t.Fatal(err) } - got := buf.String() + got := string(b) wants := []string{ ``, `Project`, diff --git a/internal/gaby/common_page.go b/internal/gaby/common_page.go new file mode 100644 index 0000000..743a78e --- /dev/null +++ b/internal/gaby/common_page.go @@ -0,0 +1,138 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "bytes" + + "github.com/google/safehtml" + "github.com/google/safehtml/template" +) + +// Exec executes the given template on the page. +func Exec(tmpl *template.Template, p page) ([]byte, error) { + var buf bytes.Buffer + if err := tmpl.Execute(&buf, p); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// page is a Gaby webpage containing a [CommonPage]. +// Any struct that embeds a [CommonPage] implements this interface. +type page interface { + // do not directly define [isCommonPage]. + isCommonPage() +} + +// A CommonPage is a partial representation of a Gaby web page, +// used to store data that is common to many pages. +// The templates in tmpl/common.tmpl are defined on this type. +type CommonPage struct { + // The ID of the page. + ID pageID + // A plain text description of the webpage. + Description string + // A list of additional stylesheets to use for this webpage. + // "/static/style.css" and [pageID.CSS] are always included + // without needing to be listed here. + Styles []safeURL + // The input form. + Form Form +} + +// Implements [page.isCommonPage]. +func (*CommonPage) isCommonPage() {} + +// A Form is a representation of an HTML form. +type Form struct { + // (Optional) Description and/or general tips for filling out the form. + Description string + // The text to display on the form's submit button. + SubmitText string + // The form's inputs. + Inputs []FormInput +} + +// A FormInput represents an input (or a group of inputs) +// to an HTML form. +type FormInput struct { + Label string // display text + Type string // type to display to the user (for tips section) + Description string // description of the input and its usage (for tips section) + + Name safeID // HTML "name" + Required bool // whether the input is required + + // Additional data, based on the type of form input. + Typed typedInput +} + +type typedInput interface { + InputType() string +} + +// TextInput is a an HTML "text" input. +type TextInput struct { + ID safeID // HTML "id" + Value string // HTML "value" +} + +// Implments [typedInput.InputType]. +func (TextInput) InputType() string { + return "text" +} + +// RadioInput is a collection of HTML "radio" inputs. +type RadioInput struct { + Choices []RadioChoice +} + +// Implements [typedInput.InputType]. +func (RadioInput) InputType() string { + return "radio" +} + +// RadioChoice is a single HTML "radio" input. +type RadioChoice struct { + Label string // display text + ID safeID // HTML "id" + Value string // HTML "value" + Checked bool // whether the button should be checked +} + +type pageID string + +func (p pageID) Endpoint() string { + return "/" + string(p) +} + +func (p pageID) Title() string { + if t, ok := titles[p]; ok { + return t + } + return string(p) +} + +func (p pageID) CSS() safeURL { + const cssFmt = "/static/%{p}.css" + u, err := safehtml.TrustedResourceURLFormatFromConstant(cssFmt, map[string]string{"p": string(p)}) + if err != nil { + panic(err) + } + return u +} + +// Shorthands for safehtml types. +type ( + safeID = safehtml.Identifier + safeURL = safehtml.TrustedResourceURL +) + +// Shorthands for safehtml functions. +var ( + toSafeID = safehtml.IdentifierFromConstant + toSafeURL = safehtml.TrustedResourceURLFromConstant +) diff --git a/internal/gaby/dbview.go b/internal/gaby/dbview.go index f868ecc..c0f424a 100644 --- a/internal/gaby/dbview.go +++ b/internal/gaby/dbview.go @@ -15,18 +15,18 @@ import ( "rsc.io/ordered" ) -var _ page = dbviewPage{} - // dbviewPage holds the fields needed to display a view of the database. type dbviewPage struct { - Form dbviewForm // the raw form inputs + CommonPage + + Params dbviewParams // the raw parameters Result *dbviewResult Error error // if non-nil, the error to display instead of the result } -type dbviewForm struct { - Start, End string // comma-separated lists; see parseOredered for details - Limit int +type dbviewParams struct { + Start, End string // comma-separated lists; see [parseOrdered] for details + Limit string // the maximum number of values to display } type dbviewResult struct { @@ -46,17 +46,18 @@ func (g *Gaby) handleDBview(w http.ResponseWriter, r *http.Request) { } // populateDBviewPage returns the contents of the dbView page. -func (g *Gaby) populateDBviewPage(r *http.Request) dbviewPage { - limit := parseInt(r.FormValue("limit"), 100) - p := dbviewPage{ - Form: dbviewForm{ +func (g *Gaby) populateDBviewPage(r *http.Request) *dbviewPage { + p := &dbviewPage{ + Params: dbviewParams{ Start: r.FormValue("start"), End: r.FormValue("end"), - Limit: limit, + Limit: formValue(r, "limit", "100"), }, } - start := parseOrdered(p.Form.Start) - end := parseOrdered(p.Form.End) + p.setCommonPage() + limit := parseInt(p.Params.Limit, 100) + start := parseOrdered(p.Params.Start) + end := parseOrdered(p.Params.End) g.slog.Info("calling dbview", "limit", limit) res, err := g.dbview(start, end, limit) g.slog.Info("done") @@ -68,6 +69,19 @@ func (g *Gaby) populateDBviewPage(r *http.Request) dbviewPage { return p } +func (p *dbviewPage) setCommonPage() { + p.CommonPage = CommonPage{ + ID: dbviewID, + Description: "View the database contents.", + Form: Form{ + Description: `Provide one key to get a single value, or two to get a range. +Keys are comma-separated lists of strings, integers, "inf" or "-inf".`, + Inputs: p.Params.inputs(), + SubmitText: "Show", + }, + } +} + func (g *Gaby) dbview(start, end []byte, limit int) (*dbviewResult, error) { if len(start) == 0 && len(end) > 0 { return nil, errors.New("missing start key") @@ -144,3 +158,55 @@ func parseInt(s string, defaultValue int) int { } return defaultValue } + +// formValue returns the form value for the key, or defaultValue +// if the form value is empty. +func formValue(r *http.Request, key string, defaultValue string) string { + if v := r.FormValue(key); v != "" { + return v + } + return defaultValue +} + +var ( + safeStart = toSafeID("start") + safeEnd = toSafeID("end") +) + +func (pm *dbviewParams) inputs() []FormInput { + return []FormInput{ + { + Label: "Get", + Type: "db key", + Description: "the starting db key", + Name: safeStart, + Required: true, + Typed: TextInput{ + ID: safeStart, + Value: pm.Start, + }, + }, + { + Label: "To", + Type: "db key", + Description: "the ending db key", + Name: safeEnd, + // optional + Typed: TextInput{ + ID: safeEnd, + Value: pm.End, + }, + }, + { + Label: "Limit", + Type: "int", + Description: "the maximum number of values to display (default: 100)", + Name: safeLimit, + Required: true, + Typed: TextInput{ + ID: safeLimit, + Value: pm.Limit, + }, + }, + } +} diff --git a/internal/gaby/main.go b/internal/gaby/main.go index 903f28a..276783d 100644 --- a/internal/gaby/main.go +++ b/internal/gaby/main.go @@ -493,30 +493,34 @@ func (g *Gaby) newServer(report func(error)) *http.ServeMux { } }) + get := func(p pageID) string { + return "GET " + p.Endpoint() + } + // /search: display a form for vector similarity search. // /search?q=...: perform a search using the value of q as input. - mux.HandleFunc("GET /search", g.handleSearch) + mux.HandleFunc(get(searchID), g.handleSearch) // /overview: display a form for LLM-generated overviews of data. // /overview?q=...: generate an overview using the value of q as input. - mux.HandleFunc("GET /overview", g.handleOverview) + mux.HandleFunc(get(overviewID), g.handleOverview) // /rules: display a form for entering an issue to check for rule violations. // /rules?q=...: generate a list of violated rules for issue q. - mux.HandleFunc("GET /rules", g.handleRules) + mux.HandleFunc(get(rulesID), g.handleRules) // /api/search: perform a vector similarity search. // POST because the arguments to the request are in the body. mux.HandleFunc("POST /api/search", g.handleSearchAPI) // /actionlog: display action log - mux.HandleFunc("GET /actionlog", g.handleActionLog) + mux.HandleFunc(get(actionlogID), g.handleActionLog) // /reviews: display review dashboard - mux.HandleFunc("GET /reviews", g.handleReviewDashboard) + mux.HandleFunc(get(reviewsID), g.handleReviewDashboard) // /dbview: view parts of the database - mux.HandleFunc("GET /dbview", g.handleDBview) + mux.HandleFunc(get(dbviewID), g.handleDBview) return mux } diff --git a/internal/gaby/overview.go b/internal/gaby/overview.go index 7471c5f..8d759a5 100644 --- a/internal/gaby/overview.go +++ b/internal/gaby/overview.go @@ -19,12 +19,12 @@ import ( "golang.org/x/oscar/internal/search" ) -var _ page = overviewPage{} - // overviewPage holds the fields needed to display the results // of a search. type overviewPage struct { - Form overviewForm // the raw form inputs + CommonPage + + Params overviewParams // the raw query params Result *overviewResult Error error // if non-nil, the error to display instead of the result } @@ -34,41 +34,30 @@ type overviewResult struct { Type string // the type of overview } -// overviewForm holds the raw inputs to the overview form. -type overviewForm struct { +// overviewParams holds the raw HTML parameters. +type overviewParams struct { Query string // the issue ID to lookup, or golang/go#12345 or github.com/golang/go/issues/12345 form OverviewType string // the type of overview to generate } // the possible overview types const ( - issueOverviewType = "issue" - relatedOverviewType = "related" + issueOverviewType = "issue_overview" + relatedOverviewType = "related_overview" ) +// validOverviewType reports whether the given type +// is a recognized overview type. +func validOverviewType(t string) bool { + return t == issueOverviewType || t == relatedOverviewType +} + // IsIssueOverview reports whether this overview result // is of type [issueOverviewType]. func (r *overviewResult) IsIssueOverview() bool { return r.Type == issueOverviewType } -// CheckRadio reports whether radio button with the given id -// should be checked. -func (p overviewPage) CheckRadio(id string) bool { - // checked returns the id of the radio button that should be checked. - checked := func() string { - // If there is no result yet, the default option - // (issue overview) should be checked. - if p.Result == nil { - return issueOverviewType - } - // Otherwise, the button corresponding to the result - // type should be checked. - return p.Result.Type - } - return id == checked() -} - func (g *Gaby) handleOverview(w http.ResponseWriter, r *http.Request) { handlePage(w, g.populateOverviewPage(r), overviewPageTmpl) } @@ -131,14 +120,20 @@ func parseIssueNumber(issueID string) (project string, issue int64, _ error) { } // populateOverviewPage returns the contents of the overview page. -func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage { - p := overviewPage{ - Form: overviewForm{ - Query: r.FormValue("q"), - OverviewType: r.FormValue("t"), - }, +func (g *Gaby) populateOverviewPage(r *http.Request) *overviewPage { + pm := overviewParams{ + Query: r.FormValue(paramQuery), + OverviewType: r.FormValue(paramOverviewType), + } + p := &overviewPage{ + Params: pm, + } + p.setCommonPage() + q := trim(p.Params.Query) + if q == "" { + return p } - proj, issue, err := parseIssueNumber(p.Form.Query) + proj, issue, err := parseIssueNumber(p.Params.Query) if err != nil { p.Error = fmt.Errorf("invalid form value: %v", err) return p @@ -147,13 +142,10 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage { proj = g.githubProjects[0] // default to first project. } if !slices.Contains(g.githubProjects, proj) { - p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", p.Form.Query) - return p - } - if issue <= 0 { + p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", p.Params.Query) return p } - overview, err := g.overview(r.Context(), proj, issue, p.Form.OverviewType) + overview, err := g.overview(r.Context(), proj, issue, p.Params.OverviewType) if err != nil { p.Error = err return p @@ -162,6 +154,76 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage { return p } +func (p *overviewPage) setCommonPage() { + p.CommonPage = CommonPage{ + ID: overviewID, + Description: "Generate overviews of golang/go issues and their comments, or summarize the relationship between a golang/go issue and its related documents.", + Styles: []safeURL{searchID.CSS()}, + Form: Form{ + Inputs: p.Params.inputs(), + SubmitText: "generate", + }, + } +} + +const ( + paramOverviewType = "t" +) + +// inputs converts the params to HTML form inputs. +func (pm *overviewParams) inputs() []FormInput { + return []FormInput{ + { + Label: "issue", + Type: "int or string", + Description: "the issue to summarize, as a number or URL (e.g. 1234, golang/go#1234, or https://github.com/golang/go/issues/1234)", + Name: safeQuery, + Required: true, + Typed: TextInput{ + ID: safeQuery, + Value: pm.Query, + }, + }, + { + Label: "overview type", + Type: "radio choice", + Description: `"issue and comments" generates an overview of the issue and its comments; "related documents" searches for related documents and summarizes them`, + Name: toSafeID(paramOverviewType), + Required: true, + Typed: RadioInput{ + Choices: []RadioChoice{ + { + Label: "issue overview", + ID: toSafeID(issueOverviewType), + Value: issueOverviewType, + Checked: pm.checkRadio(issueOverviewType), + }, + { + Label: "related documents", + ID: toSafeID(relatedOverviewType), + Value: relatedOverviewType, + Checked: pm.checkRadio(relatedOverviewType), + }, + }, + }, + }, + } +} + +// checkRadio reports whether radio button with the given value +// should be checked. +func (f *overviewParams) checkRadio(value string) bool { + // If the overview type is not set, or is set to an invalid value, + // the default option (issue overview) should be checked. + if !validOverviewType(f.OverviewType) { + return value == issueOverviewType + } + + // Otherwise, the button corresponding to the result + // type should be checked. + return value == f.OverviewType +} + // overview generates an overview of the issue of the given type. func (g *Gaby) overview(ctx context.Context, proj string, issue int64, overviewType string) (*overviewResult, error) { switch overviewType { diff --git a/internal/gaby/overview_test.go b/internal/gaby/overview_test.go index 071ad69..37500d6 100644 --- a/internal/gaby/overview_test.go +++ b/internal/gaby/overview_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/safehtml" "golang.org/x/oscar/internal/docs" "golang.org/x/oscar/internal/embeddocs" "golang.org/x/oscar/internal/github" @@ -86,12 +87,12 @@ func TestPopulateOverviewPage(t *testing.T) { for _, tc := range []struct { name string r *http.Request - want overviewPage + want *overviewPage }{ { name: "empty", r: &http.Request{}, - want: overviewPage{}, + want: &overviewPage{}, }, { name: "issue overview (default)", @@ -100,8 +101,8 @@ func TestPopulateOverviewPage(t *testing.T) { "q": {"1"}, }, }, - want: overviewPage{ - Form: overviewForm{ + want: &overviewPage{ + Params: overviewParams{ Query: "1", OverviewType: "", }, @@ -123,8 +124,8 @@ func TestPopulateOverviewPage(t *testing.T) { "t": {issueOverviewType}, }, }, - want: overviewPage{ - Form: overviewForm{ + want: &overviewPage{ + Params: overviewParams{ Query: "1", OverviewType: issueOverviewType, }, @@ -146,8 +147,8 @@ func TestPopulateOverviewPage(t *testing.T) { "t": {relatedOverviewType}, }, }, - want: overviewPage{ - Form: overviewForm{ + want: &overviewPage{ + Params: overviewParams{ Query: "1", OverviewType: relatedOverviewType, }, @@ -168,8 +169,8 @@ func TestPopulateOverviewPage(t *testing.T) { "t": {relatedOverviewType}, }, }, - want: overviewPage{ - Form: overviewForm{ + want: &overviewPage{ + Params: overviewParams{ Query: "3", OverviewType: relatedOverviewType, }, @@ -184,8 +185,8 @@ func TestPopulateOverviewPage(t *testing.T) { "t": {relatedOverviewType}, }, }, - want: overviewPage{ - Form: overviewForm{ + want: &overviewPage{ + Params: overviewParams{ Query: "unknown/project#3", OverviewType: relatedOverviewType, }, @@ -195,9 +196,11 @@ func TestPopulateOverviewPage(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { got := g.populateOverviewPage(tc.r) + tc.want.setCommonPage() if diff := cmp.Diff(got, tc.want, cmpopts.IgnoreFields(llmapp.OverviewResult{}, "Cached"), - cmpopts.EquateErrors()); diff != "" { + cmpopts.EquateErrors(), + safeHTMLcmpopt); diff != "" { t.Errorf("Gaby.populateOverviewPage() mismatch (-got +want):\n%s", diff) } }) @@ -205,6 +208,8 @@ func TestPopulateOverviewPage(t *testing.T) { } +var safeHTMLcmpopt = cmpopts.EquateComparable(safehtml.TrustedResourceURL{}, safehtml.Identifier{}) + func TestParseOverviewPageQuery(t *testing.T) { tests := []struct { in string diff --git a/internal/gaby/page.go b/internal/gaby/page.go deleted file mode 100644 index 9ac9c5b..0000000 --- a/internal/gaby/page.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2024 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package main - -import "github.com/google/safehtml" - -// A page is a Gaby web page. -// A type must implement this interface to re-use -// the templates defined in tmpl/common.tmpl. -type page interface { - // Title returns the title of the webpage. - Title() string - // Description returns a plain text description of the webpage. - Description() string - // NavID returns an identifier for the webpage as a [safeCSS]. - // NavID must be unique across Gaby's pages. - NavID() safeCSS - // Styles returns a list of stylesheets to use for this webpage, - // as [safeURL]s. - Styles() []safeURL -} - -// Shorthands safehtml types. -type ( - safeCSS = safehtml.StyleSheet - safeURL = safehtml.TrustedResourceURL -) - -// Implementations of [page]. - -func (actionLogPage) Title() string { return "Oscar Action Log" } -func (overviewPage) Title() string { return "Oscar Overviews" } -func (searchPage) Title() string { return "Oscar Search" } -func (dbviewPage) Title() string { return "Database Viewer" } - -func (actionLogPage) Description() string { return "Browse actions taken by Oscar." } -func (overviewPage) Description() string { - return "Generate overviews of golang/go issues and their comments, or summarize the relationship between a golang/go issue and its related documents." -} -func (searchPage) Description() string { - return "Search Oscar's database of GitHub issues, Go documentation, and other documents." -} -func (dbviewPage) Description() string { return "View the database contents." } - -func (actionLogPage) NavID() safeCSS { return safehtml.StyleSheetFromConstant("actionlog") } -func (overviewPage) NavID() safeCSS { return safehtml.StyleSheetFromConstant("overview") } -func (searchPage) NavID() safeCSS { return safehtml.StyleSheetFromConstant("search") } -func (dbviewPage) NavID() safeCSS { return safehtml.StyleSheetFromConstant("dbview") } - -var ( - styleCSS = safehtml.TrustedResourceURLFromConstant("static/style.css") - searchCSS = safehtml.TrustedResourceURLFromConstant("static/search.css") - actionLogCSS = safehtml.TrustedResourceURLFromConstant("static/actionlog.css") - overviewCSS = safehtml.TrustedResourceURLFromConstant("static/overview.css") - dbviewCSS = safehtml.TrustedResourceURLFromConstant("static/dbview.css") -) - -func (actionLogPage) Styles() []safeURL { return []safeURL{styleCSS, actionLogCSS} } -func (overviewPage) Styles() []safeURL { return []safeURL{styleCSS, searchCSS, overviewCSS} } -func (searchPage) Styles() []safeURL { return []safeURL{styleCSS, searchCSS} } -func (dbviewPage) Styles() []safeURL { return []safeURL{styleCSS, dbviewCSS} } diff --git a/internal/gaby/pages.go b/internal/gaby/pages.go new file mode 100644 index 0000000..854183a --- /dev/null +++ b/internal/gaby/pages.go @@ -0,0 +1,35 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +// The browsable Gaby webpages. +// Pages listed here will appear in navigation. +var pages = []pageID{ + // Dev pages. + actionlogID, dbviewID, + // User pages. + overviewID, searchID, rulesID, + // reviews omitted for now, as it loads very slowly +} + +// Gaby webpage endpoints. +const ( + actionlogID pageID = "actionlog" + overviewID pageID = "overview" + searchID pageID = "search" + dbviewID pageID = "dbview" + rulesID pageID = "rules" + reviewsID pageID = "reviews" +) + +// Gaby webpage titles. +var titles = map[pageID]string{ + actionlogID: "Action Log", + overviewID: "Overviews", + searchID: "Search", + dbviewID: "Database Viewer", + rulesID: "Rule Checker", + reviewsID: "Reviews", +} diff --git a/internal/gaby/rules.go b/internal/gaby/rules.go index c8107fc..fab8b56 100644 --- a/internal/gaby/rules.go +++ b/internal/gaby/rules.go @@ -20,9 +20,11 @@ import ( // rulesPage holds the fields needed to display the results // of an issue rule check. type rulesPage struct { - rulesForm // the raw form inputs - Result *rulesResult - Error string // if non-empty, the error to display instead of the result + CommonPage + + Params rulesParams // the raw parameters + Result *rulesResult + Error error // if non-nil, the error to display instead of the result } type rulesResult struct { @@ -31,8 +33,8 @@ type rulesResult struct { HTML safehtml.HTML // issue response as HTML } -// rulesForm holds the raw inputs to the rules form. -type rulesForm struct { +// rulesParams holds the raw inputs to the rules form. +type rulesParams struct { Query string // the issue ID to lookup } @@ -43,26 +45,27 @@ func (g *Gaby) handleRules(w http.ResponseWriter, r *http.Request) { var rulesPageTmpl = newTemplate(rulesPageTmplFile, template.FuncMap{}) // populateRulesPage returns the contents of the rules page. -func (g *Gaby) populateRulesPage(r *http.Request) rulesPage { - form := rulesForm{ - Query: r.FormValue("q"), +func (g *Gaby) populateRulesPage(r *http.Request) *rulesPage { + pm := rulesParams{ + Query: r.FormValue(paramQuery), } - p := rulesPage{ - rulesForm: form, + p := &rulesPage{ + Params: pm, } - if form.Query == "" { + p.setCommonPage() + if pm.Query == "" { return p } - proj, issue, err := parseIssueNumber(form.Query) + proj, issue, err := parseIssueNumber(pm.Query) if err != nil { - p.Error = fmt.Errorf("invalid form value %q: %w", form.Query, err).Error() + p.Error = fmt.Errorf("invalid form value %q: %w", pm.Query, err) return p } if proj == "" && len(g.githubProjects) > 0 { proj = g.githubProjects[0] // default to first project } if !slices.Contains(g.githubProjects, proj) { - p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", form.Query).Error() + p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", pm.Query) return p } if issue <= 0 { @@ -71,26 +74,49 @@ func (g *Gaby) populateRulesPage(r *http.Request) rulesPage { // Find issue in database. i, err := github.LookupIssue(g.db, proj, issue) if err != nil { - return rulesPage{ - rulesForm: form, - Error: fmt.Errorf("error looking up issue %q: %w", form.Query, err).Error(), - } + p.Error = fmt.Errorf("error looking up issue %q: %w", pm.Query, err) + return p } // TODO: this llm.TextGenerator cast is kind of ugly. Redo somehow. rules, err := rules.Issue(r.Context(), g.embed.(llm.TextGenerator), i) if err != nil { - return rulesPage{ - rulesForm: form, - Error: err.Error(), - } + p.Error = err + return p } - return rulesPage{ - rulesForm: form, - Result: &rulesResult{ - Issue: i, - IssueResult: *rules, - HTML: htmlutil.MarkdownToSafeHTML(rules.Response), + p.Result = &rulesResult{ + Issue: i, + IssueResult: *rules, + HTML: htmlutil.MarkdownToSafeHTML(rules.Response), + } + return p +} + +func (p *rulesPage) setCommonPage() { + p.CommonPage = CommonPage{ + ID: rulesID, + Description: "Generate a list of rule violations for submitted golang/go issues.", + Styles: []safeURL{searchID.CSS()}, + Form: Form{ + Inputs: p.Params.inputs(), + SubmitText: "generate", + }, + } +} + +func (pm *rulesParams) inputs() []FormInput { + return []FormInput{ + { + + Label: "issue", + Type: "int or string", + Description: "the issue to check, as a number or URL (e.g. 1234, golang/go#1234, or https://github.com/golang/go/issues/1234)", + Name: safeQuery, + Required: true, + Typed: TextInput{ + ID: safeQuery, + Value: pm.Query, + }, }, } } diff --git a/internal/gaby/search.go b/internal/gaby/search.go index 9617f60..fb1d7c6 100644 --- a/internal/gaby/search.go +++ b/internal/gaby/search.go @@ -5,7 +5,6 @@ package main import ( - "bytes" "context" "encoding/json" "fmt" @@ -19,57 +18,50 @@ import ( "golang.org/x/oscar/internal/search" ) -var _ page = actionLogPage{} - // a searchPage holds the fields needed to display the results // of a search. type searchPage struct { - searchForm // the raw query and options - Results []search.Result // the search results to display - SearchError string // if non-empty, the error to display instead of results + CommonPage + + Params searchParams // the raw query parameters + Results []search.Result // the search results to display + Error error // if non-nil, the error to display instead of results } func (g *Gaby) handleSearch(w http.ResponseWriter, r *http.Request) { - handlePage(w, g.populatePage(r), searchPageTmpl) + handlePage(w, g.populateSearchPage(r), searchPageTmpl) } -func handlePage(w http.ResponseWriter, page any, tmpl *template.Template) { - var buf bytes.Buffer - if err := tmpl.Execute(&buf, page); err != nil { +func handlePage(w http.ResponseWriter, p page, tmpl *template.Template) { + b, err := Exec(tmpl, p) + if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - _, _ = w.Write(buf.Bytes()) + _, _ = w.Write(b) } -// populatePage returns the contents of the vector search page. -func (g *Gaby) populatePage(r *http.Request) searchPage { - form := searchForm{ - Query: r.FormValue("q"), - Threshold: r.FormValue("threshold"), - Limit: r.FormValue("limit"), - Allow: r.FormValue("allow_kind"), - Deny: r.FormValue("deny_kind"), +// populateSearchPage returns the contents of the vector search page. +func (g *Gaby) populateSearchPage(r *http.Request) *searchPage { + var pm searchParams + pm.parseParams(r) + p := &searchPage{ + Params: pm, } - opts, err := form.toOptions() + p.setCommonPage() + opts, err := pm.toOptions() if err != nil { - return searchPage{ - searchForm: form, - SearchError: fmt.Errorf("invalid form value: %w", err).Error(), - } + p.Error = fmt.Errorf("invalid form value: %w", err) + return p } - q := strings.TrimSpace(form.Query) + q := trim(pm.Query) results, err := g.search(r.Context(), q, *opts) if err != nil { - return searchPage{ - searchForm: form, - SearchError: fmt.Errorf("search: %w", err).Error(), - } - } - return searchPage{ - searchForm: form, - Results: results, + p.Error = fmt.Errorf("search: %w", err) + return p } + p.Results = results + return p } // search performs a search on the query and options. @@ -109,8 +101,8 @@ func (g *Gaby) search(ctx context.Context, q string, opts search.Options) (resul return results, nil } -// searchForm holds the raw inputs to the search form. -type searchForm struct { +// searchParams holds the raw query parameters. +type searchParams struct { Query string // a text query, or an ID of a document in the database // String representations of the fields of [search.Options] @@ -119,14 +111,113 @@ type searchForm struct { Allow, Deny string // comma separated lists } -// toOptions converts a searchForm into a [search.Options], +// parseParams parses the query params from the request. +func (pm *searchParams) parseParams(r *http.Request) { + pm.Query = r.FormValue(paramQuery) + pm.Threshold = r.FormValue(paramThreshold) + pm.Limit = r.FormValue(paramLimit) + pm.Allow = r.FormValue(paramAllow) + pm.Deny = r.FormValue(paramDeny) +} + +func (p *searchPage) setCommonPage() { + p.CommonPage = CommonPage{ + ID: searchID, + Description: "Search Oscar's database of GitHub issues, Go documentation, and other documents.", + Form: Form{ + Inputs: p.Params.inputs(), + SubmitText: "search", + }, + } +} + +const ( + paramQuery = "q" + paramThreshold = "threshold" + paramLimit = "limit" + paramAllow = "allow_kind" + paramDeny = "deny_kind" +) + +var ( + safeQuery = toSafeID(paramQuery) + safeThreshold = toSafeID(paramThreshold) + safeLimit = toSafeID(paramLimit) + safeAllow = toSafeID(paramAllow) + safeDeny = toSafeID(paramDeny) +) + +// inputs converts the params into HTML form inputs. +func (pm *searchParams) inputs() []FormInput { + return []FormInput{ + { + + Label: "query", + Type: "string", + Description: "the text to search for neigbors of OR the ID (usually a URL) of a document in the vector database", + Name: safeQuery, + Required: true, + Typed: TextInput{ + ID: safeQuery, + Value: pm.Query, + }, + }, + { + + Label: "min similarity", + Type: "float64 between 0 and 1", + Description: "similarity cutoff (default: 0, allow all)", + Name: safeThreshold, + Typed: TextInput{ + ID: safeThreshold, + Value: pm.Threshold, + }, + }, + { + + Label: "max results", + Type: "int", + Description: "maximum number of results to display (default: 20)", + Name: safeLimit, + Typed: TextInput{ + ID: safeLimit, + Value: pm.Limit, + }, + }, + { + + Label: "include types", + Type: "comma-separated list", + Description: "document types to include, e.g `GitHubIssue,GoBlog` (default: empty, include all)", + Name: safeAllow, + Typed: TextInput{ + ID: safeAllow, + Value: pm.Allow, + }, + }, + { + + Label: "exclude types", + Type: "comma-separated list", + Description: "document types to filter out, e.g `GitHubIssue,GoBlog` (default: empty, exclude none)", + Name: safeDeny, + Typed: TextInput{ + ID: safeDeny, + Value: pm.Deny, + }, + }, + } +} + +var trim = strings.TrimSpace + +// toSearchOptions converts a searchParams into a [search.Options], // trimming any leading/trailing spaces. // // It returns an error if any of the form inputs is invalid. -func (f *searchForm) toOptions() (_ *search.Options, err error) { +func (f *searchParams) toOptions() (_ *search.Options, err error) { opts := &search.Options{} - trim := strings.TrimSpace splitAndTrim := func(s string) []string { vs := strings.Split(s, ",") for i, v := range vs { diff --git a/internal/gaby/search_test.go b/internal/gaby/search_test.go index 6abf88e..170d4c1 100644 --- a/internal/gaby/search_test.go +++ b/internal/gaby/search_test.go @@ -5,13 +5,15 @@ package main import ( - "bytes" "context" + "errors" "net/http" "reflect" "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "golang.org/x/oscar/internal/docs" "golang.org/x/oscar/internal/github" "golang.org/x/oscar/internal/llm" @@ -30,7 +32,7 @@ func TestSearchPageTemplate(t *testing.T) { { name: "results", page: searchPage{ - searchForm: searchForm{ + Params: searchParams{ Query: "some query", }, Results: []search.Result{ @@ -55,30 +57,31 @@ func TestSearchPageTemplate(t *testing.T) { { name: "error", page: searchPage{ - searchForm: searchForm{ + Params: searchParams{ Query: "some query", }, - SearchError: "some error", + Error: errors.New("some error"), }, }, { name: "no results", page: searchPage{ - searchForm: searchForm{ + Params: searchParams{ Query: "some query", }, }, }, } { t.Run(tc.name, func(t *testing.T) { - var buf bytes.Buffer - if err := searchPageTmpl.Execute(&buf, tc.page); err != nil { + tc.page.setCommonPage() + b, err := Exec(searchPageTmpl, &tc.page) + if err != nil { t.Fatal(err) } - got := buf.String() + got := string(b) if len(tc.page.Results) != 0 { - wants := []string{tc.page.Query} + wants := []string{tc.page.Params.Query} for _, sr := range tc.page.Results { wants = append(wants, sr.VectorResult.ID) } @@ -88,8 +91,8 @@ func TestSearchPageTemplate(t *testing.T) { t.Errorf("did not find %q in HTML", w) } } - } else if e := tc.page.SearchError; e != "" { - if !strings.Contains(got, e) { + } else if e := tc.page.Error; e != nil { + if !strings.Contains(got, e.Error()) { t.Errorf("did not find error %q in HTML", e) } } else { @@ -105,13 +108,13 @@ func TestSearchPageTemplate(t *testing.T) { func TestToOptions(t *testing.T) { tests := []struct { name string - form searchForm + form searchParams want *search.Options wantErr bool }{ { name: "basic", - form: searchForm{ + form: searchParams{ Threshold: ".55", Limit: "10", Allow: "GoBlog,GoDevPage,GitHubIssue", @@ -126,13 +129,13 @@ func TestToOptions(t *testing.T) { }, { name: "empty", - form: searchForm{}, + form: searchParams{}, // this will cause search to use defaults want: &search.Options{}, }, { name: "trim spaces", - form: searchForm{ + form: searchParams{ Threshold: " .55 ", Limit: " 10 ", Allow: " GoBlog, GoDevPage,GitHubIssue ", @@ -147,42 +150,42 @@ func TestToOptions(t *testing.T) { }, { name: "unparseable limit", - form: searchForm{ + form: searchParams{ Limit: "1.xx", }, wantErr: true, }, { name: "invalid limit", - form: searchForm{ + form: searchParams{ Limit: "1.33", }, wantErr: true, }, { name: "unparseable threshold", - form: searchForm{ + form: searchParams{ Threshold: "1x", }, wantErr: true, }, { name: "invalid threshold", - form: searchForm{ + form: searchParams{ Threshold: "-10", }, wantErr: true, }, { name: "invalid allow", - form: searchForm{ + form: searchParams{ Allow: "NotAKind, also not a kind", }, wantErr: true, }, { name: "invalid deny", - form: searchForm{ + form: searchParams{ Deny: "NotAKind, also not a kind", }, wantErr: true, @@ -192,16 +195,16 @@ func TestToOptions(t *testing.T) { t.Run(tc.name, func(t *testing.T) { got, err := tc.form.toOptions() if (err != nil) != tc.wantErr { - t.Fatalf("searchForm.toOptions() error = %v, wantErr %v", err, tc.wantErr) + t.Fatalf("Params.toOptions() error = %v, wantErr %v", err, tc.wantErr) } if !reflect.DeepEqual(got, tc.want) { - t.Errorf("searchForm.toOptions() = %v, want %v", got, tc.want) + t.Errorf("Params.toOptions() = %v, want %v", got, tc.want) } }) } } -func TestPopulatePage(t *testing.T) { +func TestPopulateSearchPage(t *testing.T) { g := newTestGaby(t) // Add test data relevant for this test. @@ -211,13 +214,13 @@ func TestPopulatePage(t *testing.T) { for _, tc := range []struct { name string url string - want searchPage + want *searchPage }{ { name: "query", url: "test/search?q=hello", - want: searchPage{ - searchForm: searchForm{ + want: &searchPage{ + Params: searchParams{ Query: "hello", }, Results: []search.Result{ @@ -234,8 +237,8 @@ func TestPopulatePage(t *testing.T) { { name: "id lookup", url: "test/search?q=id1", - want: searchPage{ - searchForm: searchForm{ + want: &searchPage{ + Params: searchParams{ Query: "id1", }, Results: []search.Result{{ @@ -250,8 +253,8 @@ func TestPopulatePage(t *testing.T) { { name: "options", url: "test/search?q=id1&threshold=.5&limit=10&allow_kind=&deny_kind=Unknown,GoBlog", - want: searchPage{ - searchForm: searchForm{ + want: &searchPage{ + Params: searchParams{ Query: "id1", Threshold: ".5", Limit: "10", @@ -264,12 +267,12 @@ func TestPopulatePage(t *testing.T) { { name: "error", url: "test/search?q=id1&deny_kind=Invalid", - want: searchPage{ - searchForm: searchForm{ + want: &searchPage{ + Params: searchParams{ Query: "id1", Deny: "Invalid", }, - SearchError: `invalid form value: unrecognized deny kind "Invalid" (case-sensitive)`, + Error: cmpopts.AnyError, }, }, } { @@ -278,9 +281,10 @@ func TestPopulatePage(t *testing.T) { if err != nil { t.Fatal(err) } - got := g.populatePage(r) - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("Gaby.search() = %v, want %v", got, tc.want) + got := g.populateSearchPage(r) + tc.want.setCommonPage() + if diff := cmp.Diff(tc.want, got, safeHTMLcmpopt, cmpopts.EquateErrors()); diff != "" { + t.Errorf("Gaby.populateSearchPage mismatch (-want +got):\n%s", diff) } }) } diff --git a/internal/gaby/static/search.css b/internal/gaby/static/search.css index 774ca55..e406a8e 100644 --- a/internal/gaby/static/search.css +++ b/internal/gaby/static/search.css @@ -37,20 +37,6 @@ div.result span { div.result { padding-bottom: 1em; } -.filter-tips-box { - font-size: .75em; - padding-bottom: .5em; -} -.toggle { - font-weight: bold; - color: #007d9c; -} -.toggle:hover { - text-decoration: underline; -} -#filter-tips { - display: none; -} .submit { padding-top: .5em; } \ No newline at end of file diff --git a/internal/gaby/static/style.css b/internal/gaby/static/style.css index 30ad625..df58e22 100644 --- a/internal/gaby/static/style.css +++ b/internal/gaby/static/style.css @@ -32,9 +32,31 @@ h1 { width: 40%; padding-bottom: .5em; } +.emph { + font-weight: bold; +} /* Wrap long text inside
. */ pre.wrap { white-space: pre-wrap; } +.filter-tips-box { + font-size: .75em; + padding-bottom: .5em; +} +.toggle { + font-weight: bold; + color: #007d9c; +} +.toggle:hover { + text-decoration: underline; +} +#filter-tips { + display: none; +} + +nav a#current-nav { + font-weight: bold; +} + diff --git a/internal/gaby/templates.go b/internal/gaby/templates.go index fafe41f..e2052b4 100644 --- a/internal/gaby/templates.go +++ b/internal/gaby/templates.go @@ -34,6 +34,16 @@ const ( ) func newTemplate(filename string, funcs template.FuncMap) *template.Template { + if funcs == nil { + funcs = make(template.FuncMap) + } + // Add common functions. + funcs["pages"] = func() []pageID { + return pages + } + funcs["dec"] = func(i int) int { + return i - 1 + } return template.Must(template.New(filename).Funcs(funcs). ParseFS(template.TrustedFSFromEmbed(tmplFS), path.Join("tmpl", filename), diff --git a/internal/gaby/templates_test.go b/internal/gaby/templates_test.go index 3500ed9..cce0037 100644 --- a/internal/gaby/templates_test.go +++ b/internal/gaby/templates_test.go @@ -26,16 +26,16 @@ func TestTemplates(t *testing.T) { for _, test := range []struct { name string tmpl *template.Template - value any + value testPage }{ - {"search", searchPageTmpl, searchPage{Results: []search.Result{{Kind: "k", Title: "t"}}}}, - {"actionlog", actionLogPageTmpl, actionLogPage{ + {"search", searchPageTmpl, &searchPage{Results: []search.Result{{Kind: "k", Title: "t"}}}}, + {"actionlog", actionLogPageTmpl, &actionLogPage{ StartTime: "t", Entries: []*actions.Entry{{Kind: "k"}}, }}, - {"overview-initial", overviewPageTmpl, overviewPage{}}, - {"overview", overviewPageTmpl, overviewPage{ - Form: overviewForm{Query: "12"}, + {"overview-initial", overviewPageTmpl, &overviewPage{}}, + {"overview", overviewPageTmpl, &overviewPage{ + Params: overviewParams{Query: "12"}, Result: &overviewResult{ IssueOverviewResult: github.IssueOverviewResult{ Issue: &github.Issue{ @@ -52,17 +52,18 @@ func TestTemplates(t *testing.T) { }, Type: issueOverviewType, }}}, - {"overview-error", overviewPageTmpl, overviewPage{ - Form: overviewForm{Query: "12"}, - Error: fmt.Errorf("an error"), + {"overview-error", overviewPageTmpl, &overviewPage{ + Params: overviewParams{Query: "12"}, + Error: fmt.Errorf("an error"), }}, } { t.Run(test.name, func(t *testing.T) { - var buf bytes.Buffer - if err := test.tmpl.Execute(&buf, test.value); err != nil { + test.value.setCommonPage() + b, err := Exec(test.tmpl, test.value) + if err != nil { t.Fatal(err) } - html := buf.String() + html := string(b) if err := validateHTML(html); err != nil { printNumbered(html) t.Fatalf("\n%s", err) @@ -71,6 +72,11 @@ func TestTemplates(t *testing.T) { } } +type testPage interface { + setCommonPage() + page +} + func printNumbered(s string) { for i, line := range strings.Split(s, "\n") { fmt.Printf("%3d %s\n", i+1, line) diff --git a/internal/gaby/tmpl/actionlog.tmpl b/internal/gaby/tmpl/actionlog.tmpl index a9976c1..5a8e173 100644 --- a/internal/gaby/tmpl/actionlog.tmpl +++ b/internal/gaby/tmpl/actionlog.tmpl @@ -5,132 +5,14 @@ license that can be found in the LICENSE file. --> - - {{template "head" .}} - - + {{template "head" .}}+{{end}} + +{{define "actionlog-result"}} + + +{{if .StartTime}} +- {{template "header" .}} - -- -- {{if .StartTime}} -+ + {{template "actionlog-result" .}} @@ -140,3 +22,129 @@ license that can be found in the LICENSE file. {{end}} + +{{define "actionlog-form"}} +Action Log from {{.StartTime}} to {{.EndTime}}
- - {{with .Entries}} -- -
- {{else}} - No entries. - {{end}} - {{end}} + {{template "nav-title" .}} + {{template "actionlog-form" .}}- - - {{range $i, $e := .}} -Created -Kind -Key -Action -Done -Result -Error -- -{{$e.Created | fmttime}} -{{$e.Kind}} -{{$e.Key | fmtkey}} -- {{- /* clicking the button shows/hides the action on the following row */ -}} - - -{{$e.Done | fmttime}} -- {{$e.Result | fmtval}}{{$e.Error}} -- - {{end}} -- -{{$e.ActionForDisplay}}-
Created | +Kind | +Key | +Action | +Done | +Result | +Error | +
---|---|---|---|---|---|---|
{{$e.Created | fmttime}} | +{{$e.Kind}} | +{{$e.Key | fmtkey}} | ++ {{- /* clicking the button shows/hides the action on the following row */ -}} + + | +{{$e.Done | fmttime}} | +{{$e.Result | fmtval}} |
+ {{$e.Error}} | +
+ {{$e.ActionForDisplay}}+ |
+
{{.Description}}
{{end}} {{define "nav"}} - + {{$current := .ID}} + {{with pages}} + {{$last := dec (len .)}} + + {{end}} +{{end}} + +{{define "toggle-script"}} + +{{end}} + +{{define "filter-tips"}} +{{.Type}}
): {{.Description}}
+ No results.
{{end}} - {{- end}} + {{if .Params.Start}}No results.
{{end}} + {{- end}}