From 80d8f53f635763ec12c53906fbbd3c0ac3febc16 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 27 Jun 2026 22:14:07 +0000 Subject: [PATCH] fix: clean-lens findings + trim the dogfood swarm to strong reviewers (#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Co-authored-by: Steve Dudenhoeffer Co-committed-by: Steve Dudenhoeffer --- .gitea/workflows/adversarial-review.yml | 44 ++++++++----------------- README.md | 10 ++++-- cmd/gadfly/emit.go | 8 +++++ cmd/gadfly/emit_test.go | 40 ++++++++++++++++++++++ 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/.gitea/workflows/adversarial-review.yml b/.gitea/workflows/adversarial-review.yml index 3362688..b58834e 100644 --- a/.gitea/workflows/adversarial-review.yml +++ b/.gitea/workflows/adversarial-review.yml @@ -4,8 +4,8 @@ # caches :latest, and this build is what carries foreman provider-type support) # as a specialist swarm and posts # 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 -# Claude Code engine (sonnet) as a competitor (M1 dropped as too slow). +# merge. Gadfly reviewing its OWN PRs — dogfooding: 6 cloud models + the Claude +# Code engine (sonnet) as a competitor. Local Macs and weak cloud models dropped. name: Adversarial Review (Gadfly) @@ -41,8 +41,8 @@ jobs: || github.actor == 'fizi' || github.actor == 'dazed')) runs-on: ubuntu-latest - # Fleet (9 cloud + M5 Mac + claude-code/sonnet, all concurrent) reviewing - # every PR with the 3-lens suite — the slow local lane dominates wall time. + # Fleet (6 cloud + claude-code/sonnet, all concurrent) reviewing every PR + # with the 3-lens suite. All cloud now, so runs are fast. timeout-minutes: 90 steps: - 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 # gadfly's own PRs as a competitor alongside the Ollama models. CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - # Local Mac (M5), reached through its foreman queue (native Ollama on the - # wire). Gadfly's GADFLY_ENDPOINT_* form with the "foreman" provider - # type: GADFLY_ENDPOINT_M5 registers provider "m5", building a - # foreman-preset Ollama client at the given URL. (M1 is dropped from - # gadfly's swarm — too slow/low-signal — so its endpoint isn't mapped.) - # Values - # (host + token) live in gitea secrets, each of the form: - # foreman|https://| - # (converted from the komodo LLM_* DSNs foreman://@). - # 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" + # Fleet: 6 cloud (3 at a time) + Claude Code (sonnet) — one consolidated + # comment each, all cloud now. The local Macs (m1/m5) and the weaker + # cloud models (gemma4, gpt-oss:120b, kimi-k2.7-code) were dropped as + # low-signal for gadfly's own PRs. claude-code/sonnet runs the Phase-1 + # engine as a competitor in its own lane (needs CLAUDE_CODE_OAUTH_TOKEN). + 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" + # cloud runs 3 at once; claude-code one at a time; both lanes parallel. + GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3,claude-code=1" # 3 cloud models x 3 lenses = 9 concurrent ollama-cloud queries (under the 10 budget). GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3" # Default => the 3-lens suite (security, correctness, error-handling). # Set the repo var GADFLY_SPECIALISTS to override (csv / "all" / "auto"). 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_MAX_STEPS: "14" # Allow-list for the comment trigger (mirrors the job-level if: guard). diff --git a/README.md b/README.md index 19c0da4..77870e1 100644 --- a/README.md +++ b/README.md @@ -350,9 +350,13 @@ 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. +item / bold lead-in. A lens whose verdict is **"No material issues found"** +emits **no** findings: its `path:line` references are verification notes +("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 diff --git a/cmd/gadfly/emit.go b/cmd/gadfly/emit.go index 6e83fce..2cfc9c0 100644 --- a/cmd/gadfly/emit.go +++ b/cmd/gadfly/emit.go @@ -121,6 +121,14 @@ func emit(results []specialistResult, elapsed time.Duration) { if r.errored { 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() for _, f := range parseFindings(r.spec, r.out) { reports = append(reports, reportPayload{ diff --git a/cmd/gadfly/emit_test.go b/cmd/gadfly/emit_test.go index a9fca88..fe1b29b 100644 --- a/cmd/gadfly/emit_test.go +++ b/cmd/gadfly/emit_test.go @@ -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) { var hits int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {