Merge pull request 'Keep runs with different tuning apart in the report, and raise max_turns to 40' (#52) from report-keeps-tuning-apart into main

Reviewed-on: dongar/nisse#52
Reviewed-by: Don Garrett <dgarrett@acm.org>

docs/Tuning-Guide.md

@@ -147,7 +147,7 @@ every request, on both routes.
   expect, which is the model's output ceiling divided by its generation
   speed.
 
-### `tuning.executor.max_turns` — default 24
+### `tuning.executor.max_turns` — default 40
 
 How many model calls one milestone may make before the executor stops with
 `capped` and asks you what to do.
@@ -161,6 +161,11 @@ How many model calls one milestone may make before the executor stops with
   the cap comfortably above the high end, around 1.5–2× the turns of your
   longer successful runs. A stronger model usually needs fewer turns, not
   more.
+- **Measured:** the default was 24 until the harder benchmark tasks arrived.
+  Haiku, used as the ceiling in calibration, hit 24 on both runs of
+  `refactor-rename` (editing ten files one at a time) and of
+  `long-milestone`, one of which had in fact finished the work. It is a guard
+  against a model trying forever, not a budget, so it was raised to 40.
 
 ### `tuning.executor.failure_limit` — default 2
 

nisse-dev.example.yaml

@@ -47,7 +47,7 @@ gitea:                                # B2
 # read at startup, so a change takes effect on the next restart.
 tuning:
   executor:
-    max_turns: 24            # model calls per milestone before it stops as capped
+    max_turns: 40            # model calls per milestone before it stops as capped
     failure_limit: 2         # consecutive failed commands before escalating (A12)
   expert:
     consult_budget: 3        # ask_expert calls per milestone (A14)

nisse-dev/src/nisse_dev/bench/results.py

@@ -10,12 +10,13 @@ import math
 import statistics
 from collections import defaultdict
 from collections.abc import Iterable
-from dataclasses import dataclass
+from dataclasses import asdict, dataclass
 from datetime import UTC, datetime
 from pathlib import Path
 from typing import Any
 
 from nisse_dev.bench.runner import RunRecord
+from nisse_dev.config import Tuning
 
 
 def results_file(directory: Path, model: str, when: datetime | None = None) -> Path:
@@ -64,21 +65,47 @@ class Row:
     cost_per_run: float
     stale: bool
     rate_limited: int = 0  # runs that waited out the proxy's rate limit
+    tuning: str = ""  # how the runs' tuning differs from today's defaults
 
     @property
     def interval(self) -> tuple[float, float]:
         return wilson(self.successes, self.runs)
 
 
+def tuning_label(tuning: dict[str, Any]) -> str:
+    """The settings in a run's tuning that differ from today's defaults, as
+    "executor.max_turns=24"; empty when there are none. Relative to today's
+    defaults so that rows at the current settings stay unlabelled."""
+    defaults = asdict(Tuning())
+    return ", ".join(
+        f"{section}.{key}={value}"
+        for section, values in sorted(tuning.items())
+        for key, value in sorted((values or {}).items())
+        if defaults.get(section, {}).get(key) != value
+    )
+
+
 def summarise(
     records: Iterable[dict[str, Any]], current_hashes: dict[str, str]
 ) -> list[Row]:
-    """One row per (model, how it was served, task, task version)."""
-    groups: dict[tuple[str, str, str, str], list[dict[str, Any]]] = defaultdict(list)
+    """One row per (model, how it was served, task, task version, tuning).
+
+    Tuning is part of the key because a knob changes the result: runs capped
+    at 24 turns and runs allowed 40 are not the same measurement."""
+    groups: dict[tuple[str, str, str, str, str], list[dict[str, Any]]] = defaultdict(
+        list
+    )
     for r in records:
-        groups[(r["backing"], r.get("note", ""), r["task"], r["task_hash"])].append(r)
+        key = (
+            r["backing"],
+            r.get("note", ""),
+            r["task"],
+            r["task_hash"],
+            tuning_label(r.get("tuning", {})),
+        )
+        groups[key].append(r)
     rows = []
-    for (backing, note, task, task_hash), runs in sorted(groups.items()):
+    for (backing, note, task, task_hash, tuning), runs in sorted(groups.items()):
         rows.append(
             Row(
                 model=backing,
@@ -93,22 +120,29 @@ def summarise(
                 cost_per_run=statistics.fmean(r["cost"] for r in runs),
                 stale=current_hashes.get(task) != task_hash,
                 rate_limited=sum(1 for r in runs if r.get("rate_limit_wait_s")),
+                tuning=tuning,
             )
         )
     return rows
 
 
 def render(rows: list[Row]) -> str:
+    # A tuning column only when some rows need one, so a report at today's
+    # settings looks as it always has.
+    tuned = any(row.tuning for row in rows)
     header = (
-        f"{'model':<34} {'note':<22} {'task':<15} {'success':>8} {'95% CI':>11} "
+        f"{'model':<34} {'note':<22} "
+        + (f"{'tuning':<24} " if tuned else "")
+        + f"{'task':<15} {'success':>8} {'95% CI':>11} "
         f"{'turns':>6} {'time':>7} {'$/run':>8} {'429':>4}"
     )
     lines = [header, "-" * len(header)]
     for row in rows:
         low, high = row.interval
         task = row.task + (" (old)" if row.stale else "")
+        tuning = f"{row.tuning[:24]:<24} " if tuned else ""
         lines.append(
-            f"{row.model[:34]:<34} {row.note[:22]:<22} {task[:15]:<15} "
+            f"{row.model[:34]:<34} {row.note[:22]:<22} {tuning}{task[:15]:<15} "
             f"{row.successes:>3}/{row.runs:<4} {low:>4.0%}-{high:<5.0%} "
             f"{row.median_turns:>6.0f} {row.mean_wall_s:>6.0f}s {row.cost_per_run:>8.4f} "
             f"{row.rate_limited or '':>4}"
@@ -119,6 +153,9 @@ def render(rows: list[Row]) -> str:
             "429 = runs that waited out the proxy's rate limit; time leaves the "
             "waits out."
         )
+    if tuned:
+        lines.append("")
+        lines.append("tuning = settings that differ from today's defaults.")
     if any(row.stale for row in rows):
         lines.append("")
         lines.append(

nisse-dev/src/nisse_dev/config.py

@@ -64,7 +64,7 @@ class ConfigError(RuntimeError):
 
 @dataclass(frozen=True)
 class ExecutorTuning:
-    max_turns: int = 24
+    max_turns: int = 40
     failure_limit: int = 2
 
 

nisse-dev/tests/test_bench.py

@@ -149,7 +149,7 @@ def test_a_solved_task_is_a_success(tmp_path: Path) -> None:
     assert record.tokens_in == 3000
     assert record.task_hash == task.content_hash
     assert record.model_info["supports_tool_choice"] is True
-    assert record.tuning["executor"]["max_turns"] == 24
+    assert record.tuning["executor"]["max_turns"] == 40
 
 
 def test_saying_finished_is_not_success(tmp_path: Path) -> None:
@@ -317,3 +317,20 @@ def test_results_stored_before_waits_were_recorded_still_load() -> None:
     del old["rate_limit_wait_s"]
     [row] = summarise([old], current)
     assert (row.mean_wall_s, row.rate_limited) == (12.0, 0)
+
+
+def test_runs_with_different_tuning_are_kept_apart() -> None:
+    current = {t.name: t.content_hash for t in TASKS}
+    capped = record_json()
+    capped["tuning"]["executor"]["max_turns"] = 7
+    rows = summarise([record_json(), capped], current)
+    assert sorted(row.tuning for row in rows) == ["", "executor.max_turns=7"]
+    table = render(rows)
+    assert "executor.max_turns=7" in table
+    assert "tuning = settings that differ" in table
+
+
+def test_a_report_at_default_tuning_has_no_tuning_column() -> None:
+    current = {t.name: t.content_hash for t in TASKS}
+    table = render(summarise([record_json()], current))
+    assert "tuning" not in table

nisse-dev/tests/test_config.py

@@ -69,7 +69,7 @@ def test_complete_config_builds_with_secrets_by_reference() -> None:
     assert config.matrix.homeserver == "https://matrix.test"
     assert config.gitea_repo == "don/demo"
     assert config.base_branch == "main"
-    assert config.tuning.executor.max_turns == 24  # absent section: defaults
+    assert config.tuning.executor.max_turns == 40  # absent section: defaults
 
 
 def test_paths_expand_the_home_directory() -> None:
@@ -272,7 +272,7 @@ def test_the_example_file_is_valid() -> None:
         "NISSE_MATRIX_TOKEN": "syt_x",
         "NISSE_GITEA_TOKEN": "gt",
     }
-    assert AgentConfig.from_file(example, env).tuning.executor.max_turns == 24
+    assert AgentConfig.from_file(example, env).tuning.executor.max_turns == 40
 
 
 # --- nothing that carries secrets may sit inside the clone -----------------