fix: clean-lens findings + trim the dogfood swarm to strong reviewers (#4)
Build & push image / build-and-push (push) Successful in 9s
Build & push image / build-and-push (push) Successful in 9s
emit() now skips findings extraction for a "No material issues found" lens (its path:line refs are verification notes, not problems), fixing the FP inflation that penalized thorough clean-pass reviewers. Also trims the dogfood swarm to the strong reviewers: drops m5/qwen3.6 (last local lane), gemma4, gpt-oss:120b, and kimi-k2.7-code — leaving 6 cloud + claude-code/sonnet. Fittingly, PR #4's own 11-model review produced 43 findings that were ALL clean-verification bullets (zero real) — a live demonstration of the bug this fixes. gofmt clean, go vet quiet, go test -race green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Steve Dudenhoeffer <steve@stevedudenhoeffer.com> Co-committed-by: Steve Dudenhoeffer <steve@stevedudenhoeffer.com>
This commit was merged in pull request #4.
This commit is contained in:
@@ -4,8 +4,8 @@
|
|||||||
# caches :latest, and this build is what carries foreman provider-type support)
|
# caches :latest, and this build is what carries foreman provider-type support)
|
||||||
# as a specialist swarm and posts
|
# as a specialist swarm and posts
|
||||||
# ONE consolidated review comment as gitea-actions. Advisory only — never blocks a
|
# ONE consolidated review comment as gitea-actions. Advisory only — never blocks a
|
||||||
# merge. Gadfly reviewing its OWN PRs — dogfooding: 9 cloud + the M5 Mac + the
|
# merge. Gadfly reviewing its OWN PRs — dogfooding: 6 cloud models + the Claude
|
||||||
# Claude Code engine (sonnet) as a competitor (M1 dropped as too slow).
|
# Code engine (sonnet) as a competitor. Local Macs and weak cloud models dropped.
|
||||||
|
|
||||||
name: Adversarial Review (Gadfly)
|
name: Adversarial Review (Gadfly)
|
||||||
|
|
||||||
@@ -41,8 +41,8 @@ jobs:
|
|||||||
|| github.actor == 'fizi'
|
|| github.actor == 'fizi'
|
||||||
|| github.actor == 'dazed'))
|
|| github.actor == 'dazed'))
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
# Fleet (9 cloud + M5 Mac + claude-code/sonnet, all concurrent) reviewing
|
# Fleet (6 cloud + claude-code/sonnet, all concurrent) reviewing every PR
|
||||||
# every PR with the 3-lens suite — the slow local lane dominates wall time.
|
# with the 3-lens suite. All cloud now, so runs are fast.
|
||||||
timeout-minutes: 90
|
timeout-minutes: 90
|
||||||
steps:
|
steps:
|
||||||
- uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:sha-86f12c1
|
- uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:sha-86f12c1
|
||||||
@@ -54,38 +54,20 @@ jobs:
|
|||||||
# below): Pro/Max subscription token. Dogfoods the Phase-1 engine on
|
# below): Pro/Max subscription token. Dogfoods the Phase-1 engine on
|
||||||
# gadfly's own PRs as a competitor alongside the Ollama models.
|
# gadfly's own PRs as a competitor alongside the Ollama models.
|
||||||
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||||
# Local Mac (M5), reached through its foreman queue (native Ollama on the
|
# Fleet: 6 cloud (3 at a time) + Claude Code (sonnet) — one consolidated
|
||||||
# wire). Gadfly's GADFLY_ENDPOINT_* form with the "foreman" provider
|
# comment each, all cloud now. The local Macs (m1/m5) and the weaker
|
||||||
# type: GADFLY_ENDPOINT_M5 registers provider "m5", building a
|
# cloud models (gemma4, gpt-oss:120b, kimi-k2.7-code) were dropped as
|
||||||
# foreman-preset Ollama client at the given URL. (M1 is dropped from
|
# low-signal for gadfly's own PRs. claude-code/sonnet runs the Phase-1
|
||||||
# gadfly's swarm — too slow/low-signal — so its endpoint isn't mapped.)
|
# engine as a competitor in its own lane (needs CLAUDE_CODE_OAUTH_TOKEN).
|
||||||
# Values
|
GADFLY_MODELS: "minimax-m3:cloud,glm-5.2:cloud,glm-5.1:cloud,deepseek-v4-pro:cloud,nemotron-3-super:cloud,qwen3-coder:480b-cloud,claude-code/sonnet"
|
||||||
# (host + token) live in gitea secrets, each of the form:
|
# cloud runs 3 at once; claude-code one at a time; both lanes parallel.
|
||||||
# foreman|https://<foreman-host>|<token>
|
GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3,claude-code=1"
|
||||||
# (converted from the komodo LLM_* DSNs foreman://<token>@<host>).
|
|
||||||
# REQUIRES a Gadfly image built with foreman provider-type support
|
|
||||||
# (the GADFLY_ENDPOINT "foreman|..." type); on an older image the m1/m5
|
|
||||||
# lanes error with "unknown provider foreman". The HTTPS-only LLM_*
|
|
||||||
# foreman:// DSN is the alternative that needs no image rebuild.
|
|
||||||
# NOTE: the Mac behind each foreman must still be awake/reachable; if a
|
|
||||||
# box is offline, that model's comment shows an error and the others
|
|
||||||
# still post. (Gitea secrets aren't auto-exposed — map each explicitly.)
|
|
||||||
GADFLY_ENDPOINT_M5: ${{ secrets.GADFLY_ENDPOINT_M5 }}
|
|
||||||
# Fleet: 9 cloud (3 at a time) + M5 Max + Claude Code (sonnet) — one
|
|
||||||
# consolidated comment each. Matches mort's cloud set so the scoreboard
|
|
||||||
# is comparable; M1 Pro is intentionally dropped here (too slow); and
|
|
||||||
# claude-code/sonnet runs the Phase-1 engine as a competitor (its own
|
|
||||||
# lane, needs CLAUDE_CODE_OAUTH_TOKEN above).
|
|
||||||
GADFLY_MODELS: "minimax-m3:cloud,glm-5.2:cloud,glm-5.1:cloud,kimi-k2.7-code:cloud,deepseek-v4-pro:cloud,nemotron-3-super:cloud,gpt-oss:120b-cloud,qwen3-coder:480b-cloud,gemma4:cloud,m5/qwen3.6:35b-mlx,claude-code/sonnet"
|
|
||||||
# cloud runs 3 at once; the Mac one at a time; claude-code one at a time;
|
|
||||||
# all three lanes run in parallel.
|
|
||||||
GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3,m5=1,claude-code=1"
|
|
||||||
# 3 cloud models x 3 lenses = 9 concurrent ollama-cloud queries (under the 10 budget).
|
# 3 cloud models x 3 lenses = 9 concurrent ollama-cloud queries (under the 10 budget).
|
||||||
GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3"
|
GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3"
|
||||||
# Default => the 3-lens suite (security, correctness, error-handling).
|
# Default => the 3-lens suite (security, correctness, error-handling).
|
||||||
# Set the repo var GADFLY_SPECIALISTS to override (csv / "all" / "auto").
|
# Set the repo var GADFLY_SPECIALISTS to override (csv / "all" / "auto").
|
||||||
GADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}
|
GADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}
|
||||||
# Per-lens deadline + bounded steps so the slow local models stay sane.
|
# Per-lens deadline + bounded steps (also bounds the claude-code lane).
|
||||||
GADFLY_TIMEOUT_SECS: "600"
|
GADFLY_TIMEOUT_SECS: "600"
|
||||||
GADFLY_MAX_STEPS: "14"
|
GADFLY_MAX_STEPS: "14"
|
||||||
# Allow-list for the comment trigger (mirrors the job-level if: guard).
|
# Allow-list for the comment trigger (mirrors the job-level if: guard).
|
||||||
|
|||||||
@@ -350,9 +350,13 @@ context (`GADFLY_REPO`, `GADFLY_PR`) automatically.
|
|||||||
|
|
||||||
Findings are extracted heuristically from each lens's markdown — a `path:line`
|
Findings are extracted heuristically from each lens's markdown — a `path:line`
|
||||||
reference anchors a finding, titled by the nearest preceding heading / numbered
|
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,
|
item / bold lead-in. A lens whose verdict is **"No material issues found"**
|
||||||
any error (or a non-2xx response) is logged to stderr only, and it **never**
|
emits **no** findings: its `path:line` references are verification notes
|
||||||
changes the review output or the exit code.
|
("verified X is safe"), not problems, so extracting them would record false
|
||||||
|
positives and unfairly penalize thorough clean-pass reviewers. 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
|
## Building locally
|
||||||
|
|
||||||
|
|||||||
@@ -121,6 +121,14 @@ func emit(results []specialistResult, elapsed time.Duration) {
|
|||||||
if r.errored {
|
if r.errored {
|
||||||
continue // a failed lens contributes no findings
|
continue // a failed lens contributes no findings
|
||||||
}
|
}
|
||||||
|
// A lens that reports "No material issues found" has nothing to flag —
|
||||||
|
// its path:line references are verification notes ("verified X at
|
||||||
|
// file:line is safe"), not problems. Extracting them pollutes the
|
||||||
|
// findings store with false positives and unfairly penalizes thorough
|
||||||
|
// reviewers that do clean passes, so a clean lens emits no findings.
|
||||||
|
if r.verdict == verdictClean {
|
||||||
|
continue
|
||||||
|
}
|
||||||
sev := r.verdict.label()
|
sev := r.verdict.label()
|
||||||
for _, f := range parseFindings(r.spec, r.out) {
|
for _, f := range parseFindings(r.spec, r.out) {
|
||||||
reports = append(reports, reportPayload{
|
reports = append(reports, reportPayload{
|
||||||
|
|||||||
@@ -179,6 +179,46 @@ func TestEmit_PostsRunsAndReports(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A clean "No material issues found" lens must NOT emit findings, even though
|
||||||
|
// its markdown contains path:line references (those are verification notes, not
|
||||||
|
// problems). /runs is still posted; /reports is not.
|
||||||
|
func TestEmit_SkipsCleanVerdictLens(t *testing.T) {
|
||||||
|
var runs, reports int32
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
switch r.URL.Path {
|
||||||
|
case "/runs":
|
||||||
|
atomic.AddInt32(&runs, 1)
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
_, _ = w.Write([]byte(`{"run_id":"x"}`))
|
||||||
|
case "/reports":
|
||||||
|
atomic.AddInt32(&reports, 1)
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
default:
|
||||||
|
http.Error(w, "not found", http.StatusNotFound)
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
t.Setenv("GADFLY_FINDINGS_URL", srv.URL)
|
||||||
|
t.Setenv("GADFLY_REPO", "owner/repo")
|
||||||
|
t.Setenv("GADFLY_PR", "7")
|
||||||
|
t.Setenv("GADFLY_MODEL", "ollama-cloud/qwen3")
|
||||||
|
|
||||||
|
// Clean verdict, but the markdown is full of path:line "verified X" notes.
|
||||||
|
cleanMarkdown := "No material issues found.\n\nVerified `run/executor.go:166` handles the error.\n"
|
||||||
|
results := []specialistResult{
|
||||||
|
{spec: Specialist{Name: "security"}, out: cleanMarkdown, verdict: verdictClean},
|
||||||
|
}
|
||||||
|
emit(results, time.Second)
|
||||||
|
|
||||||
|
if got := atomic.LoadInt32(&runs); got != 1 {
|
||||||
|
t.Fatalf("/runs received %d times, want 1 (the run is still recorded)", got)
|
||||||
|
}
|
||||||
|
if got := atomic.LoadInt32(&reports); got != 0 {
|
||||||
|
t.Fatalf("/reports received %d times, want 0 (clean lens emits no findings)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestEmit_DisabledMakesNoRequests(t *testing.T) {
|
func TestEmit_DisabledMakesNoRequests(t *testing.T) {
|
||||||
var hits int32
|
var hits int32
|
||||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|||||||
Reference in New Issue
Block a user