diff --git a/.gitattributes b/.gitattributes index ba4815bee..c9bdb0ffd 100644 --- a/.gitattributes +++ b/.gitattributes @@ -15,3 +15,7 @@ bench/tools/fixtures/** text eol=lf # compared against, exact bytes. bench/compression/corpus.snapshot.json text eol=lf bench/compression/token-counts.json text eol=lf + +# The recall verifiers hash the fixture files to prove a session changed nothing, +# so a file's bytes must be the same on every checkout. +bench/recall/fixtures/** text eol=lf diff --git a/.github/workflows/quality-gates.yml b/.github/workflows/quality-gates.yml index ecbe834c1..e96381d9d 100644 --- a/.github/workflows/quality-gates.yml +++ b/.github/workflows/quality-gates.yml @@ -711,6 +711,27 @@ jobs: # `tiktoken` and `dist/`; `bench/tools/reduction` needs `tiktoken`. They are # instrument checks and not README checks, so they are run as their own step -- # a failure names which half of the job went red without reading the log. + # THE RECALL BENCHMARK'S VERIFIERS, EXERCISED WITHOUT RUNNING A SESSION. A + # verifier that is wrong is wrong silently -- it scores real, billed sessions + # and nobody re-reads the replies -- so every one is run here against answers + # whose score is known first. Standard library only, so no install. + recall-selftest: + name: Recall Benchmark Self-Test + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.13' + + - name: Run the recall verifiers against known answers + run: python3 -m unittest discover -s bench/recall -p "test_*.py" -v + bench-proof: name: Proof Harness, Instruments That Need a Build, README Figures runs-on: ubuntu-latest diff --git a/bench/recall/README.md b/bench/recall/README.md new file mode 100644 index 000000000..df5526ff8 --- /dev/null +++ b/bench/recall/README.md @@ -0,0 +1,63 @@ +# bench/recall: calls avoided across sessions + +Does what one session works out save the next session tool calls? Every +single-session benchmark, THOL included, is blind to this by construction: +each run starts from nothing. This one runs sessions in pairs. + +| half | asks | +| -------- | --------------------------------------------------------------- | +| `derive` | work something out that the workspace does not state | +| `reuse` | a fresh session, fresh workspace: a question that depends on it | + +Three pairs, each a real defect in a small standard-library Python project: + +| pair | fixture | derived | reused | +| ---------------- | ---------- | ---------------------------------------- | ----------------------------------------- | +| `recall-tz` | `tzbook` | day buckets use the machine's local time | the variable CI sets to make it pass | +| `recall-quota` | `quotasvc` | a window documented in s is read in ms | the value that gives a 90-second window | +| `recall-migrate` | `migrun` | migrations sort as strings | which migration a fresh database runs first | + +The tasks are THOL tasks (`prompt.md`, `task.json`, `verify.py`) and run on the +rig in `bench/thol`. Each arm, pair and repetition is one unit: every attempt +at it gets a new directory with an empty graph and its own THOL results +volume, shared by that unit's two sessions only. **Calls avoided** is the +control arm's reuse tool calls minus the arm's, counted only for repetitions in +which both answered correctly: a cheap wrong answer is not a saving. + +**The truth never enters a session's container.** THOL runs the agent and its +verifier as one user on one filesystem, so a mounted truth file is one the +agent could read. None is mounted: in the container the verifier records a +deferred 0, workspaces are kept, and `score` re-runs every verifier on the host. + +## Verifiers + +Scored from ground truth in `fixtures/truth/`, which the agent never sees. A +verifier awards nothing for an empty workspace, a fixture file changed, removed +or added, an empty reply, a `derive` explanation that states the cause in the +right words but denies it, or a `reuse` answer hedged with a wrong one. + + python3 -m unittest discover -s bench/recall -p "test_*.py" + +runs every verifier against answers whose score is known in advance, checks that +no workspace file or prompt states a pair's conclusion, and checks that each +fixture really carries its defect. It runs no session and spends nothing; CI +runs it on every pull request. + +## Running it + + python3 bench/recall/recall.py plan out/recall --reps 3 + python3 bench/recall/recall.py run out/recall --approve-spend + python3 bench/recall/recall.py score out/recall + python3 bench/recall/recall.py report out/recall + +`plan` needs a new directory and prints every docker command; it spends +nothing. `run` refuses without `--approve-spend`, because every session is a +real, billed Claude Code session. Re-running `run` after a failure skips every +finished unit and restarts the failed one from an empty graph. `--campaign` is +THOL's Claude Code version pin, not a label. +The default arms are `control` and `token-optimizer-proxy-knowledge`, whose +manifest points the graph at `/proxy-graph`. + +Changing a fixture? Re-hash it, or every verifier will score a real session 0: + + python3 bench/recall/hash_fixtures.py \ No newline at end of file diff --git a/bench/recall/fixtures/migrun/README.md b/bench/recall/fixtures/migrun/README.md new file mode 100644 index 000000000..790ed1174 --- /dev/null +++ b/bench/recall/fixtures/migrun/README.md @@ -0,0 +1,5 @@ +# migrun + +Applies the SQL files in `migrations/` to a SQLite database, in order. + + python3 -m unittest discover -s tests -t . \ No newline at end of file diff --git a/bench/recall/fixtures/migrun/migrations/10_backfill_email.sql b/bench/recall/fixtures/migrun/migrations/10_backfill_email.sql new file mode 100644 index 000000000..3ad186673 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/10_backfill_email.sql @@ -0,0 +1 @@ +UPDATE users SET email = name || '@example.invalid' WHERE email IS NULL; diff --git a/bench/recall/fixtures/migrun/migrations/1_create_users.sql b/bench/recall/fixtures/migrun/migrations/1_create_users.sql new file mode 100644 index 000000000..15400af86 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/1_create_users.sql @@ -0,0 +1 @@ +CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT NOT NULL); diff --git a/bench/recall/fixtures/migrun/migrations/2_add_email.sql b/bench/recall/fixtures/migrun/migrations/2_add_email.sql new file mode 100644 index 000000000..79fc1d861 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/2_add_email.sql @@ -0,0 +1 @@ +ALTER TABLE users ADD COLUMN email TEXT; diff --git a/bench/recall/fixtures/migrun/migrations/3_create_orders.sql b/bench/recall/fixtures/migrun/migrations/3_create_orders.sql new file mode 100644 index 000000000..478369124 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/3_create_orders.sql @@ -0,0 +1 @@ +CREATE TABLE orders (id INTEGER PRIMARY KEY, user_id INTEGER NOT NULL, total INTEGER NOT NULL); diff --git a/bench/recall/fixtures/migrun/migrations/4_index_orders_user.sql b/bench/recall/fixtures/migrun/migrations/4_index_orders_user.sql new file mode 100644 index 000000000..f62240a19 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/4_index_orders_user.sql @@ -0,0 +1 @@ +CREATE INDEX orders_user ON orders (user_id); diff --git a/bench/recall/fixtures/migrun/migrations/5_create_products.sql b/bench/recall/fixtures/migrun/migrations/5_create_products.sql new file mode 100644 index 000000000..f19f3c42f --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/5_create_products.sql @@ -0,0 +1 @@ +CREATE TABLE products (id INTEGER PRIMARY KEY, title TEXT NOT NULL); diff --git a/bench/recall/fixtures/migrun/migrations/6_add_product_price.sql b/bench/recall/fixtures/migrun/migrations/6_add_product_price.sql new file mode 100644 index 000000000..de2369b51 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/6_add_product_price.sql @@ -0,0 +1 @@ +ALTER TABLE products ADD COLUMN price INTEGER NOT NULL DEFAULT 0; diff --git a/bench/recall/fixtures/migrun/migrations/7_create_order_lines.sql b/bench/recall/fixtures/migrun/migrations/7_create_order_lines.sql new file mode 100644 index 000000000..69e17460d --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/7_create_order_lines.sql @@ -0,0 +1 @@ +CREATE TABLE order_lines (order_id INTEGER NOT NULL, product_id INTEGER NOT NULL, quantity INTEGER NOT NULL); diff --git a/bench/recall/fixtures/migrun/migrations/8_seed_admin.sql b/bench/recall/fixtures/migrun/migrations/8_seed_admin.sql new file mode 100644 index 000000000..89c90055e --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/8_seed_admin.sql @@ -0,0 +1 @@ +INSERT INTO users (id, name) VALUES (1, 'admin'); diff --git a/bench/recall/fixtures/migrun/migrations/9_index_lines_order.sql b/bench/recall/fixtures/migrun/migrations/9_index_lines_order.sql new file mode 100644 index 000000000..726f68ce0 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrations/9_index_lines_order.sql @@ -0,0 +1 @@ +CREATE INDEX lines_order ON order_lines (order_id); diff --git a/bench/recall/fixtures/migrun/migrun/__init__.py b/bench/recall/fixtures/migrun/migrun/__init__.py new file mode 100644 index 000000000..5a5d070ed --- /dev/null +++ b/bench/recall/fixtures/migrun/migrun/__init__.py @@ -0,0 +1 @@ +"""A small SQL migration runner.""" \ No newline at end of file diff --git a/bench/recall/fixtures/migrun/migrun/runner.py b/bench/recall/fixtures/migrun/migrun/runner.py new file mode 100644 index 000000000..b87baf169 --- /dev/null +++ b/bench/recall/fixtures/migrun/migrun/runner.py @@ -0,0 +1,20 @@ +"""Apply every migration in a directory to a database, in order.""" + +import os +import sqlite3 + + +def pending(directory): + """The migration files to apply, in the order they are applied.""" + return sorted(name for name in os.listdir(directory) if name.endswith(".sql")) + + +def migrate(database, directory): + """Apply each pending migration; return the names applied.""" + applied = [] + with sqlite3.connect(database) as connection: + for name in pending(directory): + with open(os.path.join(directory, name), encoding="utf-8") as handle: + connection.executescript(handle.read()) + applied.append(name) + return applied \ No newline at end of file diff --git a/bench/recall/fixtures/migrun/tests/__init__.py b/bench/recall/fixtures/migrun/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/bench/recall/fixtures/migrun/tests/test_runner.py b/bench/recall/fixtures/migrun/tests/test_runner.py new file mode 100644 index 000000000..b45b57b0f --- /dev/null +++ b/bench/recall/fixtures/migrun/tests/test_runner.py @@ -0,0 +1,18 @@ +import os +import tempfile +import unittest + +from migrun.runner import migrate + +MIGRATIONS = os.path.join(os.path.dirname(__file__), "..", "migrations") + + +class FreshDatabaseTest(unittest.TestCase): + def test_every_migration_applies_to_a_fresh_database(self): + with tempfile.TemporaryDirectory() as directory: + applied = migrate(os.path.join(directory, "app.db"), MIGRATIONS) + self.assertEqual(len(applied), 10) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/bench/recall/fixtures/quotasvc/README.md b/bench/recall/fixtures/quotasvc/README.md new file mode 100644 index 000000000..6013d3ec3 --- /dev/null +++ b/bench/recall/fixtures/quotasvc/README.md @@ -0,0 +1,12 @@ +# quotasvc + +A fixed-window request limiter. + +## Configuration + +| variable | meaning | default | +| -------------- | ----------------------------------------- | ------- | +| `QUOTA_LIMIT` | requests allowed per window | `100` | +| `QUOTA_WINDOW` | length of the window, in seconds | `60` | + + python3 -m unittest discover -s tests -t . \ No newline at end of file diff --git a/bench/recall/fixtures/quotasvc/quotasvc/__init__.py b/bench/recall/fixtures/quotasvc/quotasvc/__init__.py new file mode 100644 index 000000000..5c9886a77 --- /dev/null +++ b/bench/recall/fixtures/quotasvc/quotasvc/__init__.py @@ -0,0 +1 @@ +"""A fixed-window request limiter.""" \ No newline at end of file diff --git a/bench/recall/fixtures/quotasvc/quotasvc/config.py b/bench/recall/fixtures/quotasvc/quotasvc/config.py new file mode 100644 index 000000000..06a83730b --- /dev/null +++ b/bench/recall/fixtures/quotasvc/quotasvc/config.py @@ -0,0 +1,14 @@ +"""Read the limiter's settings from the environment.""" + +DEFAULT_LIMIT = 100 +DEFAULT_WINDOW = 60_000 + + +def limit(env): + raw = env.get("QUOTA_LIMIT") + return int(raw) if raw else DEFAULT_LIMIT + + +def window(env): + raw = env.get("QUOTA_WINDOW") + return int(raw) if raw else DEFAULT_WINDOW \ No newline at end of file diff --git a/bench/recall/fixtures/quotasvc/quotasvc/limiter.py b/bench/recall/fixtures/quotasvc/quotasvc/limiter.py new file mode 100644 index 000000000..56ebe40e2 --- /dev/null +++ b/bench/recall/fixtures/quotasvc/quotasvc/limiter.py @@ -0,0 +1,24 @@ +"""Allow at most `limit` requests in each fixed window of `window_ms`.""" + + +class Limiter: + def __init__(self, limit, window_ms): + self.limit = limit + self.window_ms = window_ms + self._window_start = None + self._used = 0 + + def allow(self, now_ms): + if self._window_start is None or now_ms - self._window_start >= self.window_ms: + self._window_start = now_ms + self._used = 0 + if self._used < self.limit: + self._used += 1 + return True + return False + + +def from_env(env): + from . import config + + return Limiter(config.limit(env), config.window(env)) \ No newline at end of file diff --git a/bench/recall/fixtures/quotasvc/tests/__init__.py b/bench/recall/fixtures/quotasvc/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/bench/recall/fixtures/quotasvc/tests/test_limiter.py b/bench/recall/fixtures/quotasvc/tests/test_limiter.py new file mode 100644 index 000000000..31b70425d --- /dev/null +++ b/bench/recall/fixtures/quotasvc/tests/test_limiter.py @@ -0,0 +1,21 @@ +import unittest + +from quotasvc.limiter import Limiter, from_env + + +class LimiterTest(unittest.TestCase): + def test_limit_holds_inside_a_window(self): + limiter = Limiter(limit=3, window_ms=1000) + self.assertEqual([limiter.allow(t) for t in (0, 10, 20, 30)], [True, True, True, False]) + + def test_window_resets(self): + limiter = Limiter(limit=1, window_ms=1000) + self.assertEqual([limiter.allow(t) for t in (0, 500, 1000)], [True, False, True]) + + def test_defaults(self): + limiter = from_env({}) + self.assertEqual((limiter.limit, limiter.window_ms), (100, 60_000)) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/bench/recall/fixtures/truth/recall-migrate.json b/bench/recall/fixtures/truth/recall-migrate.json new file mode 100644 index 000000000..3eccd6cd6 --- /dev/null +++ b/bench/recall/fixtures/truth/recall-migrate.json @@ -0,0 +1,66 @@ +{ + "pair": "recall-migrate", + "fixture": "migrun", + "marker": "migrun/runner.py", + "derive": { + "require": [ + "lexicograph|alphabetic|as strings|as text|string (?:order|sort)|lexical|not numerically|character by character", + "\\b10_" + ], + "contradict": [ + "\\b1_create_users(?:\\.sql)?\\s+(?:is\\s+first|(?:is\\s+|will\\s+be\\s+)?(?:run|runs|applied|applies|comes|sorts|goes)\\s+first)", + "(? dict: + root = FIXTURES / fixture + return { + path.relative_to(root).as_posix(): hashlib.sha256(path.read_bytes()).hexdigest() + for path in sorted(root.rglob("*")) + if path.is_file() and "__pycache__" not in path.parts + } + + +def main(check: bool) -> int: + stale = [] + for file in sorted(TRUTH.glob("*.json")): + record = json.loads(file.read_text()) + want = hashes(record["fixture"]) + if record.get("fixture_hashes") != want: + stale.append(file.name) + if not check: + record["fixture_hashes"] = want + file.write_text(json.dumps(record, indent=2) + "\n", newline="\n") + if check and stale: + print("stale fixture hashes: " + ", ".join(stale)) + return 1 + print(("checked " if check else "wrote ") + f"{len(list(TRUTH.glob('*.json')))} truth file(s)") + return 0 + + +if __name__ == "__main__": + sys.exit(main("--check" in sys.argv[1:])) \ No newline at end of file diff --git a/bench/recall/recall.py b/bench/recall/recall.py new file mode 100644 index 000000000..94daed302 --- /dev/null +++ b/bench/recall/recall.py @@ -0,0 +1,305 @@ +"""bench/recall: does what one session works out save the next one calls? + + python3 bench/recall/recall.py plan [--arms A,B] [--reps N] + python3 bench/recall/recall.py run --approve-spend + python3 bench/recall/recall.py score + python3 bench/recall/recall.py report + +Each pair is two THOL tasks run back to back on the rig (bench/thol): `derive` +works something out that the workspace does not state, then `reuse` starts a +FRESH session in a fresh copy of the same workspace and asks a question whose +answer depends on it. The knowledge graph is the one thing that may survive +from one to the other. + +ONE UNIT = ONE PAIR, ONE ARM, ONE REPETITION, AND IT IS ATOMIC. Every attempt +at a unit gets a new directory holding a new, empty graph and its own THOL +results volume. Its own volume because THOL resumes by skipping any +(competitor, task, rep) already recorded at the same Claude Code version, and +every session here is `--reps 1`: in a shared volume the second repetition of +a task would be skipped as done. A unit is recorded as done only when both of +its sessions finished; `run` skips done units, so re-running after a failure +repeats no completed (billed) unit, and the failed one starts again from an +empty graph rather than inheriting what half of it wrote. + +THE TRUTH NEVER ENTERS A SESSION'S CONTAINER. THOL runs the agent and its +verifier as one user on one filesystem, so a mounted truth file is one the +agent can read. No truth is mounted; the in-container verifier records a +deferred 0, workspaces are kept (`--keep-workspaces`), and `score` runs each +verifier on the host against the kept workspace and answer. `report` uses +those scores only. + +THE MEASUREMENT IS CALLS AVOIDED, COUNTED ONLY WHERE BOTH ARMS ARE RIGHT. The +control arm has no graph, so its reuse session must do the work again. A wrong +reuse answer is not a cheap one, so a repetition counts toward calls avoided +only when both arms' reuse answers scored 1. + +`plan` spends nothing. `run` refuses without --approve-spend, because every +session is a real, billed Claude Code session. +""" + +import argparse +import json +import os +import shlex +import sqlite3 +import subprocess +import sys +from contextlib import closing +from pathlib import Path + +HERE = Path(__file__).resolve().parent +RIG = HERE.parent / "thol" +THOL_HOME = "/home/bench/thol" +DEFAULT_ARMS = "control,token-optimizer-proxy-knowledge" +# THOL_CAMPAIGN IS A VERSION PIN, NOT A LABEL: runner.py exits unless the +# installed Claude Code's `--version` starts with it. Same default as +# bench/thol/run-campaign.sh. +DEFAULT_CAMPAIGN = "2.1.251 (Claude Code)" +ROLES = ("derive", "reuse") + + +def pairs(): + records = ( + json.loads(p.read_text(encoding="utf-8")) + for p in (HERE / "fixtures" / "truth").glob("*.json") + ) + return sorted(records, key=lambda t: t["pair"]) + + +def session(unit, role, attempt_dir, out, plan): + """One docker invocation of the rig's campaign mode for one session.""" + task = f"{unit['pair']}-{role}" + mounts = [ + (RIG / "auth", "/auth", "ro"), + (attempt_dir / "results", "/results", "rw"), + (attempt_dir / "graph", "/proxy-graph", "rw"), + (out / "orders" / f"{unit['arm']}.json", "/arm-order.json", "ro"), + (HERE / "tasks" / "_recall.py", f"{THOL_HOME}/tasks/_recall.py", "ro"), + (HERE / "tasks" / task, f"{THOL_HOME}/tasks/{task}", "ro"), + (HERE / "fixtures" / unit["fixture"], f"{THOL_HOME}/fixtures/out/{unit['fixture']}", "ro"), + ] + command = ["docker", "run", "--rm"] + for host, inside, mode in mounts: + command += ["-v", f"{host}:{inside}:{mode}"] + command += [ + "-e", "THOL_ARM_ORDER_FILE=/arm-order.json", + "-e", f"THOL_CAMPAIGN={plan['campaign']}", + plan["image"], "campaign", + "-c", unit["arm"], "-t", task, "--reps", "1", "--keep-workspaces", + ] + return command + + +def load(out): + return json.loads((out / "plan.json").read_text(encoding="utf-8")) + + +def progress(out): + path = out / "progress.json" + if path.exists(): + return json.loads(path.read_text(encoding="utf-8")) + return {"done": {}, "attempts": {}} + + +def save_progress(out, state): + tmp = out / "progress.json.tmp" + tmp.write_text(json.dumps(state, indent=1) + "\n", encoding="utf-8") + os.replace(tmp, out / "progress.json") + + +def plan(args): + out = Path(args.out).resolve() + # A NEW DIRECTORY, ALWAYS. Re-planning into an old one would hand the next + # derive session whatever graph the last plan's sessions wrote. + if out.exists() and any(out.iterdir()): + sys.exit(f"recall: {out} is not empty; plan into a new directory") + arms = args.arms.split(",") + if len(set(arms)) != len(arms) or "control" not in arms: + sys.exit("recall: arms must be distinct and include control, which calls avoided is measured against") + (out / "orders").mkdir(parents=True) + for arm in arms: + (out / "orders" / f"{arm}.json").write_text( + json.dumps({"arms": [arm], "orders": [[arm]]}) + "\n", encoding="utf-8" + ) + units = [ + {"id": f"{p['pair']}-{arm}-rep{rep}", "pair": p["pair"], "fixture": p["fixture"], "arm": arm, "rep": rep} + for rep in range(1, args.reps + 1) + for p in pairs() + for arm in arms + ] + record = {"campaign": args.campaign, "image": args.image, "units": units} + (out / "plan.json").write_text(json.dumps(record, indent=1) + "\n", encoding="utf-8") + for unit in units: + for role in ROLES: + print(shlex.join(str(part) for part in session(unit, role, out / "units" / unit["id"] / "try1", out, record))) + print(f"\n{len(units)} unit(s), {2 * len(units)} session(s) planned in {out}; nothing has run and nothing was spent.") + + +def run(args, execute=subprocess.run): + if not args.approve_spend: + sys.exit("recall: refusing to run without --approve-spend; every step is a real, billed Claude Code session") + out = Path(args.out).resolve() + record = load(out) + state = progress(out) + for n, unit in enumerate(record["units"], 1): + if unit["id"] in state["done"]: + continue + attempt = state["attempts"].get(unit["id"], 0) + 1 + state["attempts"][unit["id"]] = attempt + save_progress(out, state) + attempt_dir = out / "units" / unit["id"] / f"try{attempt}" + (attempt_dir / "graph").mkdir(parents=True) + (attempt_dir / "results").mkdir() + for role in ROLES: + print(f"[{n}/{len(record['units'])}] {unit['id']} {role} (attempt {attempt})", flush=True) + if execute(session(unit, role, attempt_dir, out, record)).returncode != 0: + sys.exit( + f"recall: {unit['id']} {role} failed. Completed units are recorded; run again " + "to resume, and this unit restarts from an empty graph." + ) + # A ZERO EXIT IS NOT A RECORDED SESSION. THOL exits 0 after a run it + # recorded as a timeout or a claude_error too, and `score` reads only + # `ok` rows, so a unit marked done here with a role missing would be + # scored on one session and never retried. + missing = [role for role in ROLES if role not in ok_roles(attempt_dir, unit["pair"])] + if missing: + sys.exit( + f"recall: {unit['id']} has no ok run for {', '.join(missing)}. Completed units are " + "recorded; run again to resume, and this unit restarts from an empty graph." + ) + state["done"][unit["id"]] = attempt_dir.relative_to(out).as_posix() + save_progress(out, state) + + +def ok_roles(attempt_dir, pair): + """The roles of `pair` that THOL recorded as ok in this attempt's volume.""" + db_path = attempt_dir / "results" / "results.sqlite" + if not db_path.is_file(): + return set() + with closing(sqlite3.connect(db_path)) as db: + try: + tasks = {t for (t,) in db.execute("SELECT task FROM runs WHERE status = 'ok'")} + except sqlite3.OperationalError: + return set() + return {role for role in ROLES if f"{pair}-{role}" in tasks} + + +def sessions_of(out, unit_dir): + """THOL's rows for one attempt, with where each run's workspace and answer are.""" + results = out / unit_dir / "results" + with closing(sqlite3.connect(results / "results.sqlite")) as db: + rows = db.execute( + "SELECT run_id, task, tool_calls, num_turns, total_cost_usd FROM runs WHERE status = 'ok'" + ).fetchall() + for run_id, task, calls, turns, cost in rows: + yield { + "run_id": run_id, "task": task, "calls": calls, "turns": turns, "cost": cost, + "workspace": results / "runs" / run_id / "workspace", + "answer": results / "artifacts" / run_id / "answer.txt", + } + + +class ScoringError(RuntimeError): + """A session that cannot be scored, which is a setup error and never a 0.""" + + +def verify_on_host(task, workspace, answer): + """Run a task's verifier against the real truth, outside any session container. + + Every way this can go wrong raises: a missing workspace, answer or truth + file, a verifier that crashed or printed no score, and a deferred score -- + the in-container result, which on the host can only mean the truth was not + found. Recording any of them as a 0 would count a broken setup as a wrong + answer. + """ + truth_dir = HERE / "fixtures" / "truth" + pair = task.rsplit("-", 1)[0] + for path, what in ((workspace, "workspace"), (answer, "answer"), (truth_dir / f"{pair}.json", "truth")): + if not path.exists(): + raise ScoringError(f"{task}: no {what} at {path}") + env = {**os.environ, "WORKSPACE": str(workspace), "ANSWER_FILE": str(answer), "TRUTH_DIR": str(truth_dir)} + result = subprocess.run([sys.executable, str(HERE / "tasks" / task / "verify.py")], + env=env, capture_output=True, text=True, timeout=600) + if result.returncode != 0: + raise ScoringError(f"{task}: the verifier exited {result.returncode}: {result.stderr.strip()[-400:]}") + if "detail=deferred" in result.stdout: + raise ScoringError(f"{task}: the verifier deferred on the host, so it did not find {truth_dir}") + line = next((l for l in result.stdout.splitlines() if l.startswith("score=")), None) + if line is None: + raise ScoringError(f"{task}: the verifier printed no score") + return float(line.split("=", 1)[1]) + + +def score(args): + out = Path(args.out).resolve() + record = load(out) + state = progress(out) + # A rescore replaces the scores whole: if it fails partway, report must + # find nothing rather than the scores of an earlier, smaller run. + (out / "scores.json").unlink(missing_ok=True) + scores = [] + for unit in record["units"]: + unit_dir = state["done"].get(unit["id"]) + if unit_dir is None: + continue + for s in sessions_of(out, unit_dir): + role = s["task"].rsplit("-", 1)[1] + try: + value = verify_on_host(s["task"], s["workspace"], s["answer"]) + except ScoringError as error: + sys.exit(f"recall: cannot score {unit['id']}: {error}; no scores.json is left behind") + scores.append({ + "unit": unit["id"], "pair": unit["pair"], "arm": unit["arm"], "rep": unit["rep"], + "role": role, "calls": s["calls"], "turns": s["turns"], "cost": s["cost"], + "score": value, + }) + (out / "scores.json").write_text(json.dumps(scores, indent=1) + "\n", encoding="utf-8") + print(f"scored {len(scores)} session(s) from {len(state['done'])} completed unit(s)") + + +def report(args): + out = Path(args.out).resolve() + if not (out / "scores.json").is_file(): + sys.exit(f"recall: no scores.json in {out}; run score first") + rows = [r for r in json.loads((out / "scores.json").read_text(encoding="utf-8")) if r["role"] == "reuse"] + by = {} + for r in rows: + by.setdefault(r["pair"], {}).setdefault(r["arm"], {})[r["rep"]] = r + print("pair arm right calls turns cost avoided") + for pair in sorted(by): + control = by[pair].get("control", {}) + for arm in sorted(by[pair]): + runs = by[pair][arm] + both = [k for k in runs if k in control and runs[k]["score"] == 1 and control[k]["score"] == 1] + avoided = ( + sum(control[k]["calls"] - runs[k]["calls"] for k in both) / len(both) + if both and arm != "control" else None + ) + right = sum(1 for r in runs.values() if r["score"] == 1) + mean = lambda key: sum(r[key] or 0 for r in runs.values()) / len(runs) + print( + f"{pair:17} {arm:33} {right}/{len(runs):<4} {mean('calls'):6.1f} {mean('turns'):6.1f} " + f"{mean('cost'):6.3f} {'-' if avoided is None else f'{avoided:.1f} over {len(both)}'}" + ) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + sub = parser.add_subparsers(dest="command", required=True) + p = sub.add_parser("plan") + p.add_argument("out") + p.add_argument("--arms", default=DEFAULT_ARMS) + p.add_argument("--reps", type=int, default=3) + p.add_argument("--image", default="thol-rig:local") + p.add_argument("--campaign", default=DEFAULT_CAMPAIGN) + r = sub.add_parser("run") + r.add_argument("out") + r.add_argument("--approve-spend", action="store_true") + for name in ("score", "report"): + sub.add_parser(name).add_argument("out") + args = parser.parse_args() + {"plan": plan, "run": run, "score": score, "report": report}[args.command](args) + + +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/bench/recall/tasks/_recall.py b/bench/recall/tasks/_recall.py new file mode 100644 index 000000000..536db01ba --- /dev/null +++ b/bench/recall/tasks/_recall.py @@ -0,0 +1,91 @@ +"""What every recall verifier does, so the six of them cannot disagree. + +A recall task is a PAIR. Session 1 (`derive`) is asked to work something out +that the workspace does not state. Session 2 (`reuse`) starts cold, in a fresh +copy of the same workspace, and is asked a question whose answer depends on +that conclusion. The pair measures whether what session 1 established reaches +session 2 -- the calls it avoids are the measurement, and a correct answer is +the precondition for counting them. + +THE ANSWER IS SCORED FROM GROUND TRUTH THE AGENT NEVER SEES (TRUTH_DIR), and a +verifier awards nothing unless three things hold first: + + - the workspace is the fixture: an empty or missing workspace scores 0, so a + session that never ran cannot pass on a lucky reply; + - no fixture file was changed, removed or ADDED: both prompts say not to, + and a session that "answers" by rewriting the code has answered a + different question. An added file counts, because one can change what the + code does without touching a listed file -- `migrations/0_x.sql` runs + before every other migration. Bytecode caches and dot-directories (a + client's own state, such as `.token-optimizer/`) are not the fixture; + - there is a reply at all. + +A `reuse` answer that names the right value AND a decoy is a hedge, and scores +0: a list of candidates is not a conclusion. A `derive` answer must state the +pair's conclusion (every `require` pattern) and must not deny it (no +`contradict` pattern): the right words in a sentence that says the opposite +are not the right answer. + +THE TRUTH NEVER ENTERS THE SESSION'S CONTAINER. THOL runs the agent and the +verifier as one user on one filesystem, so a truth file mounted for the +verifier is a file the agent can `cat`. recall.py mounts none; inside the rig +this verifier finds no truth and records a deferred 0, and `recall.py score` +runs it again on the host against the kept workspace and answer, which is the +score the report uses. +""" + +import re + +from _vlib import TRUTH_DIR, WS, answer, emit, sha, truth + +IGNORED_DIRS = {"__pycache__"} + + +def inventory(root): + """Every fixture-relevant file under root, as posix relative paths.""" + files = set() + for path in root.rglob("*"): + rel = path.relative_to(root) + if not path.is_file() or any(p in IGNORED_DIRS or p.startswith(".") for p in rel.parts[:-1]): + continue + if path.suffix == ".pyc": + continue + files.add(rel.as_posix()) + return files + + +def fixture_intact(root, expected): + """True when the workspace holds exactly the fixture's files, unchanged.""" + if inventory(root) != set(expected): + return False + return all(sha(root / rel) == digest for rel, digest in expected.items()) + + +def verify(pair: str, role: str) -> None: + if not (TRUTH_DIR / f"{pair}.json").is_file(): + emit(0.0, "deferred: scored outside the session container by bench/recall/recall.py score") + t = truth(f"{pair}.json") + if not (WS / t["marker"]).is_file(): + emit(0.0, f"the workspace is not the {t['fixture']} fixture (no {t['marker']})") + if not fixture_intact(WS, t["fixture_hashes"]): + emit(0.0, "a fixture file was changed, removed or added; the prompt says to change none") + text = answer().strip() + if not text: + emit(0.0, "no final reply") + if role == "derive": + missing = [p for p in t["derive"]["require"] if not re.search(p, text, re.IGNORECASE)] + if missing: + emit(0.0, "the explanation does not state: " + "; ".join(missing)) + denied = [p for p in t["derive"]["contradict"] if re.search(p, text, re.IGNORECASE)] + if denied: + emit(0.0, "the explanation contradicts the conclusion: " + "; ".join(denied)) + emit(1.0, "the root cause is stated") + if role == "reuse": + right = any(re.search(p, text, re.IGNORECASE) for p in t["reuse"]["accept"]) + hedge = [p for p in t["reuse"]["decoys"] if re.search(p, text, re.IGNORECASE)] + if right and not hedge: + emit(1.0, "the answer depends on the derived conclusion and is correct") + if right: + emit(0.0, "the right answer was given alongside a wrong one: " + ", ".join(hedge)) + emit(0.0, "not the answer the derived conclusion implies") + emit(0.0, f"unknown role {role!r}") \ No newline at end of file diff --git a/bench/recall/tasks/_vlib.py b/bench/recall/tasks/_vlib.py new file mode 100644 index 000000000..9f3238102 --- /dev/null +++ b/bench/recall/tasks/_vlib.py @@ -0,0 +1,50 @@ +"""The THOL verifier protocol, for running these verifiers outside the rig. + +THOL (github.com/pi-infected/token-harness-optimizer-leaderboard, MIT) runs a +task's verifier as `python3 verify.py` with three environment variables: + + WORKSPACE - the run workspace, after the session + ANSWER_FILE - a file holding the agent's final reply + TRUTH_DIR - fixtures/truth, which the agent never sees + +and reads `score=<0..1>` from its output. Inside the rig the verifiers import +THOL's own `_vlib.py`, which sits beside every task there; this file is never +mounted into the rig. It exists so `test_recall.py` can run every verifier +offline against the same contract, and it implements only the part of that +contract these verifiers use. THOL's own hashes_intact is deliberately not +mirrored: it checks only the files it is given, so a file ADDED to the +workspace passes it; _recall.py checks the whole inventory instead. +""" + +import hashlib +import json +import os +import sys +from pathlib import Path + +WS = Path(os.environ["WORKSPACE"]) +TRUTH_DIR = Path(os.environ["TRUTH_DIR"]) +ANSWER_FILE = os.environ.get("ANSWER_FILE", "") + + +def answer() -> str: + try: + return Path(ANSWER_FILE).read_text() + except OSError: + return "" + + +def truth(name: str): + return json.loads((TRUTH_DIR / name).read_text()) + + +def emit(score: float, detail: str = ""): + print(f"score={max(0.0, min(1.0, score)):.4f}") + if detail: + for line in detail.splitlines(): + print(f"detail={line}") + sys.exit(0) + + +def sha(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() diff --git a/bench/recall/tasks/recall-migrate-derive/prompt.md b/bench/recall/tasks/recall-migrate-derive/prompt.md new file mode 100644 index 000000000..50dd43e29 --- /dev/null +++ b/bench/recall/tasks/recall-migrate-derive/prompt.md @@ -0,0 +1,6 @@ +Applying the migrations in the project in the current directory to a fresh +database fails (`python3 -m unittest discover -s tests -t .` shows it). + +Find the root cause. Do not change any file. + +In your final reply, state the root cause in one or two sentences. \ No newline at end of file diff --git a/bench/recall/tasks/recall-migrate-derive/task.json b/bench/recall/tasks/recall-migrate-derive/task.json new file mode 100644 index 000000000..d5646da42 --- /dev/null +++ b/bench/recall/tasks/recall-migrate-derive/task.json @@ -0,0 +1 @@ +{"id":"recall-migrate-derive","title":"recall: migration order (derive)","kind":"code","workspace":{"fixture":"migrun"},"max_turns":30,"timeout_s":900,"success_threshold":1.0} diff --git a/bench/recall/tasks/recall-migrate-derive/verify.py b/bench/recall/tasks/recall-migrate-derive/verify.py new file mode 100644 index 000000000..d5ee4a808 --- /dev/null +++ b/bench/recall/tasks/recall-migrate-derive/verify.py @@ -0,0 +1,7 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from _recall import verify # noqa: E402 + +verify("recall-migrate", "derive") \ No newline at end of file diff --git a/bench/recall/tasks/recall-migrate-reuse/prompt.md b/bench/recall/tasks/recall-migrate-reuse/prompt.md new file mode 100644 index 000000000..4c2ffb820 --- /dev/null +++ b/bench/recall/tasks/recall-migrate-reuse/prompt.md @@ -0,0 +1,5 @@ +With the project in the current directory exactly as it is now, and with no +file changed, a fresh database is migrated. + +Reply with the file name of the migration that is applied first, and nothing +else. \ No newline at end of file diff --git a/bench/recall/tasks/recall-migrate-reuse/task.json b/bench/recall/tasks/recall-migrate-reuse/task.json new file mode 100644 index 000000000..ba21661f8 --- /dev/null +++ b/bench/recall/tasks/recall-migrate-reuse/task.json @@ -0,0 +1 @@ +{"id":"recall-migrate-reuse","title":"recall: migration order (reuse)","kind":"code","workspace":{"fixture":"migrun"},"max_turns":30,"timeout_s":900,"success_threshold":1.0} diff --git a/bench/recall/tasks/recall-migrate-reuse/verify.py b/bench/recall/tasks/recall-migrate-reuse/verify.py new file mode 100644 index 000000000..6dc68a0f0 --- /dev/null +++ b/bench/recall/tasks/recall-migrate-reuse/verify.py @@ -0,0 +1,7 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from _recall import verify # noqa: E402 + +verify("recall-migrate", "reuse") \ No newline at end of file diff --git a/bench/recall/tasks/recall-quota-derive/prompt.md b/bench/recall/tasks/recall-quota-derive/prompt.md new file mode 100644 index 000000000..2abacd859 --- /dev/null +++ b/bench/recall/tasks/recall-quota-derive/prompt.md @@ -0,0 +1,7 @@ +Operators of the service in the current directory set `QUOTA_WINDOW=60`, +expecting a one-minute window, and clients get roughly a thousand times more +requests through than `QUOTA_LIMIT` allows. + +Find the root cause. Do not change any file. + +In your final reply, state the root cause in one or two sentences. \ No newline at end of file diff --git a/bench/recall/tasks/recall-quota-derive/task.json b/bench/recall/tasks/recall-quota-derive/task.json new file mode 100644 index 000000000..f3e172d23 --- /dev/null +++ b/bench/recall/tasks/recall-quota-derive/task.json @@ -0,0 +1 @@ +{"id":"recall-quota-derive","title":"recall: request window units (derive)","kind":"code","workspace":{"fixture":"quotasvc"},"max_turns":30,"timeout_s":900,"success_threshold":1.0} diff --git a/bench/recall/tasks/recall-quota-derive/verify.py b/bench/recall/tasks/recall-quota-derive/verify.py new file mode 100644 index 000000000..10f269c57 --- /dev/null +++ b/bench/recall/tasks/recall-quota-derive/verify.py @@ -0,0 +1,7 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from _recall import verify # noqa: E402 + +verify("recall-quota", "derive") \ No newline at end of file diff --git a/bench/recall/tasks/recall-quota-reuse/prompt.md b/bench/recall/tasks/recall-quota-reuse/prompt.md new file mode 100644 index 000000000..e876defbf --- /dev/null +++ b/bench/recall/tasks/recall-quota-reuse/prompt.md @@ -0,0 +1,4 @@ +On the service in the current directory, exactly as it is now and with no file +changed, an operator wants each request window to last 90 seconds. + +Reply with the exact value they must give `QUOTA_WINDOW`, and nothing else. \ No newline at end of file diff --git a/bench/recall/tasks/recall-quota-reuse/task.json b/bench/recall/tasks/recall-quota-reuse/task.json new file mode 100644 index 000000000..122b1ddda --- /dev/null +++ b/bench/recall/tasks/recall-quota-reuse/task.json @@ -0,0 +1 @@ +{"id":"recall-quota-reuse","title":"recall: request window units (reuse)","kind":"code","workspace":{"fixture":"quotasvc"},"max_turns":30,"timeout_s":900,"success_threshold":1.0} diff --git a/bench/recall/tasks/recall-quota-reuse/verify.py b/bench/recall/tasks/recall-quota-reuse/verify.py new file mode 100644 index 000000000..244d0be2b --- /dev/null +++ b/bench/recall/tasks/recall-quota-reuse/verify.py @@ -0,0 +1,7 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from _recall import verify # noqa: E402 + +verify("recall-quota", "reuse") \ No newline at end of file diff --git a/bench/recall/tasks/recall-tz-derive/prompt.md b/bench/recall/tasks/recall-tz-derive/prompt.md new file mode 100644 index 000000000..3dee7b15a --- /dev/null +++ b/bench/recall/tasks/recall-tz-derive/prompt.md @@ -0,0 +1,7 @@ +The test suite of the Python project in the current directory +(`python3 -m unittest discover -s tests -t .`) passes on some machines and +fails on others, with exactly the same code and the same Python version. + +Find the root cause. Do not change any file. + +In your final reply, state the root cause in one or two sentences. \ No newline at end of file diff --git a/bench/recall/tasks/recall-tz-derive/task.json b/bench/recall/tasks/recall-tz-derive/task.json new file mode 100644 index 000000000..703af9d1a --- /dev/null +++ b/bench/recall/tasks/recall-tz-derive/task.json @@ -0,0 +1 @@ +{"id":"recall-tz-derive","title":"recall: timezone-dependent day buckets (derive)","kind":"code","workspace":{"fixture":"tzbook"},"max_turns":30,"timeout_s":900,"success_threshold":1.0} diff --git a/bench/recall/tasks/recall-tz-derive/verify.py b/bench/recall/tasks/recall-tz-derive/verify.py new file mode 100644 index 000000000..39671738c --- /dev/null +++ b/bench/recall/tasks/recall-tz-derive/verify.py @@ -0,0 +1,7 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from _recall import verify # noqa: E402 + +verify("recall-tz", "derive") \ No newline at end of file diff --git a/bench/recall/tasks/recall-tz-reuse/prompt.md b/bench/recall/tasks/recall-tz-reuse/prompt.md new file mode 100644 index 000000000..a98da62b6 --- /dev/null +++ b/bench/recall/tasks/recall-tz-reuse/prompt.md @@ -0,0 +1,6 @@ +The CI machines for the Python project in the current directory must run its +test suite (`python3 -m unittest discover -s tests -t .`) so that it passes on +every machine, without any file being changed. + +Reply with the single environment variable assignment CI should set, in the +form NAME=value, and nothing else. \ No newline at end of file diff --git a/bench/recall/tasks/recall-tz-reuse/task.json b/bench/recall/tasks/recall-tz-reuse/task.json new file mode 100644 index 000000000..bc36d0d11 --- /dev/null +++ b/bench/recall/tasks/recall-tz-reuse/task.json @@ -0,0 +1 @@ +{"id":"recall-tz-reuse","title":"recall: timezone-dependent day buckets (reuse)","kind":"code","workspace":{"fixture":"tzbook"},"max_turns":30,"timeout_s":900,"success_threshold":1.0} diff --git a/bench/recall/tasks/recall-tz-reuse/verify.py b/bench/recall/tasks/recall-tz-reuse/verify.py new file mode 100644 index 000000000..c3eb9dc23 --- /dev/null +++ b/bench/recall/tasks/recall-tz-reuse/verify.py @@ -0,0 +1,7 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from _recall import verify # noqa: E402 + +verify("recall-tz", "reuse") \ No newline at end of file diff --git a/bench/recall/test_recall.py b/bench/recall/test_recall.py new file mode 100644 index 000000000..f938c42f3 --- /dev/null +++ b/bench/recall/test_recall.py @@ -0,0 +1,492 @@ +"""Offline self-test of bench/recall: no session is run and nothing is spent. + + python3 -m unittest discover -s bench/recall -p "test_*.py" + +A recall measurement is only as good as its verifiers, and a verifier that is +wrong is wrong silently: it scores real sessions, nobody re-reads the replies, +and a benchmark that passes everything or nothing looks exactly like one that +works. So every verifier is exercised here against answers whose score is +known before it runs, and every fixture is checked to carry the defect its +pair is about. +""" + +import importlib.util +import json +import os +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +HERE = Path(__file__).resolve().parent +TASKS = HERE / "tasks" +FIXTURES = HERE / "fixtures" +TRUTH = FIXTURES / "truth" +PAIRS = sorted(json.loads(p.read_text())["pair"] for p in TRUTH.glob("*.json")) + + +def record(pair): + return json.loads((TRUTH / f"{pair}.json").read_text()) + + +def score(pair, role, reply, workspace): + with tempfile.TemporaryDirectory() as scratch: + answer = Path(scratch) / "answer.txt" + answer.write_text(reply) + run = subprocess.run( + [sys.executable, str(TASKS / f"{pair}-{role}" / "verify.py")], + env={**os.environ, "WORKSPACE": str(workspace), "ANSWER_FILE": str(answer), "TRUTH_DIR": str(TRUTH)}, + capture_output=True, + text=True, + timeout=60, + ) + if run.returncode != 0: + raise AssertionError(f"verifier broke: {run.stderr}") + line = next(l for l in run.stdout.splitlines() if l.startswith("score=")) + return float(line.split("=", 1)[1]) + + +class Workspace: + """A fresh copy of a pair's fixture, as THOL hands one to each session.""" + + def __init__(self, pair): + self.fixture = record(pair)["fixture"] + + def __enter__(self): + self.tmp = tempfile.TemporaryDirectory() + self.path = Path(self.tmp.name) / "ws" + shutil.copytree(FIXTURES / self.fixture, self.path, ignore=shutil.ignore_patterns("__pycache__")) + return self.path + + def __exit__(self, *exc): + self.tmp.cleanup() + + +class TheTasksAreWellFormed(unittest.TestCase): + def test_every_pair_has_both_halves(self): + self.assertEqual(len(PAIRS), 3) + for pair in PAIRS: + for role in ("derive", "reuse"): + task = json.loads((TASKS / f"{pair}-{role}" / "task.json").read_text()) + self.assertEqual(task["id"], f"{pair}-{role}") + self.assertEqual(task["workspace"]["fixture"], record(pair)["fixture"]) + self.assertTrue((FIXTURES / task["workspace"]["fixture"]).is_dir()) + + def test_the_fixture_hashes_are_current(self): + spec = importlib.util.spec_from_file_location("hash_fixtures", HERE / "hash_fixtures.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + self.assertEqual(module.main(check=True), 0) + + +class TheConclusionIsNotInTheWorkspace(unittest.TestCase): + """The reuse answer must come from session 1 or from doing the work again.""" + + def test_no_workspace_file_or_prompt_states_it(self): + for pair in PAIRS: + t = record(pair) + texts = { + str(p.relative_to(FIXTURES)): p.read_text(encoding="utf-8").lower() + for p in (FIXTURES / t["fixture"]).rglob("*") + if p.is_file() and "__pycache__" not in p.parts + } + for role in ("derive", "reuse"): + texts[f"{pair}-{role}/prompt.md"] = (TASKS / f"{pair}-{role}" / "prompt.md").read_text(encoding="utf-8").lower() + for phrase in t["absent"]: + for name, text in texts.items(): + self.assertNotIn(phrase.lower(), text, f"{pair}: {name} states {phrase!r}") + + +class TheVerifiersScoreWhatTheySay(unittest.TestCase): + def test_the_reference_answers_score_full_credit(self): + for pair in PAIRS: + t = record(pair) + with Workspace(pair) as ws: + self.assertEqual(score(pair, "derive", t["reference"]["derive"], ws), 1.0, pair) + self.assertEqual(score(pair, "reuse", t["reference"]["reuse"], ws), 1.0, pair) + + def test_an_empty_reply_scores_nothing(self): + for pair in PAIRS: + with Workspace(pair) as ws: + for role in ("derive", "reuse"): + self.assertEqual(score(pair, role, " \n", ws), 0.0, f"{pair}-{role}") + + def test_wrong_and_hedged_answers_score_nothing(self): + for pair in PAIRS: + with Workspace(pair) as ws: + for wrong in record(pair)["decoyAnswers"]: + self.assertEqual(score(pair, "reuse", wrong, ws), 0.0, f"{pair}: {wrong!r}") + + def test_an_empty_workspace_scores_nothing(self): + for pair in PAIRS: + t = record(pair) + with tempfile.TemporaryDirectory() as empty: + for role in ("derive", "reuse"): + self.assertEqual(score(pair, role, t["reference"][role], empty), 0.0, f"{pair}-{role}") + + def test_a_changed_fixture_scores_nothing(self): + for pair in PAIRS: + t = record(pair) + with Workspace(pair) as ws: + marker = ws / t["marker"] + marker.write_text(marker.read_text() + "\n# edited\n") + for role in ("derive", "reuse"): + self.assertEqual(score(pair, role, t["reference"][role], ws), 0.0, f"{pair}-{role}") + + def test_a_derive_reply_that_denies_the_cause_in_the_right_words_scores_nothing(self): + # "Sorting is alphabetical, so 1_create_users.sql runs first" names both + # the sort and 10_ and says the opposite of what the fixture does. + for pair in PAIRS: + with Workspace(pair) as ws: + for wrong in record(pair)["deriveDecoys"]: + self.assertEqual(score(pair, "derive", wrong, ws), 0.0, f"{pair}: {wrong!r}") + + def test_an_added_fixture_file_scores_nothing(self): + # migrations/0_x.sql changes which migration runs first without + # touching any listed file, so the listed hashes alone cannot see it. + for pair in PAIRS: + t = record(pair) + with Workspace(pair) as ws: + extra = ws / Path(t["marker"]).parent / "0_added.sql" + extra.write_text("SELECT 1;\n") + for role in ("derive", "reuse"): + self.assertEqual(score(pair, role, t["reference"][role], ws), 0.0, f"{pair}-{role}") + + def test_bytecode_and_a_clients_own_state_are_not_the_fixture(self): + for pair in PAIRS: + t = record(pair) + with Workspace(pair) as ws: + (ws / "__pycache__").mkdir() + (ws / "__pycache__" / "x.cpython-313.pyc").write_bytes(b"\0") + (ws / ".token-optimizer" / "wiki").mkdir(parents=True) + (ws / ".token-optimizer" / "wiki" / "nodes.jsonl").write_text("{}\n") + self.assertEqual(score(pair, "reuse", t["reference"]["reuse"], ws), 1.0, pair) + + def test_a_right_cause_told_in_other_words_scores_full_credit(self): + # The contradiction check must reject named wrong causes only: "comes + # from migration 10 referencing a missing table." is the symptom, and + # the next sentence gives the ordering cause. + for pair in PAIRS: + with Workspace(pair) as ws: + for right in record(pair)["deriveAccepts"]: + self.assertEqual(score(pair, "derive", right, ws), 1.0, f"{pair}: {right!r}") + + def test_a_derive_reply_that_misses_the_cause_scores_nothing(self): + vague = "The tests fail because of a bug in the code; it needs fixing." + for pair in PAIRS: + with Workspace(pair) as ws: + self.assertEqual(score(pair, "derive", vague, ws), 0.0, pair) + + +class TheFixturesCarryTheirDefect(unittest.TestCase): + def test_migrations_sort_as_strings(self): + sys.path.insert(0, str(FIXTURES / "migrun")) + try: + from migrun.runner import pending + + self.assertEqual(pending(FIXTURES / "migrun" / "migrations")[0], "10_backfill_email.sql") + finally: + sys.path.remove(str(FIXTURES / "migrun")) + with Workspace("recall-migrate") as ws: + suite = subprocess.run( + [sys.executable, "-m", "unittest", "discover", "-s", "tests", "-t", "."], + cwd=ws, capture_output=True, text=True, timeout=120, + ) + self.assertNotEqual(suite.returncode, 0, "a fresh database must fail to migrate") + + def test_the_window_is_read_in_milliseconds(self): + sys.path.insert(0, str(FIXTURES / "quotasvc")) + try: + from quotasvc.limiter import from_env + + self.assertEqual(from_env({"QUOTA_WINDOW": "60"}).window_ms, 60) + finally: + sys.path.remove(str(FIXTURES / "quotasvc")) + self.assertIn("in seconds", (FIXTURES / "quotasvc" / "README.md").read_text()) + + @unittest.skipIf(not hasattr(__import__("time"), "tzset"), "time.tzset is POSIX-only") + def test_the_days_depend_on_the_local_time_zone(self): + def suite(zone): + with Workspace("recall-tz") as ws: + return subprocess.run( + [sys.executable, "-m", "unittest", "discover", "-s", "tests", "-t", "."], + cwd=ws, env={**os.environ, "TZ": zone}, capture_output=True, text=True, timeout=120, + ).returncode + + self.assertEqual(suite("UTC"), 0) + self.assertNotEqual(suite("Pacific/Honolulu"), 0) + + +def runner(): + spec = importlib.util.spec_from_file_location("recall", HERE / "recall.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def quiet(fn, *args, **kwargs): + import contextlib + import io + + with contextlib.redirect_stdout(io.StringIO()): + return fn(*args, **kwargs) + + +def make_plan(out, reps=2): + import argparse + + quiet(runner().plan, argparse.Namespace( + out=str(out), arms="control,knowledge", reps=reps, image="img", campaign="2.1.251 (Claude Code)")) + return json.loads((Path(out) / "plan.json").read_text(encoding="utf-8")) + + +def mounts_of(step): + return [v for i, v in enumerate(step) if i and step[i - 1] == "-v"] + + +class Ok: + returncode = 0 + + +class Fail: + returncode = 1 + + +def thol_records(step, status="ok"): + """Write the row THOL would for the session `step` ran, into its own volume.""" + import sqlite3 + from contextlib import closing + + results = Path(next(m for m in mounts_of(step) if ":/results:" in m).split(":/results:")[0]) + task = step[step.index("-t") + 1] + with closing(sqlite3.connect(results / "results.sqlite")) as db, db: + db.execute("CREATE TABLE IF NOT EXISTS runs (run_id TEXT, task TEXT, tool_calls INTEGER, " + "num_turns INTEGER, total_cost_usd REAL, status TEXT)") + db.execute("INSERT INTO runs VALUES (?,?,?,?,?,?)", (task, task, 1, 1, 0.0, status)) + return Ok() + + +class TheRunnerKeepsThePairsApart(unittest.TestCase): + """Nothing may reach a reuse session except what its own derive session left.""" + + def test_a_plan_is_one_unit_per_pair_arm_and_repetition(self): + with tempfile.TemporaryDirectory() as out: + plan = make_plan(Path(out) / "p") + self.assertEqual(len(plan["units"]), 3 * 2 * 2) + self.assertEqual(len({u["id"] for u in plan["units"]}), len(plan["units"])) + + def test_planning_into_a_used_directory_is_refused(self): + with tempfile.TemporaryDirectory() as out: + make_plan(Path(out) / "p") + with self.assertRaises(SystemExit) as refused: + make_plan(Path(out) / "p") + self.assertIn("not empty", str(refused.exception)) + + def test_a_session_sees_its_own_task_and_fixture_and_never_the_truth(self): + r = runner() + with tempfile.TemporaryDirectory() as out: + out = Path(out) / "p" + plan = make_plan(out, reps=1) + for unit in plan["units"]: + for role in r.ROLES: + step = r.session(unit, role, out / "units" / unit["id"] / "try1", out, plan) + mounts = mounts_of(step) + self.assertEqual([m for m in mounts if "truth" in m], [], unit["id"]) + tasks = [m for m in mounts if "/tasks/recall-" in m] + self.assertEqual(len(tasks), 1) + self.assertIn(f"/tasks/{unit['pair']}-{role}:", tasks[0]) + self.assertIn("--keep-workspaces", step) + self.assertIn("THOL_CAMPAIGN=2.1.251 (Claude Code)", step) + + def test_the_two_sessions_of_an_attempt_share_one_graph_and_results_volume(self): + r = runner() + with tempfile.TemporaryDirectory() as out: + out = Path(out) / "p" + plan = make_plan(out, reps=1) + unit = plan["units"][0] + attempt = out / "units" / unit["id"] / "try1" + steps = [r.session(unit, role, attempt, out, plan) for role in r.ROLES] + for wanted in (":/proxy-graph:", ":/results:"): + found = {next(m for m in mounts_of(s) if wanted in m) for s in steps} + self.assertEqual(len(found), 1, wanted) + + def test_running_without_approval_is_refused(self): + import argparse + + with tempfile.TemporaryDirectory() as out: + make_plan(Path(out) / "p", reps=1) + with self.assertRaises(SystemExit) as refused: + runner().run(argparse.Namespace(out=str(Path(out) / "p"), approve_spend=False)) + self.assertIn("approve-spend", str(refused.exception)) + + def test_a_rerun_repeats_no_finished_unit_and_restarts_a_failed_one_clean(self): + import argparse + + r = runner() + with tempfile.TemporaryDirectory() as out: + out = Path(out) / "p" + plan = make_plan(out, reps=1) + calls = [] + + def flaky(step): + calls.append(step) + graph = next(m for m in mounts_of(step) if ":/proxy-graph:" in m).split(":/proxy-graph:")[0] + # Each session leaves something in the graph, as a real one would. + (Path(graph) / f"left-{len(calls)}").write_text("x") + return Fail() if len(calls) == 4 else thol_records(step) + + args = argparse.Namespace(out=str(out), approve_spend=True) + with self.assertRaises(SystemExit): + quiet(r.run, args, execute=flaky) + self.assertEqual(len(calls), 4) + first = plan["units"][0]["id"] + failed = plan["units"][1]["id"] + self.assertIn(first, r.progress(out)["done"]) + self.assertNotIn(failed, r.progress(out)["done"]) + + calls.clear() + quiet(r.run, args, execute=lambda step: (calls.append(step), thol_records(step))[1]) + touched = {m for s in calls for m in mounts_of(s) if ":/results:" in m} + self.assertFalse(any(f"units{os.sep}{first}{os.sep}" in m or f"units/{first}/" in m for m in touched)) + self.assertEqual(len(calls), 2 * (len(plan["units"]) - 1)) + retry = out / "units" / failed / "try2" / "graph" + self.assertEqual(r.progress(out)["done"][failed], f"units/{failed}/try2") + self.assertTrue(retry.is_dir()) + self.assertFalse(any(p.name.startswith("left-") and int(p.name[5:]) <= 4 for p in retry.iterdir())) + + + def test_a_zero_exit_without_an_ok_row_does_not_complete_a_unit(self): + # THOL exits 0 after recording a timeout too; score reads only ok rows. + import argparse + + r = runner() + with tempfile.TemporaryDirectory() as out: + out = Path(out) / "p" + plan = make_plan(out, reps=1) + args = argparse.Namespace(out=str(out), approve_spend=True) + with self.assertRaises(SystemExit) as stopped: + quiet(r.run, args, execute=lambda step: thol_records( + step, "timeout" if step[step.index("-t") + 1].endswith("-reuse") else "ok")) + self.assertIn("no ok run for reuse", str(stopped.exception)) + self.assertEqual(r.progress(out)["done"], {}) + self.assertEqual(r.progress(out)["attempts"], {plan["units"][0]["id"]: 1}) + + +class ScoringHappensOnTheHost(unittest.TestCase): + def test_host_scoring_refuses_what_it_cannot_score(self): + r = runner() + with Workspace("recall-tz") as ws, tempfile.TemporaryDirectory() as scratch: + answer = Path(scratch) / "answer.txt" + with self.assertRaisesRegex(r.ScoringError, "no answer"): + r.verify_on_host("recall-tz-reuse", ws, answer) + answer.write_text("TZ=UTC") + with self.assertRaisesRegex(r.ScoringError, "no workspace"): + r.verify_on_host("recall-tz-reuse", Path(scratch) / "missing", answer) + with self.assertRaisesRegex(r.ScoringError, "no truth"): + r.verify_on_host("recall-absent-reuse", ws, answer) + self.assertEqual(r.verify_on_host("recall-tz-reuse", ws, answer), 1.0) + + def test_a_deferred_crashed_or_silent_verifier_is_an_error_not_a_zero(self): + # The check after the truth pre-check: a verifier that still defers on + # the host means the setup is wrong, and must not be recorded as a 0. + r = runner() + real = r.HERE + with tempfile.TemporaryDirectory() as fake, Workspace("recall-tz") as ws: + fake = Path(fake) + (fake / "tasks" / "recall-tz-reuse").mkdir(parents=True) + (fake / "fixtures" / "truth").mkdir(parents=True) + (fake / "fixtures" / "truth" / "recall-tz.json").write_text("{}") + answer = fake / "answer.txt" + answer.write_text("TZ=UTC") + stubs = { + "deferred": 'print("score=0.0000")\nprint("detail=deferred: no truth here")\n', + "exited 3": 'import sys\nsys.exit(3)\n', + "no score": 'print("detail=nothing to say")\n', + } + r.HERE = fake + try: + for expected, source in stubs.items(): + (fake / "tasks" / "recall-tz-reuse" / "verify.py").write_text(source) + with self.assertRaisesRegex(r.ScoringError, expected): + r.verify_on_host("recall-tz-reuse", ws, answer) + finally: + r.HERE = real + def test_inside_the_container_the_verifier_defers(self): + with Workspace("recall-tz") as ws, tempfile.TemporaryDirectory() as empty_truth: + with tempfile.TemporaryDirectory() as scratch: + answer = Path(scratch) / "answer.txt" + answer.write_text(record("recall-tz")["reference"]["reuse"]) + run = subprocess.run( + [sys.executable, str(TASKS / "recall-tz-reuse" / "verify.py")], + env={**os.environ, "WORKSPACE": str(ws), "ANSWER_FILE": str(answer), "TRUTH_DIR": empty_truth}, + capture_output=True, text=True, timeout=60, + ) + self.assertIn("score=0.0000", run.stdout) + self.assertIn("deferred", run.stdout) + + def test_score_reads_the_kept_workspace_and_answer_and_report_counts_both_right(self): + import argparse + import sqlite3 + from contextlib import closing + + r = runner() + with tempfile.TemporaryDirectory() as out: + out = Path(out) / "p" + plan = make_plan(out, reps=2) + replies = { # (arm, rep) -> (reuse reply, reuse tool calls) + ("control", 1): ("TZ=UTC", 12), + ("control", 2): ("TZ=UTC", 10), + ("knowledge", 1): ("TZ=UTC", 2), + ("knowledge", 2): ("LANG=C", 1), # cheap and wrong: not a saving + } + state = {"done": {}, "attempts": {}} + for unit in plan["units"]: + if unit["pair"] != "recall-tz": + continue + unit_dir = out / "units" / unit["id"] / "try1" + results = unit_dir / "results" + results.mkdir(parents=True) + with closing(sqlite3.connect(results / "results.sqlite")) as db, db: + db.execute("CREATE TABLE runs (run_id TEXT, task TEXT, tool_calls INTEGER, " + "num_turns INTEGER, total_cost_usd REAL, status TEXT)") + for role in ("derive", "reuse"): + run_id = f"{unit['id']}-{role}" + reply, calls = (record("recall-tz")["reference"]["derive"], 5) if role == "derive" \ + else replies[(unit["arm"], unit["rep"])] + shutil.copytree(FIXTURES / "tzbook", results / "runs" / run_id / "workspace", + ignore=shutil.ignore_patterns("__pycache__")) + (results / "artifacts" / run_id).mkdir(parents=True) + (results / "artifacts" / run_id / "answer.txt").write_text(reply) + db.execute("INSERT INTO runs VALUES (?,?,?,?,?,?)", + (run_id, f"recall-tz-{role}", calls, calls, 0.1, "ok")) + state["done"][unit["id"]] = unit_dir.relative_to(out).as_posix() + r.save_progress(out, state) + quiet(r.score, argparse.Namespace(out=str(out))) + scores = json.loads((out / "scores.json").read_text(encoding="utf-8")) + self.assertEqual(len(scores), 8) + self.assertTrue(all(s["score"] == 1.0 for s in scores if s["role"] == "derive")) + import contextlib + import io + + buffer = io.StringIO() + with contextlib.redirect_stdout(buffer): + r.report(argparse.Namespace(out=str(out))) + + # A rescore that fails (here: an answer went missing) must not + # leave the earlier scores for report to present as current. + first = next(Path(out, d) for d in state["done"].values()) + next((first / "results" / "artifacts").glob("*/answer.txt")).unlink() + with self.assertRaisesRegex(SystemExit, "cannot score"): + quiet(r.score, argparse.Namespace(out=str(out))) + self.assertFalse((out / "scores.json").exists()) + with self.assertRaisesRegex(SystemExit, "run score first"): + r.report(argparse.Namespace(out=str(out))) + line = next(l for l in buffer.getvalue().splitlines() if "knowledge" in l) + self.assertIn("10.0 over 1", line) + self.assertIn("1/2", line) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/hooks-core/adapter.mjs b/hooks-core/adapter.mjs index ebcbf278e..5d951d601 100644 --- a/hooks-core/adapter.mjs +++ b/hooks-core/adapter.mjs @@ -890,6 +890,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -970,16 +985,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1868,9 +1877,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/hooks-core/policy.mjs b/hooks-core/policy.mjs index 0c72a950d..64014cb5a 100644 --- a/hooks-core/policy.mjs +++ b/hooks-core/policy.mjs @@ -392,6 +392,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -485,6 +488,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -649,6 +653,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/cline/hooks/token-optimizer/lib/adapter.mjs b/integrations/cline/hooks/token-optimizer/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/cline/hooks/token-optimizer/lib/adapter.mjs +++ b/integrations/cline/hooks/token-optimizer/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/cline/hooks/token-optimizer/lib/policy.mjs b/integrations/cline/hooks/token-optimizer/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/cline/hooks/token-optimizer/lib/policy.mjs +++ b/integrations/cline/hooks/token-optimizer/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/codex/hooks/lib/adapter.mjs b/integrations/codex/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/codex/hooks/lib/adapter.mjs +++ b/integrations/codex/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/codex/hooks/lib/policy.mjs b/integrations/codex/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/codex/hooks/lib/policy.mjs +++ b/integrations/codex/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/codex/plugin/hooks/lib/adapter.mjs b/integrations/codex/plugin/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/codex/plugin/hooks/lib/adapter.mjs +++ b/integrations/codex/plugin/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/codex/plugin/hooks/lib/policy.mjs b/integrations/codex/plugin/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/codex/plugin/hooks/lib/policy.mjs +++ b/integrations/codex/plugin/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/copilot/.github/hooks/lib/adapter.mjs b/integrations/copilot/.github/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/copilot/.github/hooks/lib/adapter.mjs +++ b/integrations/copilot/.github/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/copilot/.github/hooks/lib/policy.mjs b/integrations/copilot/.github/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/copilot/.github/hooks/lib/policy.mjs +++ b/integrations/copilot/.github/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/cursor/hooks/lib/adapter.mjs b/integrations/cursor/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/cursor/hooks/lib/adapter.mjs +++ b/integrations/cursor/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/cursor/hooks/lib/policy.mjs b/integrations/cursor/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/cursor/hooks/lib/policy.mjs +++ b/integrations/cursor/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/gemini/hooks/lib/adapter.mjs b/integrations/gemini/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/gemini/hooks/lib/adapter.mjs +++ b/integrations/gemini/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/gemini/hooks/lib/policy.mjs b/integrations/gemini/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/gemini/hooks/lib/policy.mjs +++ b/integrations/gemini/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/kilo/hooks/lib/adapter.mjs b/integrations/kilo/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/kilo/hooks/lib/adapter.mjs +++ b/integrations/kilo/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/kilo/hooks/lib/policy.mjs b/integrations/kilo/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/kilo/hooks/lib/policy.mjs +++ b/integrations/kilo/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/opencode/hooks/lib/adapter.mjs b/integrations/opencode/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/opencode/hooks/lib/adapter.mjs +++ b/integrations/opencode/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/opencode/hooks/lib/policy.mjs b/integrations/opencode/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/opencode/hooks/lib/policy.mjs +++ b/integrations/opencode/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/qwen/hooks/lib/adapter.mjs b/integrations/qwen/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/qwen/hooks/lib/adapter.mjs +++ b/integrations/qwen/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/qwen/hooks/lib/policy.mjs b/integrations/qwen/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/qwen/hooks/lib/policy.mjs +++ b/integrations/qwen/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/integrations/windsurf/hooks/lib/adapter.mjs b/integrations/windsurf/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/integrations/windsurf/hooks/lib/adapter.mjs +++ b/integrations/windsurf/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/integrations/windsurf/hooks/lib/policy.mjs b/integrations/windsurf/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/integrations/windsurf/hooks/lib/policy.mjs +++ b/integrations/windsurf/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/plugin/hooks/lib/adapter.mjs b/plugin/hooks/lib/adapter.mjs index 89eea2645..e972bb51d 100644 --- a/plugin/hooks/lib/adapter.mjs +++ b/plugin/hooks/lib/adapter.mjs @@ -892,6 +892,21 @@ function gradeObservedTools(state, event, payload, raw) { } /** The session-start notice, shared verbatim so no client drifts its own copy. */ +/** Tool calls after which a session that has edited nothing still gets the recording guidance. */ +export const GUIDANCE_AFTER_CALLS = 10; + +/** + * The wiki_write guidance. It was part of the SessionStart block until #357 + * (M5): it is the largest part of that block, and it only matters once there is + * something to record, so it is now delivered once, on the first post-tool + * event after the session shows real work, and a session that never gets that + * far never pays for it. The wording is unchanged, because it was measured -- + * see the last paragraph. + */ +export function recordingGuidance() { + return `## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}`; +} + export function policyText( canDeny, availableTools = undefined, @@ -972,16 +987,10 @@ export function policyText( ] .filter(Boolean) .join(' '); - const recording = tools.has('wiki_write') - ? `\n\n## Record what you work out\n\nCall wiki_write when you establish something durable about this project, while\nyou still hold the context. Every claim needs at least one anchor -- a real file\npath, or path#symbol -- because an unanchored claim can never be checked against\nthe code again and would be served as current forever; unanchored writes are\nrefused.\n\nWorth recording: a decision and why the alternative was rejected, a failure and\nwhat actually caused it, a command that turned out to be the one that works.\nNot worth recording: what the code plainly says. Every wiki_write must include\nthe concrete evidence, when it applies, a calibrated confidence label, its\nscope, and what would invalidate it. Prefer the thing someone had to work out,\nbecause that exists nowhere in the source tree.\n\nRECORD BEFORE YOU ANSWER, NEVER AFTER. Your final message must be the answer to\nthe task -- never a confirmation that you saved a note. Measured: a run whose\nlast message was "Recorded. The finding captured: ..." scored 0.3 where the\nidentical task answered directly scored 0.9. The work had been done correctly\nand the artifact was written; only the last message was wrong, and the last\nmessage is what a verifier, a reviewer and the person who asked all read.${extractionNotice()}` - // WAS 158 CHARACTERS OF SELF-DESCRIPTION. "Structural graph capture remains - // active through lifecycle hooks. Durable semantic MCP writes are not - // requested because the writer schema is not proven available." -- true, - // and it changed no decision the model ever made with it. Fixed context is - // charged on every session; it has to earn its place by altering behaviour. - : ''; - - return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${recording}${projectBriefing()}`; + // THE RECORDING GUIDANCE IS DELIVERED LATER, NOT HERE (#357, M5): see + // recordingGuidance. A session that never does real work never pays for it. + + return `# Token optimization is active\n\n${outputDiscipline()}\n\n${connection}${routing}${enforcement}${utilities ? `\n\n${utilities}` : ''}${projectBriefing()}`; } /** @@ -1870,9 +1879,33 @@ async function runHook(clientName, event, invocation) { // has already run) either way. Say it once per session per kind -- see // adviseOnce -- rather than on every matching call. const advisory = verdict && adviseOnce(state, verdict) ? verdict.reason : null; + // THE RECORDING GUIDANCE, ONCE, AFTER REAL WORK (#357, M5). It used to ride in + // every SessionStart, where it is the largest fixed cost a small session pays + // and buys nothing until there is something to record. The first edit is the + // clearest sign there will be; GUIDANCE_AFTER_CALLS tool calls is the fallback + // for an investigation that edits nothing but still works things out. + // + // Only where it can arrive and is wanted: an experiment arm with harvest off + // gets no recording instructions (the nudge above has the same gate), and a + // client whose context style is silent (Windsurf) cannot deliver any context, + // so it must not spend the once-only flag on a message nobody receives. + const guided = + event === 'post-tool' && + !state.recordingGuided && + features.harvest && + contextOutput(client, eventName, '') !== null && + toolEvidence.names.has('wiki_write') && + ((Number(state.edits) || 0) >= 1 || + (Number(state.toolCalls) || 0) >= GUIDANCE_AFTER_CALLS); + if (guided) state.recordingGuided = true; saveState(payload.session_id, state, agentScope); - const context = [advisory, graphContext, recordingContext] + const context = [ + advisory, + graphContext, + guided ? recordingGuidance() : null, + recordingContext, + ] .filter(Boolean) .join('\n\n'); if (context) { diff --git a/plugin/hooks/lib/policy.mjs b/plugin/hooks/lib/policy.mjs index c9eb1f896..a161899d3 100644 --- a/plugin/hooks/lib/policy.mjs +++ b/plugin/hooks/lib/policy.mjs @@ -394,6 +394,9 @@ function emptyState() { editedFiles: [], harvestedEdits: 0, recordingNudged: false, + // The recording guidance has been delivered (#357, M5); see + // recordingGuidance in adapter.mjs. + recordingGuided: false, // Facts the search advisory has already delivered this session. Without // this in the default, loadState returned an object with no `advised`, the // router rebuilt an empty Set on every hook process, and the same advisory @@ -487,6 +490,7 @@ export function loadState(sessionId, agent) { ? parsed.harvestedEdits : 0, recordingNudged: parsed.recordingNudged === true, + recordingGuided: parsed.recordingGuided === true, // Every hook call is a new process, so a field left out of this list is // forgotten between calls: that is how "said once" kept being said on // every search, and how a repeated WebFetch was never recognised @@ -651,6 +655,10 @@ export function saveState(sessionId, state, agent) { recordingNudged: Boolean( current.recordingNudged || state.recordingNudged ), + // Same rule: guidance said once stays said. + recordingGuided: Boolean( + current.recordingGuided || state.recordingGuided + ), // UNION, AND CAPPED. Two hook processes can each deliver an advisory // before either writes, so last-writer-wins would forget one and repeat // it. A fact once told stays told -- that is the whole point of the set diff --git a/tests/hooks/assist-mode.test.mjs b/tests/hooks/assist-mode.test.mjs index 5a166bdc7..675d334fd 100644 --- a/tests/hooks/assist-mode.test.mjs +++ b/tests/hooks/assist-mode.test.mjs @@ -30,8 +30,10 @@ import { mode, MODE_ASSIST, MODE_ENFORCE, MODE_OFF } from '../../hooks-core/poli const HERE = dirname(fileURLToPath(import.meta.url)); const ROUTER = join(HERE, '..', '..', 'plugin', 'hooks', 'pretooluse-router.mjs'); const SESSION_START = join(HERE, '..', '..', 'plugin', 'hooks', 'session-start.mjs'); +const POST_TOOL = join(HERE, '..', '..', 'plugin', 'hooks', 'post-tool.mjs'); const ISOLATED_GRAPH = mkdtempSync(join(tmpdir(), 'assist-graph-')); +const ISOLATED_STATE = mkdtempSync(join(tmpdir(), 'assist-state-')); let workspace; let big; @@ -62,6 +64,10 @@ function runHook(script, payload, modeValue) { 'smart_read,smart_write,smart_edit,smart_glob,smart_grep,wiki_write', TOKEN_OPTIMIZER_WIKI_DIR: ISOLATED_GRAPH, TOKEN_OPTIMIZER_SHARED_DIR: ISOLATED_GRAPH, + // Session state is per run, not per machine: the session ids restart at + // 1 every run, so a shared state root handed one run the "already said" + // flags an earlier run had saved. + TOKEN_OPTIMIZER_STATE_DIR: ISOLATED_STATE, }, }); } @@ -146,18 +152,73 @@ describe('assist does not advertise routing it will not perform', () => { // The hook actually ran and emitted its payload. Three absences alone are // also satisfied by a crashed hook returning '' -- which would suppress the // routing advisory for entirely the wrong reason, and look identical here. - expect(out).toContain('Record what you work out'); + expect(out).toContain('Token optimization is active'); expect(out).not.toContain('Prefer only the registered'); expect(out).not.toContain('These are recommendations'); expect(out).not.toContain('denied only when'); }); - test('but the graph guidance survives, which is the point of assist', () => { - // Guards against over-correction: suppressing the whole payload would make - // assist indistinguishable from off. - expect(startUnder('assist')).toContain('Record what you work out'); + test('the graph guidance survives, deferred to the first sign of real work (#357, M5)', () => { + // Guards against over-correction: suppressing the guidance would make + // assist indistinguishable from off. It is no longer in SessionStart, + // where every session paid for it, but the first post-tool event after an + // edit delivers it -- once. + const session = `assist-deferred-${++sessionSeq}`; + const start = runHook( + SESSION_START, + { hook_event_name: 'SessionStart', cwd: workspace, session_id: session }, + 'assist' + ).stdout; + expect(start).toContain('Token optimization is active'); + expect(start).not.toContain('Record what you work out'); + const edit = { + hook_event_name: 'PostToolUse', + cwd: workspace, + session_id: session, + tool_name: 'Edit', + tool_input: { file_path: big, old_string: 'x', new_string: 'y' }, + tool_response: { success: true }, + }; + expect(runHook(POST_TOOL, edit, 'assist').stdout).toContain( + 'Record what you work out' + ); + expect(runHook(POST_TOOL, edit, 'assist').stdout).not.toContain( + 'Record what you work out' + ); }); + test('an investigation that edits nothing gets it after GUIDANCE_AFTER_CALLS tool calls', () => { + const session = `assist-calls-${++sessionSeq}`; + const bash = { + hook_event_name: 'PostToolUse', + cwd: workspace, + session_id: session, + tool_name: 'Bash', + tool_input: { command: 'git status' }, + tool_response: { stdout: 'clean', exitCode: 0 }, + }; + const read = (n) => + runHook( + ROUTER, + { + hook_event_name: 'PreToolUse', + cwd: workspace, + session_id: session, + tool_name: 'Read', + tool_input: { file_path: join(workspace, `f${n}.ts`) }, + }, + 'assist' + ); + for (let n = 0; n < 9; n++) read(n); + expect(runHook(POST_TOOL, bash, 'assist').stdout).not.toContain( + 'Record what you work out' + ); + read(9); + expect(runHook(POST_TOOL, bash, 'assist').stdout).toContain( + 'Record what you work out' + ); + }, 60_000); + test('enforce still advertises routing, so this is scoped to assist', () => { const out = startUnder('enforce'); expect(out).toContain('Prefer only the registered'); diff --git a/tests/hooks/codex-semantic-harvest.test.mjs b/tests/hooks/codex-semantic-harvest.test.mjs index 0f61fc3e2..d7fc7fa61 100644 --- a/tests/hooks/codex-semantic-harvest.test.mjs +++ b/tests/hooks/codex-semantic-harvest.test.mjs @@ -68,10 +68,17 @@ describe('Codex semantic harvest lifecycle', () => { writeFileSync(first, 'export const first = 1;\n'); writeFileSync(second, 'export const second = 2;\n'); - expect(run(POST, editPayload('semantic-1', first))).toBeNull(); + // The first edit is the first sign of real work, so it carries the deferred + // recording guidance (#357, M5) -- once. The nudge naming the files waits + // for the second edit, as before. + const opening = run(POST, editPayload('semantic-1', first)); + expect(opening.hookSpecificOutput.additionalContext).toContain( + 'Record what you work out' + ); const output = run(POST, editPayload('semantic-1', second)); const context = output.hookSpecificOutput.additionalContext; + expect(context).not.toContain('Record what you work out'); expect(context).toContain('wiki_write'); expect(context).toContain('first.ts'); expect(context).toContain('second.ts'); diff --git a/tests/hooks/mcp-capability-negotiation.test.mjs b/tests/hooks/mcp-capability-negotiation.test.mjs index 3361d1e9e..3fe4227b2 100644 --- a/tests/hooks/mcp-capability-negotiation.test.mjs +++ b/tests/hooks/mcp-capability-negotiation.test.mjs @@ -260,9 +260,17 @@ describe('session policy honesty', () => { }); test('mentions only schemas proven present', () => { - const text = policyText(true, new Set(['smart_grep', 'wiki_write']), true); + // optimize_session, not wiki_write: since #357 (M5) the wiki_write guidance + // is delivered after real work rather than at SessionStart, so this block + // no longer names it either way. + const text = policyText( + true, + new Set(['smart_grep', 'optimize_session']), + true + ); expect(text).toContain('smart_grep'); - expect(text).toContain('wiki_write'); + expect(text).toContain('optimize_session'); expect(text).not.toContain('smart_read'); + expect(text).not.toContain('get_optimization_report'); }); }); diff --git a/tests/hooks/policy-asks-for-findings.test.mjs b/tests/hooks/policy-asks-for-findings.test.mjs index be886035e..f71fa6672 100644 --- a/tests/hooks/policy-asks-for-findings.test.mjs +++ b/tests/hooks/policy-asks-for-findings.test.mjs @@ -29,10 +29,15 @@ const ADAPTER = pathToFileURL( ).href; let policyText; +let recordingGuidance; let volatileLines; +// THE GUIDANCE MOVED OUT OF policyText (#357, M5): SessionStart no longer +// carries it, and the first post-tool event after real work delivers +// recordingGuidance() once instead. What it must say is unchanged, so these +// assertions read it from where it now lives. beforeAll(async () => { - ({ policyText } = await import(ADAPTER)); + ({ policyText, recordingGuidance } = await import(ADAPTER)); ({ volatileLines } = await import( pathToFileURL(join(process.cwd(), 'hooks-core', 'cache.mjs')).href )); @@ -40,12 +45,12 @@ beforeAll(async () => { describe('the standing policy asks for findings', () => { it('names the tool that records them', () => { - expect(policyText(true)).toMatch(/wiki_write/); + expect(recordingGuidance()).toMatch(/wiki_write/); }); it('states the anchor requirement, which the tool enforces', () => { // Without this the agent learns the rule by having writes refused. - expect(policyText(true)).toMatch(/anchor/i); + expect(recordingGuidance()).toMatch(/anchor/i); }); it('says what is worth recording, not merely that recording exists', () => { @@ -53,14 +58,16 @@ describe('the standing policy asks for findings', () => { // specific -- what was TRIED AND REJECTED, and why, because that exists // nowhere in the source tree -- and the briefing has to carry that same // specificity or it produces restatements of what the code already says. - expect(policyText(true)).toMatch(/rejected|failure|decision/i); + expect(recordingGuidance()).toMatch(/rejected|failure|decision/i); }); - it('keeps the briefing in both enforcement modes', () => { - // policyText(false) is the advise-only wording used by clients that cannot - // deny a tool call. Recording findings is not an enforcement feature, so it - // must not disappear along with the denial language. - expect(policyText(false)).toMatch(/wiki_write/); + it('is the same briefing in both enforcement modes, and in neither SessionStart', () => { + // Recording findings is not an enforcement feature, so it must not depend on + // the denial wording either mode uses: it is one text, delivered after the + // session has done real work, and neither policy block repeats it. + expect(recordingGuidance()).toMatch(/wiki_write/); + expect(policyText(true)).not.toMatch(/Record what you work out/); + expect(policyText(false)).not.toMatch(/Record what you work out/); }); }); @@ -108,7 +115,7 @@ describe('the standing policy states what will extract findings after the sessio // that was buried deepest: no credential, no billing, no digest sent. process.env.TOKEN_OPTIMIZER_HARVEST_ENDPOINT = 'http://127.0.0.1:11434/v1/chat/completions'; - const text = policyText(true); + const text = recordingGuidance(); expect(text).toMatch(/local model endpoint is configured/i); expect(text).toMatch(/free and private/i); @@ -117,7 +124,7 @@ describe('the standing policy states what will extract findings after the sessio it('names what a credentialed harvest sends, and what it never sends', () => { process.env.TOKEN_OPTIMIZER_API_KEY = 'sk-test'; - const text = policyText(true); + const text = recordingGuidance(); expect(text).toMatch(/bounded digest/i); expect(text).toMatch(/never file contents/i); @@ -126,7 +133,7 @@ describe('the standing policy states what will extract findings after the sessio it('on a machine with no credential, states the consequence and names the free option', () => { // THE DEFAULT STATE, and the whole reason this line exists. `off:no-key` is // CI, corporate laptops and every subscription-only login. - const text = policyText(true); + const text = recordingGuidance(); expect(text).toMatch(/no separate extractor will run/i); expect(text).toMatch(/derived locally/i); @@ -138,7 +145,7 @@ describe('the standing policy states what will extract findings after the sessio // and this one is charged to every session's prefix. process.env.TOKEN_OPTIMIZER_HARVEST = '0'; process.env.TOKEN_OPTIMIZER_API_KEY = 'sk-test'; - const text = policyText(true); + const text = recordingGuidance(); expect(text).toMatch(/off by configuration/i); expect(text).toMatch(/derived locally/i); @@ -163,6 +170,7 @@ describe('the standing policy states what will extract findings after the sessio for (const key of KEYS) delete process.env[key]; Object.assign(process.env, env); expect(volatileLines(policyText(true))).toHaveLength(0); + expect(volatileLines(recordingGuidance())).toHaveLength(0); } }); }); diff --git a/tests/hooks/recording-guidance-delivery.test.mjs b/tests/hooks/recording-guidance-delivery.test.mjs new file mode 100644 index 000000000..1e4451ffc --- /dev/null +++ b/tests/hooks/recording-guidance-delivery.test.mjs @@ -0,0 +1,85 @@ +/** + * The deferred recording guidance (#357, M5) reaches only the sessions that can + * receive it and are meant to. + * + * Delivery is once per session, so the flag that says "delivered" must not be + * spent where nothing is delivered: an experiment arm with harvest off gets no + * recording instructions at all, and a client that cannot carry context + * (Windsurf's silent style) must leave the flag unset. + */ +import { afterAll, describe, expect, test } from '@jest/globals'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { tmpdir } from 'node:os'; +import { fileURLToPath } from 'node:url'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); +const CLAUDE_POST = join(ROOT, 'plugin', 'hooks', 'post-tool.mjs'); +const WINDSURF_POST = join(ROOT, 'integrations', 'windsurf', 'hooks', 'post-tool.mjs'); + +const scratch = mkdtempSync(join(tmpdir(), 'recording-delivery-')); +afterAll(() => rmSync(scratch, { recursive: true, force: true })); + +function hook(script, payload, env = {}) { + const state = mkdtempSync(join(scratch, 'state-')); + const result = spawnSync(process.execPath, [script], { + input: JSON.stringify(payload), + encoding: 'utf8', + timeout: 30_000, + env: { + ...process.env, + TOKEN_OPTIMIZER_MODE: 'assist', + TOKEN_OPTIMIZER_MCP_CAPABILITIES: 'smart_edit,wiki_write', + TOKEN_OPTIMIZER_WIKI_DIR: join(scratch, 'graph'), + TOKEN_OPTIMIZER_SHARED_DIR: join(scratch, 'graph'), + TOKEN_OPTIMIZER_STATE_DIR: state, + ...env, + }, + }); + const saved = readdirSync(state) + .filter((name) => name.endsWith('.json')) + .map((name) => JSON.parse(readFileSync(join(state, name), 'utf8'))); + return { stdout: result.stdout, saved }; +} + +const file = join(scratch, 'edited.ts'); +writeFileSync(file, 'export const a = 1;\n'); + +const claudeEdit = (session) => ({ + hook_event_name: 'PostToolUse', + session_id: session, + cwd: scratch, + tool_name: 'Edit', + tool_input: { file_path: file, old_string: '1', new_string: '2' }, + tool_response: { success: true }, +}); + +describe('the deferred recording guidance', () => { + test('arrives on the first edit in the full arm (the control for the tests below)', () => { + const { stdout, saved } = hook(CLAUDE_POST, claudeEdit('full-arm')); + expect(stdout).toContain('Record what you work out'); + expect(saved.some((s) => s.recordingGuided === true)).toBe(true); + }); + + test('never arrives in an arm with harvest off, and the flag stays unspent', () => { + for (const arm of ['optimizer', 'retrieval', 'baseline']) { + const { stdout, saved } = hook(CLAUDE_POST, claudeEdit(`arm-${arm}`), { + TOKEN_OPTIMIZER_EXPERIMENT_ARM: arm, + }); + expect(stdout).not.toContain('Record what you work out'); + expect(saved.some((s) => s.recordingGuided === true)).toBe(false); + } + }); + + test('is not marked delivered for a client that cannot carry context', () => { + const { stdout, saved } = hook(WINDSURF_POST, { + agent_action_name: 'post_write_code', + trajectory_id: 'windsurf-silent', + tool_info: { file_path: file, working_directory: scratch, edits: [] }, + }); + expect(stdout.trim()).toBe(''); + expect(saved.length).toBeGreaterThan(0); + expect(saved.some((s) => s.recordingGuided === true)).toBe(false); + }); +}); \ No newline at end of file