carlos / todomvc-bakeoff Public
feat: review, fix and blind readability scripts
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9 files changed,
+268
−19
builders/prompts/readability.md+18 −0builders/prompts/review.md+23 −0builders/run-fix.sh+14 −0builders/run-readability.sh+25 −0builders/run-review.sh+12 −0harness/cmd/bakeoff/main.go+41 −2harness/codestats.go+58 −17harness/harness_test.go+25 −0harness/readability.go+52 −0
diff --git a/builders/prompts/readability.md b/builders/prompts/readability.md| new file mode 100644 |
| index 0000000..f64a98d |
| --- /dev/null |
| +++ b/builders/prompts/readability.md |
| @@ -0,0 +1,18 @@ |
| +Below is the complete source of a small web app (a server-rendered |
| +TodoMVC with a little JavaScript), written in {LANGUAGE}. Judge how |
| +readable it is for an experienced developer who knows {LANGUAGE} but has |
| +never seen this code. Judge the code, not the choice of language. |
| + |
| +Score each from 1 (poor) to 5 (excellent): |
| +- naming: names say what things are and do |
| +- structure: files and functions are organised so you can find things |
| +- traceability: how easily you can follow one request (e.g. toggling an |
| + item) from the route to the database and back to the HTML |
| +- errors: errors are handled clearly and visibly, not swallowed |
| +- comments: comments explain why where it isn't obvious, without noise |
| + |
| +Then an overall score from 1 to 10, and notes of at most 120 words naming |
| +the strongest and weakest points. |
| + |
| +Reply with only this JSON object and nothing else: |
| +{"naming":n,"structure":n,"traceability":n,"errors":n,"comments":n,"overall":n,"notes":"..."} |
diff --git a/builders/prompts/review.md b/builders/prompts/review.md| new file mode 100644 |
| index 0000000..bb01ebf |
| --- /dev/null |
| +++ b/builders/prompts/review.md |
| @@ -0,0 +1,23 @@ |
| +You are reviewing one entry in the CARLOS TodoMVC bakeoff: the |
| +**{LANGUAGE}** app in `apps/{SLUG}/`. It was built by another agent from |
| +`BRIEF.md` and `CONTRACT.md` (read both), against the TodoMVC app spec |
| +(https://github.com/tastejs/todomvc/blob/master/app-spec.md) as the |
| +contract adapts it. |
| + |
| +Review for **correctness, spec compliance and security only**. Don't |
| +suggest performance tuning, style changes or refactors. Every app gets |
| +exactly one fix round, so list only what a reasonable person would need |
| +fixed: behaviour that's wrong, a contract or brief requirement that's |
| +missed, or a security problem (injection, cross-site, cross-list access, |
| +cookie handling, unsafe SQL, resource exhaustion from a single request, |
| +anything that leaks or corrupts another visitor's data). |
| + |
| +Check the platform contract too: `--socket`/`--db`/`--listen`, |
| +`/healthz`, `/api/version`, `CARLOS_HOST`, static or libc-only linkage, |
| +WAL, additive migrations, SIGTERM, and the Makefile targets including |
| +`clean-deps`, `bump` and `unbump`. |
| + |
| +Write a numbered list. Start each item with exactly one tag: |
| +`[security]`, `[correctness]` or `[spec]`, then the file and line, what's |
| +wrong, and how it shows up. If nothing needs fixing, write exactly |
| +`No findings.` Write nothing else. |
diff --git a/builders/run-fix.sh b/builders/run-fix.sh| new file mode 100755 |
| index 0000000..3e38a54 |
| --- /dev/null |
| +++ b/builders/run-fix.sh |
| @@ -0,0 +1,14 @@ |
| +#!/usr/bin/env bash |
| +# The same builder session fixes review findings and redeploys. |
| +# builders/run-fix.sh <slug> <fix|security> <findings-file> |
| +set -euo pipefail |
| +slug=$1 phase=$2 findings=$3 |
| +repo=$(cd "$(dirname "$0")/.." && pwd) |
| +session=$(cat "$repo/builders/logs/$slug.session") |
| +prompt="An independent reviewer read your app. Fix the findings below (still unattended: no questions), keep the shared suite passing locally, redeploy with make deploy, re-run scripts/test-app.sh against https://todo-$slug.bfj.oncarlos.com, commit, and update STATUS.md. Fix only what is listed. If you disagree with a finding, leave the code as it is and say why in STATUS.md under '## Review findings not fixed'. |
| + |
| +$(cat "$findings")" |
| +cd "$repo/.worktrees/$slug" |
| +timeout 2h "$repo/builders/claude.sh" -p --resume "$session" "$prompt" \ |
| + > "$repo/builders/logs/$slug.$phase.jsonl" 2> "$repo/builders/logs/$slug.$phase.stderr" |
| +echo "$slug $phase exited $?" |
diff --git a/builders/run-readability.sh b/builders/run-readability.sh| new file mode 100755 |
| index 0000000..902649f |
| --- /dev/null |
| +++ b/builders/run-readability.sh |
| @@ -0,0 +1,25 @@ |
| +#!/usr/bin/env bash |
| +# Fable scores one app's readability three times, blind to which agent or |
| +# app it is (only the language is named); the per-field median is kept. |
| +# builders/run-readability.sh <slug> |
| +set -euo pipefail |
| +slug=$1 |
| +repo=$(cd "$(dirname "$0")/.." && pwd) |
| +cd "$repo" |
| +language=$(cd harness && go run ./cmd/language "$slug") |
| +work=$(mktemp -d -p "${TMPDIR:-$HOME/.cache}") |
| +trap 'rm -rf "$work"' EXIT |
| +{ |
| + sed "s/{LANGUAGE}/$language/g" builders/prompts/readability.md |
| + echo |
| + bakeoff source --slug "$slug" | sed -e "s/todo-$slug/todo-app/g" -e "s|apps/$slug|apps/app|g" |
| +} > "$work/prompt.md" |
| +replies=() |
| +for i in 1 2 3; do |
| + claude -p --model claude-fable-5-1 --output-format json --tools "" \ |
| + --mcp-config '{"mcpServers":{}}' --strict-mcp-config --no-session-persistence \ |
| + < "$work/prompt.md" > "builders/logs/$slug.readability$i.json" |
| + python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["result"])' "builders/logs/$slug.readability$i.json" > "$work/reply$i" |
| + replies+=("$work/reply$i") |
| +done |
| +bakeoff readability --slug "$slug" --replies "$(IFS=,; echo "${replies[*]}")" |
diff --git a/builders/run-review.sh b/builders/run-review.sh| new file mode 100755 |
| index 0000000..3459916 |
| --- /dev/null |
| +++ b/builders/run-review.sh |
| @@ -0,0 +1,12 @@ |
| +#!/usr/bin/env bash |
| +# Astra reviews one app, read-only, in the app's worktree. |
| +# builders/run-review.sh <slug> <n> (n = 1 first review, 2 re-review) |
| +set -euo pipefail |
| +slug=$1 n=$2 |
| +repo=$(cd "$(dirname "$0")/.." && pwd) |
| +language=$(cd "$repo/harness" && go run ./cmd/language "$slug") |
| +prompt=$(sed -e "s/{SLUG}/$slug/g" -e "s/{LANGUAGE}/$language/g" "$repo/builders/prompts/review.md") |
| +cd "$repo/.worktrees/$slug" |
| +codex exec -s read-only --skip-git-repo-check --json \ |
| + -o "$repo/builders/reviews/$slug.$n.md" "$prompt" </dev/null > "$repo/builders/logs/$slug.review$n.jsonl" |
| +echo "review $n for $slug: $(grep -c '^\s*[0-9]\+\.' "$repo/builders/reviews/$slug.$n.md" || true) findings" |
diff --git a/harness/cmd/bakeoff/main.go b/harness/cmd/bakeoff/main.go| index 5ad5a20..96d1b6a 100644 |
| --- a/harness/cmd/bakeoff/main.go |
| +++ b/harness/cmd/bakeoff/main.go |
| @@ -1,7 +1,7 @@ |
| // Command bakeoff measures one app and records the result in |
| // results/<slug>.json. Run it from the repo root. |
| // |
| -// bakeoff <ttfb|wake|load|weight|code|build|tokens|behaviour> --slug <s> [flags] |
| +// bakeoff <ttfb|wake|load|weight|code|build|tokens|behaviour|source|readability> --slug <s> [flags] |
| package main |
| import ( |
| @@ -35,6 +35,7 @@ func main() { |
| phase := fs.String("phase", "", "tokens: build|fix|security|review") |
| logs := fs.String("logs", "builders/logs", "tokens: log directory") |
| file := fs.String("file", "", "behaviour: Playwright JSON report") |
| + replies := fs.String("replies", "", "readability: comma-separated files, one scorer reply each") |
| fs.Parse(os.Args[2:]) |
| if *slug == "" { |
| usage() |
| @@ -64,6 +65,10 @@ func main() { |
| err = tokens(*slug, *logs, *phase) |
| case "behaviour": |
| err = behaviour(*slug, *file) |
| + case "source": |
| + err = source(*dir) |
| + case "readability": |
| + err = readability(*slug, *replies) |
| default: |
| usage() |
| } |
| @@ -74,7 +79,7 @@ func main() { |
| } |
| func usage() { |
| - fmt.Fprintln(os.Stderr, "usage: bakeoff <ttfb|wake|load|weight|code|build|tokens|behaviour> --slug <s> [flags]") |
| + fmt.Fprintln(os.Stderr, "usage: bakeoff <ttfb|wake|load|weight|code|build|tokens|behaviour|source|readability> --slug <s> [flags]") |
| os.Exit(2) |
| } |
| @@ -249,3 +254,37 @@ func behaviour(slug, file string) error { |
| fmt.Printf("%s behaviour: js-on %+v, js-off %+v\n", slug, got["js-on"], got["js-off"]) |
| return b.Update(slug, func(r *b.AppResult) { r.BehaviourJSOn, r.BehaviourJSOff = got["js-on"], got["js-off"] }) |
| } |
| + |
| +// source prints the app's own code, file by file, for the readability scorer. |
| +func source(dir string) error { |
| + files, err := b.SourceFiles(dir) |
| + if err != nil { |
| + return err |
| + } |
| + for _, f := range files { |
| + body, err := os.ReadFile(filepath.Join(dir, f)) |
| + if err != nil { |
| + return err |
| + } |
| + fmt.Printf("===== %s =====\n%s\n", f, body) |
| + } |
| + return nil |
| +} |
| + |
| +func readability(slug, replies string) error { |
| + var texts []string |
| + for _, f := range strings.Split(replies, ",") { |
| + body, err := os.ReadFile(f) |
| + if err != nil { |
| + return err |
| + } |
| + texts = append(texts, string(body)) |
| + } |
| + r, err := b.MedianReadability(texts) |
| + if err != nil { |
| + return err |
| + } |
| + fmt.Printf("%s readability: overall %.0f/10 (naming %.0f, structure %.0f, traceability %.0f, errors %.0f, comments %.0f)\n", |
| + slug, r.Overall, r.Naming, r.Structure, r.Traceability, r.Errors, r.Comments) |
| + return b.Update(slug, func(a *b.AppResult) { a.Readability = r }) |
| +} |
diff --git a/harness/codestats.go b/harness/codestats.go| index 7d85213..38f77e3 100644 |
| --- a/harness/codestats.go |
| +++ b/harness/codestats.go |
| @@ -6,6 +6,7 @@ import ( |
| "os" |
| "path/filepath" |
| "regexp" |
| + "sort" |
| "strings" |
| ) |
| @@ -21,15 +22,25 @@ var ( |
| vanName = regexp.MustCompile(`^van[-.]`) |
| ) |
| -// CountCode classifies every source file under dir. Build output, vendored |
| -// code, lockfiles, CSS and docs are skipped; DEPS is counted as Deps. |
| -func CountCode(dir string) (CodeStats, error) { |
| - var c CodeStats |
| - err := filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { |
| +// Roles a file can play in CountCode and SourceFiles. |
| +const ( |
| + roleServer = "server" |
| + roleTemplate = "templates" |
| + roleJS = "js" |
| + roleTest = "tests" |
| + roleDeps = "deps" |
| +) |
| + |
| +// walkCode calls f for every counted file under dir with its slash path |
| +// relative to dir and its role. Build output, vendored code, lockfiles, |
| +// CSS and docs are skipped. |
| +func walkCode(dir string, f func(rel, role string) error) error { |
| + return filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { |
| if err != nil { |
| return err |
| } |
| rel, _ := filepath.Rel(dir, p) |
| + rel = filepath.ToSlash(rel) |
| if d.IsDir() { |
| if p != dir && skipDirs[d.Name()] { |
| return filepath.SkipDir |
| @@ -37,19 +48,13 @@ func CountCode(dir string) (CodeStats, error) { |
| return nil |
| } |
| if rel == "DEPS" { |
| - n, err := lines(p) |
| - c.Deps = n |
| - return err |
| + return f(rel, roleDeps) |
| } |
| ext := strings.ToLower(filepath.Ext(p)) |
| if skipExts[ext] || !sourceExts[ext] || vanName.MatchString(d.Name()) { |
| return nil |
| } |
| - n, err := lines(p) |
| - if err != nil { |
| - return err |
| - } |
| - parts := strings.Split(filepath.ToSlash(rel), "/") |
| + parts := strings.Split(rel, "/") |
| inDir := func(names ...string) bool { |
| for _, seg := range parts[:len(parts)-1] { |
| for _, n := range names { |
| @@ -62,19 +67,55 @@ func CountCode(dir string) (CodeStats, error) { |
| } |
| switch { |
| case inDir("test", "tests") || testName.MatchString(d.Name()): |
| - c.Tests += n |
| + return f(rel, roleTest) |
| case ext == ".js" || ext == ".mjs": |
| - c.JS += n |
| + return f(rel, roleJS) |
| case templateExt[ext] || inDir("templates", "views"): |
| - c.Templates += n |
| - default: |
| + return f(rel, roleTemplate) |
| + } |
| + return f(rel, roleServer) |
| + }) |
| +} |
| + |
| +// CountCode counts non-blank lines under dir by role; DEPS counts as Deps. |
| +func CountCode(dir string) (CodeStats, error) { |
| + var c CodeStats |
| + err := walkCode(dir, func(rel, role string) error { |
| + n, err := lines(filepath.Join(dir, rel)) |
| + if err != nil { |
| + return err |
| + } |
| + switch role { |
| + case roleServer: |
| c.Server += n |
| + case roleTemplate: |
| + c.Templates += n |
| + case roleJS: |
| + c.JS += n |
| + case roleTest: |
| + c.Tests += n |
| + case roleDeps: |
| + c.Deps = n |
| } |
| return nil |
| }) |
| return c, err |
| } |
| +// SourceFiles lists the app's own server, template and JS files (no tests), |
| +// sorted: what a reader of the app has to read. |
| +func SourceFiles(dir string) ([]string, error) { |
| + var out []string |
| + err := walkCode(dir, func(rel, role string) error { |
| + if role == roleServer || role == roleTemplate || role == roleJS { |
| + out = append(out, rel) |
| + } |
| + return nil |
| + }) |
| + sort.Strings(out) |
| + return out, err |
| +} |
| + |
| func lines(path string) (int, error) { |
| f, err := os.Open(path) |
| if err != nil { |
diff --git a/harness/harness_test.go b/harness/harness_test.go| index 0c7ba3b..951e9c5 100644 |
| --- a/harness/harness_test.go |
| +++ b/harness/harness_test.go |
| @@ -205,3 +205,28 @@ func TestParsePlaywright(t *testing.T) { |
| t.Fatalf("got %+v (flaky counts as failed)", got) |
| } |
| } |
| + |
| +func TestSourceFiles(t *testing.T) { |
| + got, err := SourceFiles("testdata/codetree") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := []string{"main.go", "static/app.js", "views/index.html"} |
| + if strings.Join(got, ",") != strings.Join(want, ",") { |
| + t.Fatalf("got %v, want %v", got, want) |
| + } |
| +} |
| + |
| +func TestMedianReadability(t *testing.T) { |
| + r, err := MedianReadability([]string{ |
| + `{"naming":4,"structure":3,"traceability":5,"errors":2,"comments":3,"overall":7,"notes":"a"}`, |
| + "```json\n{\"naming\":2,\"structure\":4,\"traceability\":4,\"errors\":3,\"comments\":3,\"overall\":6,\"notes\":\"b\"}\n```", |
| + `{"naming":5,"structure":5,"traceability":3,"errors":4,"comments":2,"overall":8,"notes":"c"}`, |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if r.Naming != 4 || r.Structure != 4 || r.Traceability != 4 || r.Errors != 3 || r.Comments != 3 || r.Overall != 7 || r.Notes != "a" { |
| + t.Fatalf("got %+v (notes come from the run whose overall is the median)", r) |
| + } |
| +} |
diff --git a/harness/readability.go b/harness/readability.go| new file mode 100644 |
| index 0000000..0ddffe5 |
| --- /dev/null |
| +++ b/harness/readability.go |
| @@ -0,0 +1,52 @@ |
| +package bakeoff |
| + |
| +import ( |
| + "encoding/json" |
| + "fmt" |
| + "sort" |
| + "strings" |
| +) |
| + |
| +// MedianReadability combines several scorer replies: each score is the |
| +// median across replies, and the notes come from the reply whose overall |
| +// score is the median. Replies may wrap the JSON in a code fence. |
| +func MedianReadability(replies []string) (Readability, error) { |
| + type reply struct { |
| + Naming, Structure, Traceability, Errors, Comments, Overall float64 |
| + Notes string |
| + } |
| + var rs []reply |
| + for _, s := range replies { |
| + i, j := strings.Index(s, "{"), strings.LastIndex(s, "}") |
| + if i < 0 || j < i { |
| + return Readability{}, fmt.Errorf("no JSON object in reply %q", s) |
| + } |
| + var r reply |
| + if err := json.Unmarshal([]byte(s[i:j+1]), &r); err != nil { |
| + return Readability{}, err |
| + } |
| + rs = append(rs, r) |
| + } |
| + if len(rs) == 0 { |
| + return Readability{}, fmt.Errorf("no replies") |
| + } |
| + med := func(get func(reply) float64) float64 { |
| + v := make([]float64, len(rs)) |
| + for i, r := range rs { |
| + v[i] = get(r) |
| + } |
| + return median(v) |
| + } |
| + out := Readability{ |
| + Naming: med(func(r reply) float64 { return r.Naming }), |
| + Structure: med(func(r reply) float64 { return r.Structure }), |
| + Traceability: med(func(r reply) float64 { return r.Traceability }), |
| + Errors: med(func(r reply) float64 { return r.Errors }), |
| + Comments: med(func(r reply) float64 { return r.Comments }), |
| + Overall: med(func(r reply) float64 { return r.Overall }), |
| + } |
| + byOverall := append([]reply(nil), rs...) |
| + sort.SliceStable(byOverall, func(i, j int) bool { return byOverall[i].Overall < byOverall[j].Overall }) |
| + out.Notes = byOverall[len(byOverall)/2].Notes |
| + return out, nil |
| +} |