feat: optional findings telemetry — emit runs+findings to a gadfly-reports store
Build & push image / build-and-push (push) Successful in 8s
Build & push image / build-and-push (push) Successful in 8s
After each review the binary POSTs the run + its heuristically-extracted findings to GADFLY_FINDINGS_URL (off unless set). Advisory: any error only goes to stderr — never touches stdout, the exit code, or the review. stdlib net/http only (no new deps). entrypoint.sh derives GADFLY_REPO/GADFLY_PR and passes through GADFLY_FINDINGS_URL/GADFLY_FINDINGS_TOKEN. Also renames store references from the old 'docket' name to 'gadfly-reports'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -254,6 +254,26 @@ The reviewer binary reads these (the stub/entrypoint set sane defaults):
|
||||
| `GADFLY_MAX_DIFF_CHARS` | 60000 | diff chars embedded in the prompt (full diff via `get_diff`) |
|
||||
| `GADFLY_TRIGGER_PHRASE` | `@gadfly review` | comment phrase that re-triggers |
|
||||
| `GADFLY_ALLOWED_USERS` | *(collaborators)* | comma-separated allow-list for comment triggers |
|
||||
| `GADFLY_FINDINGS_URL` | — | gadfly-reports store base URL; set to enable findings telemetry (off when empty) |
|
||||
| `GADFLY_FINDINGS_TOKEN` | — | bearer token for the gadfly-reports store (sent as `Authorization: Bearer …`) |
|
||||
| `GADFLY_REPO` | *(from `GITEA_API`)* | `owner/repo` slug stamped on emitted runs/findings (set by `entrypoint.sh`) |
|
||||
| `GADFLY_PR` | *(from event)* | PR number stamped on emitted runs/findings (set by `entrypoint.sh`) |
|
||||
|
||||
## Findings telemetry (optional)
|
||||
|
||||
Gadfly can record what it found so model quality can be tracked over time. It is
|
||||
**off by default** and purely advisory: set **`GADFLY_FINDINGS_URL`** to a
|
||||
[gadfly-reports](https://gitea.stevedudenhoeffer.com/steve/gadfly-reports) store base URL and,
|
||||
after each review, the binary best-effort `POST`s the run (`/runs`) and the
|
||||
findings it surfaced (`/reports`) to that store. Add **`GADFLY_FINDINGS_TOKEN`**
|
||||
to send an `Authorization: Bearer …` header. `entrypoint.sh` supplies the run
|
||||
context (`GADFLY_REPO`, `GADFLY_PR`) automatically.
|
||||
|
||||
Findings are extracted heuristically from each lens's markdown — a `path:line`
|
||||
reference anchors a finding, titled by the nearest preceding heading / numbered
|
||||
item / bold lead-in. The emit is strictly best-effort: a short (~10s) timeout,
|
||||
any error (or a non-2xx response) is logged to stderr only, and it **never**
|
||||
changes the review output or the exit code.
|
||||
|
||||
## Building locally
|
||||
|
||||
|
||||
@@ -0,0 +1,386 @@
|
||||
package main
|
||||
|
||||
// Optional findings telemetry. After a review, Gadfly can POST the run and the
|
||||
// findings it surfaced to a "gadfly-reports" store (gitea.stevedudenhoeffer.com-style
|
||||
// HTTP API) so model quality can be tracked over time and graded later.
|
||||
//
|
||||
// This is strictly ADVISORY plumbing and must never affect the review itself:
|
||||
// - It is OFF unless GADFLY_FINDINGS_URL is set.
|
||||
// - It writes ONLY to stderr on any error; it never touches stdout (stdout is
|
||||
// the review markdown consumed by run.sh) and never changes the exit code.
|
||||
// - It depends only on the Go stdlib (net/http).
|
||||
//
|
||||
// Findings are extracted heuristically from each lens's markdown: a "path:line"
|
||||
// reference (e.g. run/executor.go:166) anchors a finding, whose title is the
|
||||
// nearest preceding markdown heading / numbered item / bold lead-in (else the
|
||||
// first sentence of the finding's paragraph). This is best-effort signal for the
|
||||
// store to aggregate — not a structured contract the reviewer guarantees.
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"os"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
const (
|
||||
// findingsHTTPTimeout bounds each telemetry POST. Short on purpose — a slow
|
||||
// or dead store must never delay the review's stdout (already printed) or the
|
||||
// process exit.
|
||||
findingsHTTPTimeout = 10 * time.Second
|
||||
// maxFindingsPerLens caps how many findings a single lens contributes, so a
|
||||
// pathological lens output can't blow up the /reports payload.
|
||||
maxFindingsPerLens = 20
|
||||
)
|
||||
|
||||
// pathLineRe matches a source-path-with-extension followed by ":<line>", e.g.
|
||||
// run/executor.go:166 or compact/compactor.go:225. The path may contain
|
||||
// directories, dots, dashes and underscores; backtracking lets the final ".ext"
|
||||
// be split off the greedy path run. Surrounding backticks/parens are ignored
|
||||
// because they are not in the character class.
|
||||
var pathLineRe = regexp.MustCompile(`([A-Za-z0-9_./-]+\.[A-Za-z0-9]+):(\d+)`)
|
||||
|
||||
// numberedRe matches a numbered list/heading lead-in: "1. Title" or "1) Title".
|
||||
var numberedRe = regexp.MustCompile(`^\d+[.)]\s+(.+)$`)
|
||||
|
||||
// finding is one extracted issue: where it points and a derived human title.
|
||||
type finding struct {
|
||||
file string
|
||||
line int
|
||||
title string
|
||||
detail string
|
||||
}
|
||||
|
||||
// runPayload is the POST /runs body. Field names match the gadfly-reports API EXACTLY.
|
||||
// Token/cost fields are pointers so they marshal to JSON null when unknown
|
||||
// (Gadfly does not currently meter tokens or cost).
|
||||
type runPayload struct {
|
||||
RunID string `json:"run_id"`
|
||||
Repo string `json:"repo"`
|
||||
PR int `json:"pr"`
|
||||
Model string `json:"model"`
|
||||
Provider string `json:"provider"`
|
||||
Lenses int `json:"lenses"`
|
||||
DurationSecs float64 `json:"duration_secs"`
|
||||
InputTokens *int `json:"input_tokens"`
|
||||
OutputTokens *int `json:"output_tokens"`
|
||||
CostUSD *float64 `json:"cost_usd"`
|
||||
}
|
||||
|
||||
// reportPayload is one element of the POST /reports JSON array. Field names match
|
||||
// the gadfly-reports API EXACTLY.
|
||||
type reportPayload struct {
|
||||
Repo string `json:"repo"`
|
||||
PR int `json:"pr"`
|
||||
Lens string `json:"lens"`
|
||||
File string `json:"file"`
|
||||
Line int `json:"line"`
|
||||
Title string `json:"title"`
|
||||
Model string `json:"model"`
|
||||
Provider string `json:"provider"`
|
||||
RunID string `json:"run_id"`
|
||||
RawSeverity string `json:"raw_severity"`
|
||||
Detail string `json:"detail"`
|
||||
}
|
||||
|
||||
// emit posts the run and its findings to the gadfly-reports store. It is a no-op unless
|
||||
// GADFLY_FINDINGS_URL is set. Best-effort: any error is logged to stderr and
|
||||
// otherwise ignored. It NEVER returns an error, never calls os.Exit, and never
|
||||
// writes to stdout.
|
||||
func emit(results []specialistResult, elapsed time.Duration) {
|
||||
base := strings.TrimRight(strings.TrimSpace(os.Getenv("GADFLY_FINDINGS_URL")), "/")
|
||||
if base == "" {
|
||||
return // telemetry disabled
|
||||
}
|
||||
|
||||
repo := strings.TrimSpace(os.Getenv("GADFLY_REPO"))
|
||||
pr := envAtoi("GADFLY_PR")
|
||||
model := strings.TrimSpace(os.Getenv("GADFLY_MODEL"))
|
||||
provider := modelProvider()
|
||||
runID := fmt.Sprintf("%s#%d:%s", repo, pr, model)
|
||||
|
||||
run := runPayload{
|
||||
RunID: runID,
|
||||
Repo: repo,
|
||||
PR: pr,
|
||||
Model: model,
|
||||
Provider: provider,
|
||||
Lenses: len(results),
|
||||
DurationSecs: elapsed.Seconds(),
|
||||
// InputTokens/OutputTokens/CostUSD stay nil -> JSON null (not metered).
|
||||
}
|
||||
|
||||
var reports []reportPayload
|
||||
for _, r := range results {
|
||||
if r.errored {
|
||||
continue // a failed lens contributes no findings
|
||||
}
|
||||
sev := r.verdict.label()
|
||||
for _, f := range parseFindings(r.spec, r.out) {
|
||||
reports = append(reports, reportPayload{
|
||||
Repo: repo,
|
||||
PR: pr,
|
||||
Lens: r.spec.Name,
|
||||
File: f.file,
|
||||
Line: f.line,
|
||||
Title: f.title,
|
||||
Model: model,
|
||||
Provider: provider,
|
||||
RunID: runID,
|
||||
RawSeverity: sev,
|
||||
Detail: f.detail,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
client := &http.Client{Timeout: findingsHTTPTimeout}
|
||||
token := strings.TrimSpace(os.Getenv("GADFLY_FINDINGS_TOKEN"))
|
||||
|
||||
if err := postJSON(client, base+"/runs", token, run); err != nil {
|
||||
fmt.Fprintln(os.Stderr, "gadfly: findings emit (/runs) failed:", err)
|
||||
}
|
||||
if len(reports) > 0 {
|
||||
if err := postJSON(client, base+"/reports", token, reports); err != nil {
|
||||
fmt.Fprintln(os.Stderr, "gadfly: findings emit (/reports) failed:", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// parseFindings extracts findings from one lens's markdown output. A lens whose
|
||||
// result errored (its output is the inline failure notice) yields nothing.
|
||||
// Findings are anchored on "path:line" references, deduped by (file,line) keeping
|
||||
// the first occurrence, and capped at maxFindingsPerLens.
|
||||
func parseFindings(sp Specialist, out string) []finding {
|
||||
_ = sp // signature kept symmetric with the caller; lens identity isn't needed here
|
||||
trimmed := strings.TrimSpace(out)
|
||||
if trimmed == "" || strings.HasPrefix(trimmed, "⚠️") {
|
||||
return nil // empty or a "reviewer failed" notice
|
||||
}
|
||||
|
||||
lines := strings.Split(out, "\n")
|
||||
var findings []finding
|
||||
seen := map[string]bool{}
|
||||
|
||||
for _, loc := range pathLineRe.FindAllStringSubmatchIndex(out, -1) {
|
||||
file := out[loc[2]:loc[3]]
|
||||
lineStr := out[loc[4]:loc[5]]
|
||||
ln, err := strconv.Atoi(lineStr)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
key := file + ":" + lineStr
|
||||
if seen[key] {
|
||||
continue
|
||||
}
|
||||
seen[key] = true
|
||||
|
||||
li := strings.Count(out[:loc[0]], "\n") // line index of the match
|
||||
findings = append(findings, finding{
|
||||
file: file,
|
||||
line: ln,
|
||||
title: deriveTitle(lines, li),
|
||||
detail: paragraphAt(lines, li),
|
||||
})
|
||||
if len(findings) >= maxFindingsPerLens {
|
||||
break
|
||||
}
|
||||
}
|
||||
return findings
|
||||
}
|
||||
|
||||
// deriveTitle finds the nearest preceding markdown heading / numbered item /
|
||||
// bold lead-in at or above line li; failing that, the first sentence of the
|
||||
// paragraph containing li.
|
||||
func deriveTitle(lines []string, li int) string {
|
||||
for i := li; i >= 0 && i < len(lines); i-- {
|
||||
if t, ok := headingTitle(lines[i]); ok {
|
||||
return t
|
||||
}
|
||||
}
|
||||
return paragraphTitle(lines, li)
|
||||
}
|
||||
|
||||
// headingTitle returns the title carried by a markdown heading-like line:
|
||||
// an ATX "#"/"##"/"###" heading, a numbered "1." lead-in, or a "**bold**"
|
||||
// lead-in (optionally behind a list bullet). ok is false for ordinary lines.
|
||||
func headingTitle(line string) (string, bool) {
|
||||
t := strings.TrimSpace(line)
|
||||
if t == "" {
|
||||
return "", false
|
||||
}
|
||||
if strings.HasPrefix(t, "#") {
|
||||
h := strings.TrimSpace(strings.TrimLeft(t, "#"))
|
||||
if h != "" {
|
||||
return cleanHeading(h), true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
if m := numberedRe.FindStringSubmatch(t); m != nil {
|
||||
if rest := strings.TrimSpace(m[1]); rest != "" {
|
||||
return cleanHeading(rest), true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
// Bold lead-in, possibly behind a "- "/"* "/"+ " bullet. Strip only the
|
||||
// bullet marker (NOT the bold asterisks) before testing for "**".
|
||||
b := t
|
||||
for _, pfx := range []string{"- ", "* ", "+ "} {
|
||||
if strings.HasPrefix(b, pfx) {
|
||||
b = strings.TrimSpace(b[len(pfx):])
|
||||
break
|
||||
}
|
||||
}
|
||||
if strings.HasPrefix(b, "**") {
|
||||
if title := firstBold(b); title != "" {
|
||||
return title, true
|
||||
}
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
// cleanHeading normalizes an extracted heading into a concise title: prefer a
|
||||
// leading "**bold**" span, otherwise trim a trailing colon and truncate.
|
||||
func cleanHeading(s string) string {
|
||||
s = strings.TrimSpace(s)
|
||||
if strings.HasPrefix(s, "**") {
|
||||
if b := firstBold(s); b != "" {
|
||||
return b
|
||||
}
|
||||
}
|
||||
s = strings.TrimRight(s, ": ")
|
||||
return truncate(s, 120)
|
||||
}
|
||||
|
||||
// paragraphTitle falls back to the first sentence/line of the paragraph (the
|
||||
// contiguous block of non-blank lines) containing line li, stripped of markdown
|
||||
// list/emphasis markers.
|
||||
func paragraphTitle(lines []string, li int) string {
|
||||
if li < 0 || li >= len(lines) {
|
||||
return ""
|
||||
}
|
||||
start := li
|
||||
for start > 0 && strings.TrimSpace(lines[start-1]) != "" {
|
||||
start--
|
||||
}
|
||||
first := leadingSentence(stripMarkers(lines[start]))
|
||||
return truncate(strings.TrimSpace(first), 120)
|
||||
}
|
||||
|
||||
// leadingSentence returns text up to the first sentence terminator that is
|
||||
// followed by a space (". " / "! " / "? "), so a dot inside a filename like
|
||||
// "util.go" does not prematurely cut the sentence. With no internal terminator
|
||||
// it returns the line minus any trailing terminal punctuation. (auto.go's
|
||||
// firstSentence splits on any ".", which would mis-cut a "path:line" sentence.)
|
||||
func leadingSentence(s string) string {
|
||||
best := -1
|
||||
for _, sep := range []string{". ", "! ", "? "} {
|
||||
if i := strings.Index(s, sep); i >= 0 && (best == -1 || i < best) {
|
||||
best = i
|
||||
}
|
||||
}
|
||||
if best >= 0 {
|
||||
return s[:best]
|
||||
}
|
||||
return strings.TrimRight(s, ".!?")
|
||||
}
|
||||
|
||||
// paragraphAt returns the paragraph (contiguous non-blank lines) containing li,
|
||||
// joined onto one line and truncated — used as the finding's detail context.
|
||||
func paragraphAt(lines []string, li int) string {
|
||||
if li < 0 || li >= len(lines) {
|
||||
return ""
|
||||
}
|
||||
start, end := li, li
|
||||
for start > 0 && strings.TrimSpace(lines[start-1]) != "" {
|
||||
start--
|
||||
}
|
||||
for end < len(lines)-1 && strings.TrimSpace(lines[end+1]) != "" {
|
||||
end++
|
||||
}
|
||||
var parts []string
|
||||
for i := start; i <= end; i++ {
|
||||
if t := strings.TrimSpace(lines[i]); t != "" {
|
||||
parts = append(parts, t)
|
||||
}
|
||||
}
|
||||
return truncate(strings.Join(parts, " "), 500)
|
||||
}
|
||||
|
||||
// stripMarkers removes a leading list bullet / numbered marker and surrounding
|
||||
// emphasis/backticks from a line, leaving plain text.
|
||||
func stripMarkers(s string) string {
|
||||
s = strings.TrimSpace(s)
|
||||
for _, pfx := range []string{"- ", "* ", "+ "} {
|
||||
if strings.HasPrefix(s, pfx) {
|
||||
s = strings.TrimSpace(s[len(pfx):])
|
||||
break
|
||||
}
|
||||
}
|
||||
if m := numberedRe.FindStringSubmatch(s); m != nil {
|
||||
s = strings.TrimSpace(m[1])
|
||||
}
|
||||
return strings.Trim(s, "*`# ")
|
||||
}
|
||||
|
||||
// firstBold returns the text inside the first "**...**" span of s, or "".
|
||||
func firstBold(s string) string {
|
||||
i := strings.Index(s, "**")
|
||||
if i < 0 {
|
||||
return ""
|
||||
}
|
||||
rest := s[i+2:]
|
||||
j := strings.Index(rest, "**")
|
||||
if j < 0 {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimSpace(rest[:j])
|
||||
}
|
||||
|
||||
// truncate shortens s to at most n runes, appending an ellipsis when cut.
|
||||
func truncate(s string, n int) string {
|
||||
r := []rune(s)
|
||||
if len(r) <= n {
|
||||
return s
|
||||
}
|
||||
return strings.TrimSpace(string(r[:n])) + "…"
|
||||
}
|
||||
|
||||
// postJSON marshals payload and POSTs it as application/json, attaching a Bearer
|
||||
// token when non-empty. A non-2xx response is an error. The body is drained and
|
||||
// closed so the connection can be reused.
|
||||
func postJSON(client *http.Client, url, token string, payload any) error {
|
||||
body, err := json.Marshal(payload)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
if token != "" {
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
}
|
||||
resp, err := client.Do(req)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||
return fmt.Errorf("%s: status %d", url, resp.StatusCode)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// envAtoi reads an integer env var, returning 0 when unset or unparseable.
|
||||
func envAtoi(name string) int {
|
||||
n, _ := strconv.Atoi(strings.TrimSpace(os.Getenv(name)))
|
||||
return n
|
||||
}
|
||||
@@ -0,0 +1,214 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// sampleLensMarkdown is a realistic lens output: a bold verdict lead-in, an ATX
|
||||
// "###" heading anchoring one finding, a numbered + bold lead-in anchoring
|
||||
// another, plus a duplicate "path:line" reference that must be deduped away.
|
||||
const sampleLensMarkdown = "" +
|
||||
"**Blocking issues found**\n" +
|
||||
"\n" +
|
||||
"### Swallowed error\n" +
|
||||
"In `run/executor.go:166` the returned error is ignored, so a failed step is\n" +
|
||||
"reported as success.\n" +
|
||||
"\n" +
|
||||
"1. **Missing nil check** — `compact/compactor.go:225` dereferences `cfg`\n" +
|
||||
" which can be nil when the caller passes an empty config.\n" +
|
||||
"\n" +
|
||||
"See again run/executor.go:166 for the same root cause.\n"
|
||||
|
||||
func TestParseFindings(t *testing.T) {
|
||||
got := parseFindings(Specialist{Name: "security"}, sampleLensMarkdown)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("parseFindings returned %d findings, want 2 (deduped): %+v", len(got), got)
|
||||
}
|
||||
|
||||
if got[0].file != "run/executor.go" || got[0].line != 166 {
|
||||
t.Errorf("finding[0] location = %s:%d, want run/executor.go:166", got[0].file, got[0].line)
|
||||
}
|
||||
if got[0].title != "Swallowed error" {
|
||||
t.Errorf("finding[0] title = %q, want %q (nearest ### heading)", got[0].title, "Swallowed error")
|
||||
}
|
||||
|
||||
if got[1].file != "compact/compactor.go" || got[1].line != 225 {
|
||||
t.Errorf("finding[1] location = %s:%d, want compact/compactor.go:225", got[1].file, got[1].line)
|
||||
}
|
||||
if got[1].title != "Missing nil check" {
|
||||
t.Errorf("finding[1] title = %q, want %q (numbered bold lead-in)", got[1].title, "Missing nil check")
|
||||
}
|
||||
if got[1].detail == "" {
|
||||
t.Error("finding[1] detail should carry the paragraph context, got empty")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseFindings_ErroredLensYieldsNone(t *testing.T) {
|
||||
notice := "⚠️ This reviewer failed to complete: context deadline exceeded (file.go:10)"
|
||||
if f := parseFindings(Specialist{Name: "security"}, notice); len(f) != 0 {
|
||||
t.Errorf("a failure-notice lens should yield no findings, got %+v", f)
|
||||
}
|
||||
if f := parseFindings(Specialist{Name: "security"}, " "); len(f) != 0 {
|
||||
t.Errorf("empty output should yield no findings, got %+v", f)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseFindings_FallbackParagraphTitle(t *testing.T) {
|
||||
md := "No material issues found.\n\nThe helper in src/util.go:42 returns the wrong code. Investigate.\n"
|
||||
got := parseFindings(Specialist{Name: "correctness"}, md)
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("want 1 finding, got %d: %+v", len(got), got)
|
||||
}
|
||||
if got[0].title != "The helper in src/util.go:42 returns the wrong code" {
|
||||
t.Errorf("fallback title = %q, want first sentence of the paragraph", got[0].title)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_PostsRunsAndReports(t *testing.T) {
|
||||
var runs, reports int32
|
||||
var runBody map[string]any
|
||||
var reportBody []map[string]any
|
||||
var sawAuth string
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if a := r.Header.Get("Authorization"); a != "" {
|
||||
sawAuth = a
|
||||
}
|
||||
data, _ := io.ReadAll(r.Body)
|
||||
switch r.URL.Path {
|
||||
case "/runs":
|
||||
atomic.AddInt32(&runs, 1)
|
||||
_ = json.Unmarshal(data, &runBody)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(`{"run_id":"x"}`))
|
||||
case "/reports":
|
||||
atomic.AddInt32(&reports, 1)
|
||||
_ = json.Unmarshal(data, &reportBody)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(`{"finding_ids":[1,2]}`))
|
||||
default:
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
t.Setenv("GADFLY_FINDINGS_URL", srv.URL)
|
||||
t.Setenv("GADFLY_FINDINGS_TOKEN", "secret-token")
|
||||
t.Setenv("GADFLY_REPO", "owner/repo")
|
||||
t.Setenv("GADFLY_PR", "7")
|
||||
t.Setenv("GADFLY_MODEL", "ollama-cloud/qwen3")
|
||||
t.Setenv("GADFLY_PROVIDER", "")
|
||||
|
||||
results := []specialistResult{
|
||||
{spec: Specialist{Name: "security", Title: "🔒 Security"}, out: sampleLensMarkdown, verdict: verdictBlocking},
|
||||
}
|
||||
|
||||
emit(results, 3*time.Second)
|
||||
|
||||
if got := atomic.LoadInt32(&runs); got != 1 {
|
||||
t.Fatalf("/runs received %d times, want exactly 1", got)
|
||||
}
|
||||
if got := atomic.LoadInt32(&reports); got != 1 {
|
||||
t.Fatalf("/reports received %d times, want exactly 1", got)
|
||||
}
|
||||
|
||||
if sawAuth != "Bearer secret-token" {
|
||||
t.Errorf("Authorization header = %q, want %q", sawAuth, "Bearer secret-token")
|
||||
}
|
||||
|
||||
// --- /runs body: exact field names + values ---
|
||||
for _, k := range []string{"run_id", "repo", "pr", "model", "provider", "lenses", "duration_secs", "input_tokens", "output_tokens", "cost_usd"} {
|
||||
if _, ok := runBody[k]; !ok {
|
||||
t.Errorf("/runs body missing field %q (got keys %v)", k, keysOf(runBody))
|
||||
}
|
||||
}
|
||||
if runBody["run_id"] != "owner/repo#7:ollama-cloud/qwen3" {
|
||||
t.Errorf("run_id = %v, want owner/repo#7:ollama-cloud/qwen3", runBody["run_id"])
|
||||
}
|
||||
if runBody["repo"] != "owner/repo" {
|
||||
t.Errorf("repo = %v, want owner/repo", runBody["repo"])
|
||||
}
|
||||
if runBody["pr"] != float64(7) {
|
||||
t.Errorf("pr = %v, want 7", runBody["pr"])
|
||||
}
|
||||
if runBody["model"] != "ollama-cloud/qwen3" {
|
||||
t.Errorf("model = %v, want ollama-cloud/qwen3", runBody["model"])
|
||||
}
|
||||
if runBody["provider"] != "ollama-cloud" {
|
||||
t.Errorf("provider = %v, want ollama-cloud", runBody["provider"])
|
||||
}
|
||||
if runBody["lenses"] != float64(1) {
|
||||
t.Errorf("lenses = %v, want 1", runBody["lenses"])
|
||||
}
|
||||
if runBody["duration_secs"] != float64(3) {
|
||||
t.Errorf("duration_secs = %v, want 3", runBody["duration_secs"])
|
||||
}
|
||||
if v, ok := runBody["input_tokens"]; !ok || v != nil {
|
||||
t.Errorf("input_tokens = %v, want JSON null", v)
|
||||
}
|
||||
|
||||
// --- /reports body: exact field names + values ---
|
||||
if len(reportBody) != 2 {
|
||||
t.Fatalf("/reports array length = %d, want 2", len(reportBody))
|
||||
}
|
||||
for _, k := range []string{"repo", "pr", "lens", "file", "line", "title", "model", "provider", "run_id", "raw_severity", "detail"} {
|
||||
if _, ok := reportBody[0][k]; !ok {
|
||||
t.Errorf("/reports[0] missing field %q (got keys %v)", k, keysOf(reportBody[0]))
|
||||
}
|
||||
}
|
||||
if reportBody[0]["lens"] != "security" {
|
||||
t.Errorf("reports[0].lens = %v, want security", reportBody[0]["lens"])
|
||||
}
|
||||
if reportBody[0]["file"] != "run/executor.go" {
|
||||
t.Errorf("reports[0].file = %v, want run/executor.go", reportBody[0]["file"])
|
||||
}
|
||||
if reportBody[0]["line"] != float64(166) {
|
||||
t.Errorf("reports[0].line = %v, want 166", reportBody[0]["line"])
|
||||
}
|
||||
if reportBody[0]["raw_severity"] != "Blocking issues found" {
|
||||
t.Errorf("reports[0].raw_severity = %v, want 'Blocking issues found'", reportBody[0]["raw_severity"])
|
||||
}
|
||||
if reportBody[0]["run_id"] != "owner/repo#7:ollama-cloud/qwen3" {
|
||||
t.Errorf("reports[0].run_id = %v, want owner/repo#7:ollama-cloud/qwen3", reportBody[0]["run_id"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_DisabledMakesNoRequests(t *testing.T) {
|
||||
var hits int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&hits, 1)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
t.Setenv("GADFLY_FINDINGS_URL", "") // disabled
|
||||
t.Setenv("GADFLY_REPO", "owner/repo")
|
||||
t.Setenv("GADFLY_PR", "7")
|
||||
t.Setenv("GADFLY_MODEL", "ollama-cloud/qwen3")
|
||||
|
||||
results := []specialistResult{
|
||||
{spec: Specialist{Name: "security"}, out: sampleLensMarkdown, verdict: verdictBlocking},
|
||||
}
|
||||
|
||||
// Must not panic and must not hit the server.
|
||||
emit(results, time.Second)
|
||||
emit(nil, 0)
|
||||
|
||||
if got := atomic.LoadInt32(&hits); got != 0 {
|
||||
t.Fatalf("telemetry disabled but server received %d requests, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
func keysOf(m map[string]any) []string {
|
||||
ks := make([]string, 0, len(m))
|
||||
for k := range m {
|
||||
ks = append(ks, k)
|
||||
}
|
||||
return ks
|
||||
}
|
||||
@@ -122,6 +122,7 @@ func main() {
|
||||
}
|
||||
|
||||
func run() error {
|
||||
start := time.Now()
|
||||
repoDir := os.Getenv("GADFLY_REPO_DIR")
|
||||
diffFile := os.Getenv("GADFLY_DIFF_FILE")
|
||||
systemFile := os.Getenv("GADFLY_SYSTEM_FILE")
|
||||
@@ -193,6 +194,10 @@ func run() error {
|
||||
results := runSpecialists(mdl, fsTools, base, specialists, task, diff)
|
||||
|
||||
fmt.Println(renderConsolidated(results))
|
||||
|
||||
// Optional, best-effort telemetry. OFF unless GADFLY_FINDINGS_URL is set;
|
||||
// any failure is logged to stderr and never affects stdout or the exit code.
|
||||
emit(results, time.Since(start))
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -39,6 +39,9 @@
|
||||
# GADFLY_TRIGGER_PHRASE comment phrase that triggers a re-review (default "@gadfly review")
|
||||
# GADFLY_ALLOWED_USERS comma-separated usernames allowed to comment-trigger;
|
||||
# empty => fall back to "is a repo collaborator"
|
||||
# GADFLY_FINDINGS_URL optional gadfly-reports store base URL; set to POST the run +
|
||||
# findings for model-quality tracking (off when empty)
|
||||
# GADFLY_FINDINGS_TOKEN optional bearer token for the gadfly-reports store
|
||||
set -uo pipefail
|
||||
|
||||
# One model by default: the specialist suite already provides breadth, so a
|
||||
@@ -124,6 +127,17 @@ log "cloning ${REPO_PATH} @ ${BRANCH}"
|
||||
git clone --depth=1 --branch "$BRANCH" "$CLONE_URL" "$REPO_DIR" 2>/dev/null \
|
||||
|| die "clone of ${REPO_PATH}@${BRANCH} failed"
|
||||
|
||||
# --- findings telemetry (optional) -----------------------------------------
|
||||
# Plumb the run context to the binary (inherited through run.sh and the gadfly
|
||||
# process env). The binary's emit is OFF unless GADFLY_FINDINGS_URL is set; when
|
||||
# set it POSTs the run + its findings to a gadfly-reports store. GADFLY_FINDINGS_URL /
|
||||
# GADFLY_FINDINGS_TOKEN come from the consumer's stub env and are re-exported so
|
||||
# they reach the binary even if unset (empty => disabled).
|
||||
export GADFLY_REPO="$REPO_PATH"
|
||||
export GADFLY_PR="$PR"
|
||||
export GADFLY_FINDINGS_URL="${GADFLY_FINDINGS_URL:-}"
|
||||
export GADFLY_FINDINGS_TOKEN="${GADFLY_FINDINGS_TOKEN:-}"
|
||||
|
||||
# --- review once per model, with per-provider concurrency -------------------
|
||||
# GADFLY_MODELS is the provider-agnostic name; OLLAMA_REVIEW_MODELS is a
|
||||
# back-compat alias. GADFLY_PROVIDER / GADFLY_BASE_URL / GADFLY_API_KEY and any
|
||||
|
||||
@@ -91,6 +91,13 @@ jobs:
|
||||
GADFLY_PROVIDER: ${{ vars.GADFLY_PROVIDER }}
|
||||
GADFLY_BASE_URL: ${{ vars.GADFLY_BASE_URL }}
|
||||
GADFLY_MODELS: ${{ vars.GADFLY_MODELS }}
|
||||
# --- Findings telemetry (optional; OFF by default) ------------------
|
||||
# Set GADFLY_FINDINGS_URL to a gadfly-reports store base URL to POST each run +
|
||||
# its findings for model-quality tracking. Advisory only: failures are
|
||||
# logged to stderr and never affect the review. Add a bearer token if
|
||||
# the store requires auth. (GADFLY_REPO / GADFLY_PR are derived for you.)
|
||||
# GADFLY_FINDINGS_URL: ${{ vars.GADFLY_FINDINGS_URL }}
|
||||
# GADFLY_FINDINGS_TOKEN: ${{ secrets.GADFLY_FINDINGS_TOKEN }}
|
||||
EVENT_NAME: ${{ github.event_name }}
|
||||
PR: ${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}
|
||||
PR_BRANCH: ${{ github.head_ref }}
|
||||
|
||||
Reference in New Issue
Block a user