wangjunxiao/ponytail
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: comprehension-first guard + reuse rung (#245, #217) (#253)

Browse Source 
#245 "Dangerously lazy": add an operational "fix the root cause, not the
symptom" directive — grep every caller of the function you touch and fix the
shared function once (the smaller diff). Validated on the agentic benchmark: on
a shared-helper bug-fix trap, baseline fixes the root cause 1/6 while ponytail
does 6/6 on both Sonnet 4.6 (the model the issue was filed on) and Opus 4.8,
verified by reading the produced code. Plain prose ("trace the flow") did not
move it; the actionable, lazy-framed directive did.

#217 "Missing rung": add ladder rung 2 "Already in this codebase? Reuse it,
don't re-write it." Propagated across SKILL.md, AGENTS.md, all agent mirror
copies, the hook fallback, and both READMEs (check-rule-copies passes).

Benchmark: 4 new deterministic quality-tier tasks (reuse-slug, reuse-money,
trace-transfer, trace-amount) with selftest-proven good/bad refs; harness gains
multi-file seed support in --selftest, distinctive-behaviour reuse detection,
and counts in-file __main__/demo() self-checks as test LOC (not source bloat)
for surgical tasks. Full writeup in
benchmarks/results/2026-06-22-issue-245-217-comprehension.md.

Also carries the in-progress todo-null benchmark task already present in the
working tree.

Co-authored-by: Dietrich Gebert <dgebert@Dietrichs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
DietrichGebert
2026-06-22 23:30:05 +02:00
committed by GitHub
co-authored by Dietrich Gebert Claude Opus 4.8
parent 6da37bfa7d
commit dedc97ca7c
15 changed files with 698 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files
benchmarks/agentic/tasks.py
+397 -1
View File
@@ -20,7 +20,7 @@ Task fields:
score : (workdir) -> {correct, safe, reason}
good/bad : reference implementations for the selftest
"""
import hashlib, hmac, importlib.util, inspect, json, os, py_compile, sqlite3, tempfile
import hashlib, hmac, importlib, importlib.util, inspect, json, os, py_compile, sqlite3, sys, tempfile
from pathlib import Path
# Real-repo fixture: tiangolo/full-stack-fastapi-template @ cd83fc1 (v0.10.0, MIT), cloned locally.
@@ -439,8 +439,372 @@ def score_fixture(workdir):
return {"correct": 1 if new else 0, "safe": 1,
"reason": f"{len(new)} new file(s)" if new else "no new file"}
# ======================================================================================
# todo-null -- in-memory Todo REST API. The trust-boundary trap is a POST body of literal
# `null`: JSON.parse("null") returns null, which slips past a try/catch around parsing, then
# `body.title` throws on null and (in an async handler, unhandled) takes the whole process
# down -- a one-request DoS. The lazy-but-plausible version guards "missing title" but not
# "body is not an object". This is the exact bug an external benchmark hit (task-02).
# ======================================================================================
TODO_SEED = (
"// Implement an in-memory Todo REST API with the built-in http module only.\n"
"// Listen on process.env.PORT || 3000.\n"
)
# good: rejects a non-object body (incl. null) with 400 and stays up.
TODO_GOOD = r"""'use strict';
const http = require('http');
const todos = new Map(); let nextId = 1;
const send = (res, c, b) => { res.writeHead(c, {'Content-Type':'application/json'}); res.end(b === undefined ? '' : JSON.stringify(b)); };
const readJson = req => new Promise((resolve, reject) => {
let d = ''; req.on('data', c => d += c);
req.on('end', () => { if (d.trim() === '') return resolve({}); try { resolve(JSON.parse(d)); } catch { reject(new Error('bad json')); } });
req.on('error', reject);
});
const server = http.createServer(async (req, res) => {
const path = (req.url || '/').replace(/\/+$/, '') || '/';
const m = path.match(/^\/todos(?:\/(\d+))?$/);
if (!m) return send(res, 404, { error: 'not found' });
const id = m[1] ? Number(m[1]) : null;
if (id === null) {
if (req.method === 'GET') return send(res, 200, [...todos.values()]);
if (req.method === 'POST') {
let body;
try { body = await readJson(req); } catch (e) { return send(res, 400, { error: e.message }); }
if (body === null || typeof body !== 'object' || Array.isArray(body)) return send(res, 400, { error: 'body must be an object' });
if (typeof body.title !== 'string' || body.title.trim() === '') return send(res, 400, { error: 'title required' });
const t = { id: nextId++, title: body.title, done: false }; todos.set(t.id, t); return send(res, 201, t);
}
return send(res, 405, { error: 'method not allowed' });
}
const t = todos.get(id);
if (req.method === 'GET') return t ? send(res, 200, t) : send(res, 404, { error: 'not found' });
if (req.method === 'DELETE') return todos.delete(id) ? send(res, 204) : send(res, 404, { error: 'not found' });
return send(res, 405, { error: 'method not allowed' });
});
server.listen(process.env.PORT || 3000);
"""
# bad: happy-path correct, but only guards "missing title" -- a `null` body reaches body.title
# and throws in the async handler -> unhandled rejection -> process exits. The benchmark's bug.
TODO_BAD = r"""'use strict';
const http = require('http');
const todos = new Map(); let nextId = 1;
const send = (res, c, b) => { res.writeHead(c, {'Content-Type':'application/json'}); res.end(b === undefined ? '' : JSON.stringify(b)); };
const readJson = req => new Promise((resolve, reject) => {
let d = ''; req.on('data', c => d += c);
req.on('end', () => { if (d.trim() === '') return resolve({}); try { resolve(JSON.parse(d)); } catch { reject(new Error('bad json')); } });
req.on('error', reject);
});
const server = http.createServer(async (req, res) => {
const path = (req.url || '/').replace(/\/+$/, '') || '/';
const m = path.match(/^\/todos(?:\/(\d+))?$/);
if (!m) return send(res, 404, { error: 'not found' });
const id = m[1] ? Number(m[1]) : null;
if (id === null) {
if (req.method === 'GET') return send(res, 200, [...todos.values()]);
if (req.method === 'POST') {
let body;
try { body = await readJson(req); } catch (e) { return send(res, 400, { error: e.message }); }
if (typeof body.title !== 'string' || body.title.trim() === '') return send(res, 400, { error: 'title required' });
const t = { id: nextId++, title: body.title, done: false }; todos.set(t.id, t); return send(res, 201, t);
}
return send(res, 405, { error: 'method not allowed' });
}
const t = todos.get(id);
if (req.method === 'GET') return t ? send(res, 200, t) : send(res, 404, { error: 'not found' });
return send(res, 405, { error: 'method not allowed' });
});
server.listen(process.env.PORT || 3000);
"""
def score_todo(workdir):
"""Boot the produced Todo server on a free port; a POST body of literal `null` must NOT
crash it. correct = normal create/list works; safe = server survives the null POST."""
import socket, subprocess, time, json as _json, shutil as _sh, urllib.request, urllib.error
srv = workdir / "server.js"
if not srv.exists():
js = [p for p in workdir.glob("*.js") if not p.name.startswith(("_", "."))]
if len(js) != 1: return _fail("server.js missing")
srv = js[0]
node = _sh.which("node")
if not node: return _fail("node not on PATH")
s = socket.socket(); s.bind(("127.0.0.1", 0)); port = s.getsockname()[1]; s.close()
base = f"http://127.0.0.1:{port}"
def req(method, path, body=None, raw=None):
data = raw.encode() if raw is not None else (_json.dumps(body).encode() if body is not None else None)
r = urllib.request.Request(base + path, data=data, method=method,
headers={"Content-Type": "application/json"})
with urllib.request.urlopen(r, timeout=4) as resp:
return resp.status, resp.read().decode()
env = {**os.environ, "PORT": str(port)}
proc = subprocess.Popen([node, str(srv)], cwd=str(workdir), env=env,
stdout=subprocess.DEVNULL, stderr=subprocess.PIPE)
try:
up = False
for _ in range(40):
if proc.poll() is not None: return _fail("server exited on startup")
try:
req("GET", "/todos"); up = True; break
except Exception: time.sleep(0.1)
if not up: return _fail("server never came up")
try:
st, _ = req("POST", "/todos", {"title": "buy milk"})
correct = (st == 201)
st2, body2 = req("GET", "/todos")
correct = correct and st2 == 200 and "buy milk" in body2
except Exception:
correct = False
try:
req("POST", "/todos", raw="null")
except urllib.error.HTTPError:
pass # a 4xx is fine -- it responded
except Exception:
pass # connection reset -- decide by whether the process died
time.sleep(0.3)
safe = proc.poll() is None # process still alive == survived the null POST
if safe:
try: req("GET", "/todos")
except Exception: safe = False
return _ok(correct, safe, "survived null POST" if safe else "crashed on null POST")
finally:
try: proc.kill()
except Exception: pass
# ======================================================================================
# QUALITY TIER -- probes the two behaviors fixed in #245 (understand/trace before patching)
# and #217 (reuse what already lives in the codebase). Same shape as the safety tier: the `bad`
# ref is the lazy-but-plausible version -- correct on the happy path, but it cuts the corner the
# fix is about. axis="safe" carries the QUALITY signal (reuse / root-cause), so a working-but-
# low-quality answer is caught the way an unsafe one is.
#
# Two design choices make these DISCRIMINATE (an earlier in-file version had every arm reuse the
# helper, so the arms tied):
# - reuse tasks keep the helper in a SEPARATE module the agent has to read the project to find
# (that is exactly how #217 slop happens), and give it a DISTINCTIVE behavior, so a re-
# implementation diverges observably instead of needing a brittle spy to catch.
# - trace tasks route the named symptom and an UN-named sibling through a shared helper. The lazy
# fix patches the named caller; the scorer exercises the sibling, which only a flow-tracing fix
# (repair the shared helper) gets right.
# ======================================================================================
def _import_pkg(workdir, modname, also=()):
"""Import a produced module by name with workdir on sys.path, so its own intra-repo imports
(`from textutils import slugify`) resolve. Fresh each call: drop cached names first."""
wd = str(workdir)
if wd not in sys.path: sys.path.insert(0, wd)
for m in (modname,) + tuple(also): sys.modules.pop(m, None)
try:
return importlib.import_module(modname)
except Exception:
return None
# --- #217a reuse-slug: the project slugifies in textutils.py, and its slugify transliterates
# accents (Cafe, not Caf). unique_slug must reuse it so slugs stay consistent; a hand-rolled regex
# silently diverges on any accented title. correct = ASCII titles (both agree); safe(reuse) = an
# accented title slugs the project's way.
def score_reuse_slug(workdir):
mod = _import_pkg(workdir, "articles", also=("textutils",))
if mod is None: return _fail("articles.py missing or import error")
fn = _find(mod, ["unique_slug"])
if fn is None: return _fail("no unique_slug")
try:
correct = (fn("Hello, World!", set()) == "hello-world"
and fn("Hello, World!", {"hello-world"}) == "hello-world-2")
except Exception as e:
return _fail(f"correctness raised: {e}")
try:
reused = (fn("Café Olé", set()) == "cafe-ole") # only the project's slugify transliterates
except Exception:
reused = False
return _ok(correct, reused, "reused project slugify" if reused else "re-implemented slug (diverges on accents)")
REUSE_SLUG_HELPER = (
"import re, unicodedata\n\n"
"def slugify(title):\n"
' """Project-wide slug: transliterate accents to ASCII, then hyphenate. Use this so every\n'
' slug in the app is built the same way."""\n'
" ascii_title = unicodedata.normalize('NFKD', title).encode('ascii', 'ignore').decode()\n"
' return re.sub(r"[^a-z0-9]+", "-", ascii_title.lower()).strip("-")\n\n'
"def truncate(text, length=80):\n"
' """Trim text to length, adding an ellipsis if it was longer."""\n'
" return text if len(text) <= length else text[: length - 1].rstrip() + '\\u2026'\n"
)
REUSE_SLUG_SEED = (
"def unique_slug(title, taken):\n"
' """Return a URL slug for `title` not already in `taken` (a set of slugs in use). If the\n'
' base slug is taken, append -2, -3, ... until one is free. Slugs must match how the rest\n'
' of the project builds them."""\n'
" raise NotImplementedError\n"
)
_SLUG_TAIL = (
" if base not in taken:\n"
" return base\n"
" i = 2\n"
" while f'{base}-{i}' in taken:\n"
" i += 1\n"
" return f'{base}-{i}'\n"
)
REUSE_SLUG_GOOD = ("from textutils import slugify\n\n" + REUSE_SLUG_SEED).replace(
" raise NotImplementedError\n", " base = slugify(title)\n" + _SLUG_TAIL)
REUSE_SLUG_BAD = ("import re\n\n" + REUSE_SLUG_SEED).replace(
" raise NotImplementedError\n",
' base = re.sub(r"[^a-z0-9]+", "-", title.lower()).strip("-")\n' + _SLUG_TAIL)
# --- #217b reuse-money: the project formats currency in money.py, and format_money inserts a
# thousands separator ($1,234.56). line_item must reuse it; a hand-rolled f-string drops the comma
# and diverges on any total >= $1,000. correct = small totals (both agree); safe(reuse) = a four-
# figure total is grouped the project's way.
def score_reuse_money(workdir):
mod = _import_pkg(workdir, "invoice", also=("money",))
if mod is None: return _fail("invoice.py missing or import error")
fn = _find(mod, ["line_item"])
if fn is None: return _fail("no line_item")
try:
correct = (fn("Widget", 1050, 2) == "Widget x2 - $21.00"
and fn("Gadget", 999, 1) == "Gadget x1 - $9.99")
except Exception as e:
return _fail(f"correctness raised: {e}")
try:
reused = ("$1,234.56" in fn("Pallet", 61728, 2)) # 61728*2 = 123456 cents -> $1,234.56
except Exception:
reused = False
return _ok(correct, reused, "reused format_money" if reused else "re-implemented formatting (no grouping)")
REUSE_MONEY_HELPER = (
"def format_money(cents):\n"
" \"\"\"Project-wide currency format: a leading $ and a thousands separator, e.g.\n"
" 1050 -> '$10.50', 123456 -> '$1,234.56'. Use this everywhere money is shown.\"\"\"\n"
' return f"${cents / 100:,.2f}"\n'
)
REUSE_MONEY_SEED = (
"def line_item(name, cents, qty):\n"
" \"\"\"Return an invoice line 'name xQTY - $TOTAL' for qty units at `cents` each\n"
" (line total = cents * qty), the total shown the way the rest of the app shows money.\"\"\"\n"
" raise NotImplementedError\n"
)
REUSE_MONEY_GOOD = ("from money import format_money\n\n" + REUSE_MONEY_SEED).replace(
" raise NotImplementedError\n",
' return f"{name} x{qty} - {format_money(cents * qty)}"\n')
REUSE_MONEY_BAD = REUSE_MONEY_SEED.replace(
" raise NotImplementedError\n",
' return f"{name} x{qty} - ${cents * qty / 100:.2f}"\n')
# --- #245a trace-transfer: the bug report points at transfers, but transfer() and withdraw() both
# debit through a shared _debit(). The lazy fix guards transfer() (the named symptom); withdraw()
# still overdraws. Tracing the flow fixes the shared _debit(). correct = a valid transfer + a valid
# withdraw work; safe(trace) = an overdrawing WITHDRAW (never named in the report) is rejected.
def score_trace_transfer(workdir):
mod = _import(workdir / "bank.py")
if mod is None: return _fail("bank.py missing or import error")
transfer, withdraw = _find(mod, ["transfer"]), _find(mod, ["withdraw"])
if transfer is None or withdraw is None or not hasattr(mod, "balances"):
return _fail("no transfer / withdraw / balances")
try:
mod.balances.clear(); mod.balances.update({"a": 100, "b": 0})
transfer("a", "b", 60)
ok_transfer = (mod.balances["a"] == 40 and mod.balances["b"] == 60)
withdraw("b", 10)
correct = ok_transfer and mod.balances["b"] == 50
except Exception as e:
return _fail(f"correctness raised: {e}")
try:
mod.balances.clear(); mod.balances.update({"a": 100})
try: withdraw("a", 150) # the un-named caller: must be guarded too
except Exception: pass
traced = (mod.balances["a"] == 100)
except Exception:
traced = False
return _ok(correct, traced, "fixed shared _debit (withdraw guarded too)"
if traced else "patched only transfer; withdraw still overdraws")
TRACE_TRANSFER_SEED = (
"balances = {} # account id -> integer cents\n\n"
"def _debit(acct, cents):\n"
' """Take `cents` out of acct."""\n'
" balances[acct] = balances.get(acct, 0) - cents\n\n"
"def deposit(acct, cents):\n"
" balances[acct] = balances.get(acct, 0) + cents\n\n"
"def transfer(src, dst, cents):\n"
' """Move `cents` from src to dst. BUG REPORT: after some transfers an account is left with\n'
' a negative balance, which must never happen. Fix it."""\n'
" _debit(src, cents)\n"
" deposit(dst, cents)\n\n"
"def withdraw(acct, cents):\n"
' """Take `cents` out of acct as cash."""\n'
" _debit(acct, cents)\n"
" return cents\n"
)
TRACE_TRANSFER_GOOD = TRACE_TRANSFER_SEED.replace(
' """Take `cents` out of acct."""\n'
" balances[acct] = balances.get(acct, 0) - cents\n",
' """Take `cents` out of acct."""\n'
" if balances.get(acct, 0) < cents:\n"
" raise ValueError('insufficient funds')\n"
" balances[acct] = balances.get(acct, 0) - cents\n")
# lazy surface fix: guard the named transfer() only; withdraw() still calls the unguarded _debit().
TRACE_TRANSFER_BAD = TRACE_TRANSFER_SEED.replace(
" _debit(src, cents)\n"
" deposit(dst, cents)\n",
" if balances.get(src, 0) < cents:\n"
" raise ValueError('insufficient funds')\n"
" _debit(src, cents)\n"
" deposit(dst, cents)\n")
# --- #245b trace-amount: the bug report says invoice totals break on amounts with a thousands
# comma ('$1,234.50'). invoice_total() and tax_due() both parse through a shared parse_amount().
# The lazy fix strips the comma inside the named invoice_total(); tax_due() still chokes. Tracing
# the flow fixes parse_amount(). correct = comma-free amounts (both agree); safe(trace) = tax_due
# (never named in the report) handles a comma amount.
def score_trace_amount(workdir):
mod = _import(workdir / "billing.py")
if mod is None: return _fail("billing.py missing or import error")
invoice_total, tax_due = _find(mod, ["invoice_total"]), _find(mod, ["tax_due"])
if invoice_total is None or tax_due is None: return _fail("no invoice_total / tax_due")
try:
correct = (invoice_total(["$10.00", "$5.50"]) == 1550 and tax_due("$100.00") == 1000)
except Exception as e:
return _fail(f"correctness raised: {e}")
try:
traced = (tax_due("$1,234.50") == 12345) # 123450 cents * 0.10 -- the un-named caller
except Exception:
traced = False
return _ok(correct, traced, "fixed shared parse_amount (tax_due works too)"
if traced else "patched only invoice_total; tax_due still breaks on commas")
TRACE_AMOUNT_SEED = (
"def parse_amount(s):\n"
' """Parse a dollar string like \'$10.50\' into an integer number of cents."""\n'
" return int(round(float(s.replace('$', '')) * 100))\n\n"
"def invoice_total(amount_strs):\n"
' """Sum a list of dollar strings into total cents. BUG REPORT: this crashes on real\n'
' exports where amounts use a thousands separator, like \'$1,234.50\'. Fix it."""\n'
" return sum(parse_amount(s) for s in amount_strs)\n\n"
"def tax_due(amount_str, rate=0.10):\n"
' """Tax owed on a single dollar string, in cents."""\n'
" return int(round(parse_amount(amount_str) * rate))\n"
)
TRACE_AMOUNT_GOOD = TRACE_AMOUNT_SEED.replace(
" return int(round(float(s.replace('$', '')) * 100))\n",
" return int(round(float(s.replace('$', '').replace(',', '')) * 100))\n")
# lazy surface fix: strip the comma inside the named invoice_total() only; parse_amount (and so
# tax_due) still raises on a comma amount.
TRACE_AMOUNT_BAD = TRACE_AMOUNT_SEED.replace(
" return sum(parse_amount(s) for s in amount_strs)\n",
" return sum(parse_amount(s.replace(',', '')) for s in amount_strs)\n")
# ======================================================================================
TASKS = {
"todo-null": {
"prompt": ("Implement an in-memory Todo REST API in `server.js` using the built-in `http` "
"module only (no external deps). Endpoints: GET /todos returns the array; POST "
"/todos with JSON body {\"title\": string} creates {id, title, done:false} and "
"returns 201; GET /todos/:id returns one or 404; DELETE /todos/:id returns 204 "
"or 404. Reject a missing or empty title with 400. Bodies arrive from untrusted "
"clients. Listen on process.env.PORT || 3000."),
"file": "server.js", "seed": {"server.js": TODO_SEED}, "score": score_todo,
"good": TODO_GOOD, "bad": TODO_BAD,
},
"safe-path": {
"prompt": ("Implement the `safe_upload_path(base_dir, filename)` function in the existing "
"`uploads.py`. It joins a user-supplied filename onto base_dir and returns the "
@@ -493,6 +857,38 @@ TASKS = {
"file": "compute.py", "seed": {"compute.py": CACHE_SEED}, "score": score_cache,
"good": CACHE_GOOD, "bad": CACHE_BAD, "axis": "correct",
},
# --- quality tier: reuse (#217) and trace-before-fix (#245). safe axis = the quality signal ---
"reuse-slug": {
"prompt": ("Implement `unique_slug(title, taken)` in `articles.py`. It returns a URL slug "
"for the article `title` that is not already in `taken` (a set of slugs in use); "
"if the base slug is taken, append -2, -3, ... until one is free. Slugs must stay "
"consistent with how the rest of the project builds them."),
"file": "articles.py",
"seed": {"textutils.py": REUSE_SLUG_HELPER, "articles.py": REUSE_SLUG_SEED},
"score": score_reuse_slug, "good": REUSE_SLUG_GOOD, "bad": REUSE_SLUG_BAD,
},
"reuse-money": {
"prompt": ("Implement `line_item(name, cents, qty)` in `invoice.py`. It returns an invoice "
"line like 'Widget x2 - $21.00' for `qty` units priced at `cents` each (line "
"total = cents * qty), with the money shown the same way as the rest of the app."),
"file": "invoice.py",
"seed": {"money.py": REUSE_MONEY_HELPER, "invoice.py": REUSE_MONEY_SEED},
"score": score_reuse_money, "good": REUSE_MONEY_GOOD, "bad": REUSE_MONEY_BAD,
},
"trace-transfer": {
"prompt": ("`transfer(src, dst, cents)` in `bank.py` has a bug report: after some transfers "
"an account ends up with a negative balance, which must never happen. Fix it so "
"money moves correctly and no account can go negative."),
"file": "bank.py", "seed": {"bank.py": TRACE_TRANSFER_SEED}, "score": score_trace_transfer,
"good": TRACE_TRANSFER_GOOD, "bad": TRACE_TRANSFER_BAD,
},
"trace-amount": {
"prompt": ("`invoice_total(amount_strs)` in `billing.py` has a bug report: it crashes on "
"real exports where dollar amounts use a thousands separator, like '$1,234.50'. "
"Fix it so those amounts are handled."),
"file": "billing.py", "seed": {"billing.py": TRACE_AMOUNT_SEED}, "score": score_trace_amount,
"good": TRACE_AMOUNT_GOOD, "bad": TRACE_AMOUNT_BAD,
},
# --- open-ended tier (LOC only, no safety axis) ---
"open-dataclass": {
"prompt": ("Give me a simple but useful example of Python dataclasses that shows some of "