amadan

carlos / todomvc-bakeoff Public

feat: record review findings; per-phase tokens for resumed sessions; Go review and fix round

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 0f093187ef6f58d5fb1edeb0f6553271b17d8e54 parent 2eeda2a
7 files changed, +127 −12
  • builders/reviews/go.1.md +3 −0
  • builders/reviews/go.2.md +1 −0
  • harness/cmd/bakeoff/main.go +43 −0
  • harness/findings.go +34 −0
  • harness/harness_test.go +24 −0
  • harness/tokens.go +8 −0
  • results/go.json +14 −12
diff --git a/builders/reviews/go.1.md b/builders/reviews/go.1.md
new file mode 100644
index 0000000..ca72015
--- /dev/null
+++ b/builders/reviews/go.1.md
@@ -0,0 +1,3 @@
+1. [spec] [apps/go/web/web.go:268](/home/paulca/amadan.net/carlos/todomvc-bakeoff/.worktrees/go/apps/go/web/web.go:268): Any cookie decoding to 32 bytes is accepted, even if the server never issued it. For example, 43 `A` characters becomes a persistent, predictable list identity instead of receiving a fresh random cookie. Visitors supplying that same unissued value share a list. Track issued identities and replace unknown cookies.
+
+2. [correctness] [apps/go/web/static/app.js:41](/home/paulca/amadan.net/carlos/todomvc-bakeoff/.worktrees/go/apps/go/web/static/app.js:41): Every response removes the entire list, including any edit started while that request was pending. Toggle one item, then edit another before the toggle response arrives: the response replaces the edit field and discards the draft. Preserve active edits across fragment replacements or defer replacement until the edit finishes.
\ No newline at end of file
diff --git a/builders/reviews/go.2.md b/builders/reviews/go.2.md
new file mode 100644
index 0000000..7e6f9b1
--- /dev/null
+++ b/builders/reviews/go.2.md
@@ -0,0 +1 @@
+1. [spec] [apps/go/main.go:114](/home/paulca/amadan.net/carlos/todomvc-bakeoff/.worktrees/go/apps/go/main.go:114): `checkLoopback` accepts any loopback address, including `127.0.0.2` and `::1`. The brief explicitly requires binding only to `127.0.0.1`. These alternative `--listen` addresses pass validation and start a listener; restrict validation to the required address.
\ No newline at end of file
diff --git a/harness/cmd/bakeoff/main.go b/harness/cmd/bakeoff/main.go
index fa3ffa1..1883503 100644
--- a/harness/cmd/bakeoff/main.go
+++ b/harness/cmd/bakeoff/main.go
@@ -68,6 +68,8 @@ func main() {
err = behaviour(*slug, *file)
case "report":
err = renderReport(*file)
+ case "findings":
+ err = findings(*slug, *file)
case "source":
err = source(*dir)
case "readability":
@@ -225,6 +227,28 @@ func tokens(slug, logs, phase string) error {
}
total = total.Add(t)
}
+ // Fix and security rounds resume the build session, whose result
+ // events report usage cumulatively: subtract the phase before.
+ if phase == "fix" || phase == "security" {
+ prev := []string{"build"}
+ if phase == "security" {
+ prev = []string{"fix", "build"}
+ }
+ for _, p := range prev {
+ f := filepath.Join(logs, slug+"."+p+".jsonl")
+ fh, err := os.Open(f)
+ if err != nil {
+ continue
+ }
+ before, err := b.ParseClaudeLog(fh)
+ fh.Close()
+ if err != nil {
+ return fmt.Errorf("%s: %w", f, err)
+ }
+ total = total.Since(before)
+ break
+ }
+ }
fmt.Printf("%s %s tokens: %+v\n", slug, phase, total)
return b.Update(slug, func(r *b.AppResult) {
switch phase {
@@ -318,3 +342,22 @@ func renderReport(copyFile string) error {
}
return report.Render(rs, report.ParseCopy(string(text)), os.Stdout)
}
+
+// findings records what a review left unfixed and exits 4 if any of it
+// is a security finding (which earns the security-only round).
+func findings(slug, file string) error {
+ text, err := os.ReadFile(file)
+ if err != nil {
+ return err
+ }
+ fs := b.ParseFindings(string(text))
+ fmt.Printf("%s: %d findings remain\n", slug, len(fs))
+ if err := b.Update(slug, func(r *b.AppResult) { r.ReviewFindings = fs }); err != nil {
+ return err
+ }
+ if b.HasSecurity(fs) {
+ fmt.Printf("%s: security findings remain\n", slug)
+ os.Exit(4)
+ }
+ return nil
+}
diff --git a/harness/findings.go b/harness/findings.go
new file mode 100644
index 0000000..e19be42
--- /dev/null
+++ b/harness/findings.go
@@ -0,0 +1,34 @@
+package bakeoff
+
+import (
+ "regexp"
+ "strings"
+)
+
+var findingStart = regexp.MustCompile(`^\s*\d+\.\s+`)
+
+// ParseFindings splits a reviewer's numbered list into one string per item,
+// joining wrapped lines. "No findings." yields none.
+func ParseFindings(text string) []string {
+ var out []string
+ for _, line := range strings.Split(text, "\n") {
+ line = strings.TrimSpace(line)
+ switch {
+ case findingStart.MatchString(line):
+ out = append(out, findingStart.ReplaceAllString(line, ""))
+ case line != "" && len(out) > 0:
+ out[len(out)-1] += " " + line
+ }
+ }
+ return out
+}
+
+// HasSecurity reports whether any finding is tagged [security].
+func HasSecurity(findings []string) bool {
+ for _, f := range findings {
+ if strings.HasPrefix(f, "[security]") {
+ return true
+ }
+ }
+ return false
+}
diff --git a/harness/harness_test.go b/harness/harness_test.go
index 951e9c5..0a4618c 100644
--- a/harness/harness_test.go
+++ b/harness/harness_test.go
@@ -230,3 +230,27 @@ func TestMedianReadability(t *testing.T) {
t.Fatalf("got %+v (notes come from the run whose overall is the median)", r)
}
}
+
+func TestParseFindings(t *testing.T) {
+ got := ParseFindings("1. [spec] [a.go:1](/x/a.go:1): first\n continues here.\n\n2. [security] b: second\n")
+ if len(got) != 2 || got[0] != "[spec] [a.go:1](/x/a.go:1): first continues here." || got[1] != "[security] b: second" {
+ t.Fatalf("got %q", got)
+ }
+ if len(ParseFindings("No findings.")) != 0 {
+ t.Fatal("No findings. must parse to none")
+ }
+ if !HasSecurity(got) || HasSecurity(got[:1]) {
+ t.Fatal("HasSecurity wrong")
+ }
+}
+
+func TestTokensSince(t *testing.T) {
+ build := Tokens{Input: 82, Output: 56663, CacheRead: 4280334, CacheWrite: 132243, Turns: 46, Seconds: 693, CostUSD: 3.05}
+ fixCumulative := Tokens{Input: 106, Output: 65224, CacheRead: 6096946, CacheWrite: 145431, Turns: 12, Seconds: 183, CostUSD: 3.69}
+ got := fixCumulative.Since(build)
+ want := Tokens{Input: 24, Output: 8561, CacheRead: 1816612, CacheWrite: 13188, Turns: 12, Seconds: 183}
+ if got.Input != want.Input || got.Output != want.Output || got.CacheRead != want.CacheRead || got.CacheWrite != want.CacheWrite ||
+ got.Turns != 12 || got.Seconds != 183 || got.CostUSD < 0.63 || got.CostUSD > 0.65 {
+ t.Fatalf("got %+v", got)
+ }
+}
diff --git a/harness/tokens.go b/harness/tokens.go
index 6f583d2..ce45e5f 100644
--- a/harness/tokens.go
+++ b/harness/tokens.go
@@ -109,3 +109,11 @@ func ParseCodexLog(r io.Reader) (Tokens, error) {
}
return t, err
}
+
+// Since turns a resumed session's cumulative totals into that phase's own:
+// claude reports usage and cost for the whole session on resume, but turns
+// and duration for the resumed run only.
+func (t Tokens) Since(prev Tokens) Tokens {
+ return Tokens{t.Input - prev.Input, t.Output - prev.Output, t.CacheRead - prev.CacheRead, t.CacheWrite - prev.CacheWrite,
+ t.Turns, t.Seconds, t.CostUSD - prev.CostUSD}
+}
diff --git a/results/go.json b/results/go.json
index 8299739..389aa63 100644
--- a/results/go.json
+++ b/results/go.json
@@ -62,13 +62,13 @@
"CostUSD": 3.0475988
},
"TokensFix": {
- "Input": 0,
- "Output": 0,
- "CacheRead": 0,
- "CacheWrite": 0,
- "Turns": 0,
- "Seconds": 0,
- "CostUSD": 0
+ "Input": 24,
+ "Output": 8561,
+ "CacheRead": 1816612,
+ "CacheWrite": 13188,
+ "Turns": 12,
+ "Seconds": 183.065,
+ "CostUSD": 0.6401423999999998
},
"TokensSecurity": {
"Input": 0,
@@ -80,11 +80,11 @@
"CostUSD": 0
},
"TokensReview": {
- "Input": 0,
- "Output": 0,
- "CacheRead": 0,
+ "Input": 514223,
+ "Output": 2623,
+ "CacheRead": 421504,
"CacheWrite": 0,
- "Turns": 0,
+ "Turns": 2,
"Seconds": 0,
"CostUSD": 0
},
@@ -97,6 +97,8 @@
"Overall": 0,
"Notes": ""
},
- "ReviewFindings": null,
+ "ReviewFindings": [
+ "[spec] [apps/go/main.go:114](/home/paulca/amadan.net/carlos/todomvc-bakeoff/.worktrees/go/apps/go/main.go:114): `checkLoopback` accepts any loopback address, including `127.0.0.2` and `::1`. The brief explicitly requires binding only to `127.0.0.1`. These alternative `--listen` addresses pass validation and start a listener; restrict validation to the required address."
+ ],
"Notes": null
}