diff --git a/tools/bfoptimizer/README.rst b/tools/bfoptimizer/README.rst new file mode 100644 index 000000000..3edc24b0c --- /dev/null +++ b/tools/bfoptimizer/README.rst @@ -0,0 +1,101 @@ +bfoptimizer +=========== + +bfoptimizer uses OpenRouter to propose and implement BPF optimizations. It +builds and tests each change, commits it, and keeps improvements after +benchmarking. + +Setup +----- + +Install the Python dependencies:: + + python3 -m pip install GitPython diskcache jinja2 numpy paramiko pint rich textual textual-plot + +Install Bash and ripgrep for the agent's shell and search tools. The source +checkout also needs the usual bpfilter build and benchmark dependencies. +Neither the Anthropic SDK nor the Claude Agent SDK is required. + +Set ``OPENROUTER_API_KEY`` in your environment. Run from the repository root:: + + python3 -m tools.bfoptimizer --sources-dir /path/to/clean/bpfilter + +The source checkout must have no uncommitted or untracked files. bfoptimizer +commits validated changes and resets failed attempts to their starting commit. + +Models +------ + +Pass OpenRouter model IDs directly. A new model needs no code change:: + + python3 -m tools.bfoptimizer \ + --proposal-model anthropic/claude-opus-5.5 \ + --impl-model anthropic/claude-sonnet-5.5 \ + --commit-model anthropic/claude-haiku-5.5 \ + --effort high + +The proposal and implementation models default to +``anthropic/claude-opus-5.5``. The commit model defaults to the implementation +model. Proposal and implementation models must support tool calling. Commit +messages use plain text, so the commit model does not need structured outputs. + +At startup, bfoptimizer checks each model ID against OpenRouter's public model +list, and checks that the proposal and implementation models support tool +calling. An ID can also use an OpenRouter preset, which is a configuration +saved in the OpenRouter web app. ``@preset/name`` alone is not checked, +because the preset selects the model. ``vendor/model@preset/name`` is +checked as ``vendor/model``. + +``--effort`` accepts ``low``, ``medium``, ``high``, ``xhigh``, and ``max``. +bfoptimizer sends this as OpenRouter's unified ``reasoning.effort`` parameter. +Support and interpretation depend on the model and provider. Commit messages +use ``low`` effort and no ``max_tokens``: reasoning tokens count against that +limit, and a small one could leave no room for the message. + +The displayed cost is the sum of OpenRouter's ``usage.cost`` field over every +request, commit messages included. It misses responses that time out, because +OpenRouter still bills them but bfoptimizer never receives their usage. + +Agent behavior +-------------- + +Each phase has a limit of 100 model responses. The agent can read files, search +with ripgrep, list files with glob patterns, and execute Bash commands. The +implementation phase also exposes exact text edits and file writes under +``src/libbpfilter/cgen/``. Relative paths and shell commands start at the +repository root. The root ``AGENTS.md`` instructions are included when present. +``CLAUDE.md`` and ``CLAUDE.local.md`` are not. + +The proposal prompt prohibits writes. Bash runs with the invoking user's +permissions in both phases. The file tool restrictions do not sandbox Bash. +Commands get no standard input and no ``OPENROUTER_API_KEY``, because their +output goes to the model provider. + +Shell commands time out after 120 seconds. Tool output is truncated at 32,000 +characters. Read returns up to 2,000 lines and ends with the offset to continue +from. Grep and Glob skip ignored files, such as the build directory. Tools +reject the arguments they do not support. + +bfoptimizer retries rate limits, server errors, and network failures up to 6 +times, with an exponential backoff that honors ``Retry-After``. Responses are +not streamed, so a response must arrive within 30 minutes. A response that +times out is not retried: OpenRouter keeps generating and billing it, and a +retry would likely pay for the same slow generation again. Other API failures +and exhausted response limits fail the attempt through the existing reset path. + +Requests in the tool loop set a top-level ``cache_control``, which Anthropic +models need to cache the conversation from one tool round to the next. + +The loop preserves reasoning details between tool calls. It does not provide +the Claude Agent SDK's context compaction, subagents, or session management. +Choose a model with enough context for the source files and tool results. + +References +---------- + +See OpenRouter's documentation on `tool calling`_, `reasoning tokens`_, and +`prompt caching`_ for model requirements and parameter behavior. + +.. _tool calling: https://openrouter.ai/docs/guides/features/tool-calling +.. _reasoning tokens: https://openrouter.ai/docs/guides/best-practices/reasoning-tokens +.. _prompt caching: https://openrouter.ai/docs/guides/best-practices/prompt-caching diff --git a/tools/bfoptimizer/__main__.py b/tools/bfoptimizer/__main__.py index 239f2dc02..9fd2955f4 100644 --- a/tools/bfoptimizer/__main__.py +++ b/tools/bfoptimizer/__main__.py @@ -16,16 +16,17 @@ from tests.benchmarks.bfbencher import compare_table from .ai import generate_commit_message, query_implementation, query_proposal +from .api import api_key, models from .build import MakeTarget, benchmark, configure, make from .log import RenderAs from .models import ( + DEFAULT_MODEL, AttemptResult, AttemptStep, CommitInfo, Config, Effort, History, - Model, ) from .tui import Optimizer @@ -139,9 +140,7 @@ async def _optimization_loop(history: History) -> None: await asyncio.to_thread(repo.git.add, ".") diff = await asyncio.to_thread(repo.git.diff, "--staged") - commit_msg = await asyncio.to_thread( - generate_commit_message, attempt.plan, diff - ) + commit_msg = await generate_commit_message(history, attempt.plan, diff) log.info(f"Commit: {commit_msg}") commit = await asyncio.to_thread(repo.index.commit, commit_msg) @@ -197,6 +196,31 @@ async def _optimization_loop(history: History) -> None: log.info("Optimization completed!") +def _check_models(parser: argparse.ArgumentParser, args: argparse.Namespace) -> None: + """Reject the model IDs that would fail only once their phase starts.""" + + try: + catalog = models() + except RuntimeError as error: + parser.error(str(error)) + + for option, model, needs_tools in ( + ("--proposal-model", args.proposal_model, True), + ("--impl-model", args.impl_model, True), + ("--commit-model", args.commit_model, False), + ): + # Presets ("@preset/...") and variants (":nitro") are not catalog + # entries: check the model they apply to, if the ID names one. + base = model.partition("@")[0] + if not base: + continue + entry = catalog.get(base) or catalog.get(base.partition(":")[0]) + if entry is None: + parser.error(f"{option}: {model} is not an OpenRouter model ID") + if needs_tools and "tools" not in entry.get("supported_parameters", []): + parser.error(f"{option}: {model} does not support tool calling") + + def main() -> None: parser = argparse.ArgumentParser( prog="bfoptimizer", @@ -226,17 +250,21 @@ def main() -> None: ) _ = ai_g.add_argument( "--proposal-model", - type=Model, - default=Model.CLAUDE_OPUS_5_5, - choices=list(Model), - help="Model for the proposal phase (default: %(default)s).", + default=DEFAULT_MODEL, + metavar="MODEL", + help="OpenRouter model ID for proposals (default: %(default)s).", ) _ = ai_g.add_argument( "--impl-model", - type=Model, - default=Model.CLAUDE_OPUS_5_5, - choices=list(Model), - help="Model for the implementation phase (default: %(default)s).", + default=DEFAULT_MODEL, + metavar="MODEL", + help="OpenRouter model ID for implementation (default: %(default)s).", + ) + _ = ai_g.add_argument( + "--commit-model", + default=None, + metavar="MODEL", + help="OpenRouter model ID for commit messages (default: --impl-model).", ) _ = ai_g.add_argument( "--effort", @@ -304,6 +332,13 @@ def main() -> None: args = parser.parse_args() + try: + api_key() + except RuntimeError as error: + parser.error(str(error)) + + if args.commit_model is None: + args.commit_model = args.impl_model if args.build_dir is None: args.build_dir = args.sources_dir / "build" if args.benchmark_exclude_flags is None: @@ -327,8 +362,11 @@ def main() -> None: print(repo.git.status("--short"), file=sys.stderr) sys.exit(1) + _check_models(parser, args) + history = History(Config(**vars(args))) Optimizer(history, lambda: _optimization_loop(history)).run() -main() +if __name__ == "__main__": + main() diff --git a/tools/bfoptimizer/agent_tools.py b/tools/bfoptimizer/agent_tools.py new file mode 100644 index 000000000..58a76fffe --- /dev/null +++ b/tools/bfoptimizer/agent_tools.py @@ -0,0 +1,280 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +"""Local tools exposed to OpenRouter models during optimization.""" + +import asyncio +import json +import os +import pathlib +import signal +import subprocess +import tempfile + +from .log import log_tool + +MAX_OUTPUT_CHARS = 32_000 +COMMAND_TIMEOUT = 120 +READ_LIMIT = 2000 + + +def _tool(name: str, description: str, properties: dict, required: list[str]) -> dict: + return { + "type": "function", + "function": { + "name": name, + "description": description, + "parameters": { + "type": "object", + "properties": properties, + "required": required, + "additionalProperties": False, + }, + }, + } + + +_STRING = {"type": "string"} +_READ_TOOLS = [ + _tool( + "Read", + f"Read a UTF-8 file with line numbers. Paths are relative to the repository root. Returns up to {READ_LIMIT} lines by default, and ends with the offset to continue from when the file is longer.", + { + "file_path": _STRING, + "offset": {"type": "integer", "minimum": 1}, + "limit": {"type": "integer", "minimum": 1}, + }, + ["file_path"], + ), + _tool( + "Grep", + "Search file contents with a ripgrep regular expression, skipping ignored files. Use Bash for other ripgrep options.", + {"pattern": _STRING, "path": _STRING}, + ["pattern"], + ), + _tool( + "Glob", + "List files using a glob relative to the repository root, including **. Ignored files, such as the build directory, are skipped.", + {"pattern": _STRING}, + ["pattern"], + ), + _tool( + "Bash", + f"Run a Bash command from the repository root, without standard input. Commands time out after {COMMAND_TIMEOUT} seconds.", + {"command": _STRING}, + ["command"], + ), +] + + +def _write_tools(cgen_dir: str) -> list[dict]: + return [ + _tool( + "Edit", + f"Replace one exact occurrence of old_string with new_string in a file under {cgen_dir}/.", + {"file_path": _STRING, "old_string": _STRING, "new_string": _STRING}, + ["file_path", "old_string", "new_string"], + ), + _tool( + "Write", + f"Create or overwrite a UTF-8 file under {cgen_dir}/.", + {"file_path": _STRING, "content": _STRING}, + ["file_path", "content"], + ), + ] + + +class AgentTools: + def __init__(self, sources_dir: pathlib.Path, cgen_dir: str, *, writable: bool): + self.root = sources_dir.resolve() + self.write_root = (self.root / cgen_dir).resolve() + writes = _write_tools(cgen_dir) if writable else [] + self.definitions = [*_READ_TOOLS, *writes] + self._parameters = { + tool["function"]["name"]: tool["function"]["parameters"] + for tool in self.definitions + } + self._writes = {tool["function"]["name"] for tool in writes} + + def _path(self, file_path: str, *, write: bool = False) -> pathlib.Path: + path = (self.root / file_path).resolve() + boundary = self.write_root if write else self.root + if not path.is_relative_to(boundary): + raise ValueError(f"path must be under {boundary}") + + return path + + async def execute(self, name: str, raw_arguments: str) -> str: + """Return tool failures to the model so it can correct its arguments.""" + try: + parameters = self._parameters.get(name) + if parameters is None: + raise ValueError(f"tool is not available: {name}") + + arguments = json.loads(raw_arguments) + if not isinstance(arguments, dict): + raise ValueError("tool arguments must be an object") + + log_tool(name, arguments) + + # Ignoring an argument would let the model believe it took effect. + supported = parameters["properties"] + unknown = sorted(arguments.keys() - supported.keys()) + if unknown: + raise ValueError( + f"unsupported arguments: {', '.join(unknown)} (supported: {', '.join(supported)})" + ) + + result = await self._execute(name, **arguments) + if len(result) > MAX_OUTPUT_CHARS: + result = result[:MAX_OUTPUT_CHARS] + "\n[output truncated]" + + return result + except ( + OSError, + ValueError, + TypeError, + KeyError, + subprocess.SubprocessError, + ) as error: + return f"Error: {error}" + + async def _execute(self, name: str, **arguments) -> str: + if name == "Bash": + return await self._command("bash", "-c", arguments["command"]) + if name == "Grep": + path = self._path(arguments.get("path", ".")) + return await self._command( + "rg", + "--line-number", + "--no-heading", + "--color=never", + "--", + arguments["pattern"], + str(path), + ) + # File operations run in a worker so they do not block the TUI. + return await asyncio.to_thread(self._file_tool, name, **arguments) + + def _file_tool(self, name: str, **arguments) -> str: + if name == "Glob": + return self._glob(arguments["pattern"]) + + path = self._path(arguments["file_path"], write=name in self._writes) + if name == "Read": + return self._read( + path, arguments.get("offset", 1), arguments.get("limit", READ_LIMIT) + ) + if name == "Edit": + old = arguments["old_string"] + content = path.read_text(encoding="utf-8") + if not old or content.count(old) != 1: + raise ValueError("old_string must match exactly once") + path.write_text( + content.replace(old, arguments["new_string"], 1), encoding="utf-8" + ) + elif name == "Write": + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(arguments["content"], encoding="utf-8") + return f"Updated {path.relative_to(self.root)}" + + def _glob(self, pattern: str) -> str: + if pathlib.Path(pattern).is_absolute() or ".." in pathlib.Path(pattern).parts: + raise ValueError("glob pattern must be relative to the repository root") + # Unlike Path.glob(), Git skips ignored trees such as the build + # directory, as Grep does. --others adds the files the agent created. + listing = subprocess.run( + [ + "git", + "ls-files", + "-z", + "--cached", + "--others", + "--exclude-standard", + "--", + f":(glob){pattern}", + ], + cwd=self.root, + stdin=subprocess.DEVNULL, + capture_output=True, + timeout=COMMAND_TIMEOUT, + ) + if listing.returncode != 0: + raise ValueError(listing.stderr.decode(errors="replace").strip()) + + paths = { + path + for path in listing.stdout.decode(errors="replace").split("\0") + # The index still lists tracked files deleted from the tree. + if path and (self.root / path).is_file() + } + + return "\n".join(sorted(paths)) or "No files found." + + def _read(self, path: pathlib.Path, offset: int, limit: int) -> str: + if offset < 1 or limit < 1: + raise ValueError("offset and limit must be positive") + + lines = path.read_text(encoding="utf-8").splitlines() + if offset > len(lines): + return ( + f"[Offset {offset} is past the end: the file has {len(lines)} lines.]" + ) + + shown: list[str] = [] + size = 0 + for number, line in enumerate(lines[offset - 1 : offset - 1 + limit], offset): + entry = f"{number}: {line}" + size += len(entry) + 1 + # Stop on a line boundary, and leave room for the note below. + if shown and size > MAX_OUTPUT_CHARS - 100: + break + shown.append(entry) + + last = offset + len(shown) - 1 + if last < len(lines): + shown.append( + f"[Lines {offset}-{last} of {len(lines)}. Read from offset {last + 1} for more.]" + ) + + return "\n".join(shown) + + async def _command(self, *command: str) -> str: + # Output goes to a file, not a pipe: process.wait() would also wait for + # background jobs that keep a pipe open. The file is in the temporary + # directory, which can be tmpfs: a huge output then uses memory until + # the command ends. + with tempfile.TemporaryFile() as output: + process = await asyncio.create_subprocess_exec( + *command, + cwd=self.root, + # The TUI reads the terminal: commands must not take its input. + stdin=asyncio.subprocess.DEVNULL, + stdout=output, + stderr=asyncio.subprocess.STDOUT, + # Command output goes to the model provider. + env={ + key: value + for key, value in os.environ.items() + if key != "OPENROUTER_API_KEY" + }, + start_new_session=True, + ) + + try: + await asyncio.wait_for(process.wait(), timeout=COMMAND_TIMEOUT) + except (TimeoutError, asyncio.CancelledError) as error: + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + await process.wait() + if isinstance(error, asyncio.CancelledError): + raise + return f"Error: command timed out after {COMMAND_TIMEOUT} seconds" + + output.seek(0) + # A UTF-8 character takes up to 4 bytes. Reading this many bytes + # gives execute() more than MAX_OUTPUT_CHARS characters whenever + # the output has them, so it marks the result as truncated. + data = output.read(4 * (MAX_OUTPUT_CHARS + 1)) + + return f"Exit code: {process.returncode}\n{data.decode(errors='replace')}" diff --git a/tools/bfoptimizer/ai.py b/tools/bfoptimizer/ai.py index 73f8c2321..25755ccae 100644 --- a/tools/bfoptimizer/ai.py +++ b/tools/bfoptimizer/ai.py @@ -1,15 +1,14 @@ # Copyright (c) Meta Platforms, Inc. and affiliates. -import functools -import json import logging import pathlib +import re -import anthropic -import claude_agent_sdk import jinja2 +from .agent_tools import AgentTools +from .api import completion, error_text from .log import log_message -from .models import Attempt, Effort, History, Model +from .models import Effort, History _env = jinja2.Environment( loader=jinja2.FileSystemLoader(pathlib.Path(__file__).parent / "templates"), @@ -18,11 +17,7 @@ undefined=jinja2.StrictUndefined, ) -# Cap on conversation turns per agent query (ClaudeAgentOptions.max_turns). MAX_TOOL_ROUNDS = 100 - -# The optimization scope: agents run from this directory, and both prompt -# templates reference it as {{ cgen_dir }}. _CGEN_DIR = "src/libbpfilter/cgen" log = logging.getLogger("bfoptimizer") @@ -38,70 +33,96 @@ def implementation_system_prompt(history: History, failures) -> str: ) -@functools.cache -def _anthropic_client() -> anthropic.Anthropic: - return anthropic.Anthropic() +def _system_prompt(history: History) -> str: + prompt = ( + "You are a performance engineer with a deep understanding of BPF bytecode, " + "the kernel BPF subsystem, packet filtering, and packet matching. " + "Use the available tools to inspect the repository and complete the task. " + "All relative paths and shell commands start at the repository root: " + f"{history.config.sources_dir.resolve()}. " + "Do not build, test, or commit. The optimizer handles these steps." + ) + path = history.config.sources_dir / "AGENTS.md" + if path.is_file(): + prompt += f"\n\nProject instructions (AGENTS.md):\n{path.read_text()}" + return prompt -async def _query( - history: History, - prompt: str, - model: Model, - allowed_tools: list[str], - disallowed_tools: list[str] | None = None, -) -> str | None: - """Run a claude_agent_sdk query, logging messages and accumulating cost. +async def _request( + history: History, model: str, messages: list[dict], effort: Effort, **options +) -> dict: + """Send one completion request, count its cost, and return its message.""" - Returns the final result text, or None if the query produced none. - """ - attempt: Attempt | None = history.current_attempt - if not attempt: + attempt = history.current_attempt + if attempt is None: raise RuntimeError("model queried, but no Attempt in progress") - result: str | None = None - async for message in claude_agent_sdk.query( - prompt=prompt, - options=claude_agent_sdk.ClaudeAgentOptions( - system_prompt={ # how it behaves, every turn - "type": "preset", - "preset": "claude_code", # keep the coding persona - "append": "You are a performance engineer with a deep understanding " - "of the BPF bytecode and subsystem in the kernel, as well as packet " - "filtering and usual packet matching practices.", - }, - cwd=str(history.config.sources_dir / _CGEN_DIR), - allowed_tools=allowed_tools, - disallowed_tools=disallowed_tools or [], - permission_mode="bypassPermissions", - max_turns=MAX_TOOL_ROUNDS, - model=model.value, - **model.agent_options(history.config.effort), - stderr=lambda line: log.debug(line), - ), - ): + response = await completion( + model, messages, reasoning={"effort": effort.value}, **options + ) + attempt.cost += (response.get("usage") or {}).get("cost") or 0.0 + choices = response.get("choices") + if not isinstance(choices, list) or not choices or not isinstance(choices[0], dict): + raise RuntimeError("OpenRouter returned no assistant message") + + choice = choices[0] + if choice.get("error"): + raise RuntimeError(f"OpenRouter provider error: {error_text(choice['error'])}") + if not isinstance(choice.get("message"), dict): + raise RuntimeError("OpenRouter returned no assistant message") + + # Some providers leave finish_reason empty on complete responses. + if choice.get("finish_reason") not in {"stop", "tool_calls", None}: + raise RuntimeError(f"OpenRouter completion failed: {choice['finish_reason']}") + + return choice["message"] + + +async def _query( + history: History, prompt: str, model: str, *, writable: bool = False +) -> str | None: + """Run the tool loop, preserving provider reasoning and tracking every request.""" + + tools = AgentTools(history.config.sources_dir, _CGEN_DIR, writable=writable) + messages = [ + {"role": "system", "content": _system_prompt(history)}, + {"role": "user", "content": prompt}, + ] + + for _ in range(MAX_TOOL_ROUNDS): + message = await _request( + history, + model, + messages, + history.config.effort, + tools=tools.definitions, + # Each round resends the whole conversation, which Anthropic + # models only cache when asked to. + cache_control={"type": "ephemeral"}, + ) log_message(message) - if isinstance(message, claude_agent_sdk.ResultMessage): - attempt.cost += message.total_cost_usd or 0.0 - if not message.is_error: - result = message.result + calls = message.get("tool_calls") or [] + if not calls: + return message.get("content") or None + + # Echo reasoning_details verbatim, including encrypted provider state. + messages.append(message) + for call in calls: + function = call["function"] + result = await tools.execute(function["name"], function["arguments"]) + messages.append( + {"role": "tool", "tool_call_id": call["id"], "content": result} + ) - return result + raise RuntimeError(f"OpenRouter agent exceeded {MAX_TOOL_ROUNDS} rounds") async def query_proposal(history: History) -> None: log.info("Requesting the proposal") - plan = await _query( - history, - proposal_system_prompt(history), - history.config.proposal_model, - allowed_tools=["Read", "Grep", "Glob", "Bash", "ToolSearch"], - disallowed_tools=["ScheduleWakeup"], + history, proposal_system_prompt(history), history.config.proposal_model ) - - # An empty plan is left for the caller to handle: it aborts the attempt, - # not the run. attempt = history.current_attempt attempt.plan = plan or "" if attempt.plan: @@ -111,50 +132,54 @@ async def query_proposal(history: History) -> None: async def query_implementation(history: History, failures) -> None: log.info("Requesting the implementation") + # The final message is not needed: the edits are already on disk, and the + # build and tests decide whether they are valid. await _query( history, implementation_system_prompt(history, failures), history.config.impl_model, - allowed_tools=["Read", "Grep", "Glob", "Bash", "Edit", "Write"], + writable=True, ) - cost = history.current_attempt.cost - log.info(f"Implementation completed (total cost ${cost:.2f})") - - -def generate_commit_message(plan: str, diff: str) -> str: - response = _anthropic_client().messages.create( - model=Model.CLAUDE_HAIKU_5_5.value, - max_tokens=500, - # Reserve the output budget for the commit message. - thinking={"type": "disabled"}, - output_config={ - "effort": Effort.LOW.value, - "format": { - "type": "json_schema", - "schema": { - "type": "object", - "properties": { - "subject": {"type": "string"}, - "body": {"type": "string"}, - }, - "required": ["subject", "body"], - "additionalProperties": False, - }, - }, - }, - messages=[ + log.info( + f"Implementation completed (total cost ${history.current_attempt.cost:.2f})" + ) + + +_COMMIT_LABEL = re.compile(r"^(subject|body):\s*", re.IGNORECASE) + + +def _commit_message(text: str) -> str: + """Remove the Markdown fence and labels that a model may add.""" + lines = text.strip().splitlines() + if len(lines) > 1 and lines[0].startswith("```") and lines[-1] == "```": + lines = lines[1:-1] + lines = [_COMMIT_LABEL.sub("", line) for line in lines] + subject, _, body = "\n".join(lines).strip().partition("\n") + return f"{subject.strip()}\n\n{body.strip()}".strip() + + +async def generate_commit_message(history: History, plan: str, diff: str) -> str: + # No max_tokens: reasoning counts against it, and could leave no room for + # the message itself. + message = await _request( + history, + history.config.commit_model, + [ { "role": "user", - "content": f"""Generate a git commit message (subject and body) for this change to the bpfilter project. + "content": f"""Generate a git commit message for this change to the bpfilter project. -Subject: `component: subcomponent: short description` +Return only the commit message as plain text: the subject line, a blank line, +then the body. Do not add labels, Markdown fences, or commentary. + +The subject line follows `component: subcomponent: short description`: - Components: lib, cli, tests, build, tools, doc - Subcomponents examples: cgen, matcher, chain, rule, hook - Lowercase, imperative mood, no period, ≤72 chars total - Describe WHY the change improves things, not what lines changed -Body: 2-4 sentences explaining the intent behind the change and why it +The body has 2-4 sentences explaining the intent behind the change and why it improves the generated BPF programs. Wrap lines at 72 characters. Do not repeat the subject or describe the diff line by line. @@ -166,6 +191,11 @@ def generate_commit_message(plan: str, diff: str) -> str: """, } ], + Effort.LOW, ) - message = json.loads(response.content[0].text) - return f"{message['subject'].strip()}\n\n{message['body'].strip()}" + + text = _commit_message(message.get("content") or "") + if not text: + raise RuntimeError("OpenRouter returned an empty commit message") + + return text diff --git a/tools/bfoptimizer/api.py b/tools/bfoptimizer/api.py new file mode 100644 index 000000000..b4aae729f --- /dev/null +++ b/tools/bfoptimizer/api.py @@ -0,0 +1,170 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +"""OpenRouter Chat Completions transport, using only the standard library.""" + +import asyncio +import concurrent.futures +import http.client +import json +import logging +import os +import threading +import urllib.error +import urllib.request + +API_URL = "https://openrouter.ai/api/v1" + +# Responses are not streamed: the body only arrives once the whole generation, +# reasoning included, is done. +REQUEST_TIMEOUT = 1800 +MAX_RETRIES = 6 + +log = logging.getLogger("bfoptimizer") + + +class TransientError(RuntimeError): + """Request failure that the same request may not hit again.""" + + def __init__(self, message: str, retry_after: float = 0.0) -> None: + super().__init__(message) + self.retry_after = retry_after + + +def api_key() -> str: + key = os.environ.get("OPENROUTER_API_KEY", "").strip() + if not key: + raise RuntimeError("set OPENROUTER_API_KEY before running bfoptimizer") + + return key + + +def error_text(error: object) -> str: + """Return the message of an OpenRouter error object.""" + + if isinstance(error, dict) and error.get("message"): + return str(error["message"]) + + return str(error) + + +def _retryable(code: object) -> bool: + return isinstance(code, int) and (code in (408, 429) or code >= 500) + + +def _retry_after(headers) -> float: + try: + return float(headers.get("Retry-After") or 0) + except ValueError: + return 0.0 + + +def _http_error(error: urllib.error.HTTPError) -> RuntimeError: + detail = error.reason + try: + with error: + detail = error_text(json.load(error)["error"]) + except (OSError, ValueError, KeyError, TypeError, http.client.HTTPException): + pass + + message = f"OpenRouter HTTP {error.code}: {detail}" + if _retryable(error.code): + return TransientError(message, _retry_after(error.headers)) + + return RuntimeError(message) + + +def _send(request: urllib.request.Request, timeout: float) -> dict: + """Send one request, raising TransientError for failures worth repeating.""" + + try: + response = urllib.request.urlopen(request, timeout=timeout) + except urllib.error.HTTPError as error: + raise _http_error(error) from error + except (OSError, http.client.HTTPException) as error: + raise TransientError(f"OpenRouter request failed: {error}") from error + + with response: + try: + result = json.load(response) + except TimeoutError as error: + # A non-streaming generation continues, and is billed, after a + # disconnect: a retry would likely pay for the same slow one again. + raise RuntimeError( + f"OpenRouter response took over {timeout} seconds" + ) from error + except (OSError, http.client.HTTPException) as error: + raise TransientError(f"OpenRouter response failed: {error}") from error + except ValueError as error: + raise RuntimeError("OpenRouter returned invalid JSON") from error + + if not isinstance(result, dict): + raise RuntimeError("OpenRouter returned an invalid response") + + # Failures after a provider accepted the request arrive with HTTP 200. + failure = result.get("error") + if failure: + code = failure.get("code") if isinstance(failure, dict) else None + message = f"OpenRouter error {code}: {error_text(failure)}" + if _retryable(code): + raise TransientError(message) + raise RuntimeError(message) + + return result + + +def _detached(function, *args) -> asyncio.Future: + """Run a blocking call in a daemon thread. + + Unlike asyncio.to_thread(), an abandoned call does not delay the process + exit, so quitting does not wait for a response that can take minutes. + """ + + future: concurrent.futures.Future = concurrent.futures.Future() + + def run() -> None: + if not future.set_running_or_notify_cancel(): + return + try: + future.set_result(function(*args)) + except BaseException as error: + future.set_exception(error) + + threading.Thread(target=run, daemon=True).start() + + return asyncio.wrap_future(future) + + +async def completion(model: str, messages: list[dict], **options) -> dict: + """Request a chat completion, retrying failures that may be transient.""" + request = urllib.request.Request( + f"{API_URL}/chat/completions", + data=json.dumps({"model": model, "messages": messages, **options}).encode(), + headers={ + "Authorization": f"Bearer {api_key()}", + "Content-Type": "application/json", + "X-Title": "bfoptimizer", + }, + method="POST", + ) + + for retry in range(MAX_RETRIES): + try: + return await _detached(_send, request, REQUEST_TIMEOUT) + except TransientError as error: + # Honor Retry-After, but do not let a bogus value stall the run. + delay = max(2**retry, min(error.retry_after, 300)) + log.warning(f"{error}, retrying in {delay:g} seconds") + await asyncio.sleep(delay) + + return await _detached(_send, request, REQUEST_TIMEOUT) + + +def models() -> dict[str, dict]: + """Return the public OpenRouter model catalog, keyed by model ID.""" + + request = urllib.request.Request( + f"{API_URL}/models", headers={"X-Title": "bfoptimizer"} + ) + try: + return {model["id"]: model for model in _send(request, 60)["data"]} + except (KeyError, TypeError) as error: + raise RuntimeError("OpenRouter returned an invalid model list") from error diff --git a/tools/bfoptimizer/log.py b/tools/bfoptimizer/log.py index eb360a78e..0cfbbb846 100644 --- a/tools/bfoptimizer/log.py +++ b/tools/bfoptimizer/log.py @@ -1,11 +1,9 @@ # Copyright (c) Meta Platforms, Inc. and affiliates. -"""Translation of claude_agent_sdk messages into bfoptimizer log records.""" +"""Translation of OpenRouter messages into bfoptimizer log records.""" import enum import logging -import claude_agent_sdk - log = logging.getLogger("bfoptimizer") @@ -15,71 +13,26 @@ class RenderAs(enum.StrEnum): RICH = "rich" -# Input keys to log for tool-use blocks: a curated subset for tools whose -# input is too noisy to show in full (Write's content, Edit's old/new -# strings), None to hide the block entirely. Tools not listed here log -# their full input. -_BLOCK_KV: dict[str, list[str] | None] = { +_TOOL_KEYS = { "Read": ["file_path"], "Write": ["file_path"], "Edit": ["file_path"], - "Agent": [], "Grep": ["path", "pattern"], - "ToolSearch": ["query"], "Glob": ["pattern"], "Bash": ["command"], - "ExitPlanMode": None, } -def log_message(message: claude_agent_sdk.Message) -> None: - """Log a claude_agent_sdk message to the bfoptimizer logger.""" - if isinstance(message, claude_agent_sdk.AssistantMessage): - for block in message.content: - _log_block(block) - elif isinstance(message, claude_agent_sdk.ResultMessage): - if message.is_error: - log.error(f"API error: {message.result}") - elif isinstance( - message, - ( - claude_agent_sdk.TaskProgressMessage, - claude_agent_sdk.TaskNotificationMessage, - claude_agent_sdk.TaskUpdatedMessage, - claude_agent_sdk.TaskStartedMessage, - ), - ): - # Subagent lifecycle events. These subclass SystemMessage, so they - # must be matched before the SystemMessage branch below, which would - # otherwise catch them and log their repr. - pass - elif isinstance(message, claude_agent_sdk.SystemMessage): - if message.subtype not in ["init", "thinking_tokens"]: - log.info(message) - elif isinstance(message, claude_agent_sdk.UserMessage): - pass - else: - log.info(f"Unsupported message: {message}") +def log_message(message: dict) -> None: + """Log assistant text without exposing encrypted reasoning blocks.""" + if message.get("content"): + log.info(message["content"], extra={"render_as": RenderAs.MARKDOWN}) -def _log_block(block: claude_agent_sdk.ContentBlock) -> None: - if isinstance(block, claude_agent_sdk.ThinkingBlock): - if not block.thinking: - # Some models return empty thinking blocks - return - log.info(block.thinking, extra={"render_as": RenderAs.MARKDOWN}) - elif isinstance(block, claude_agent_sdk.ToolUseBlock): - keys = _BLOCK_KV.get(block.name, list(block.input)) - if keys is None: - return - log.info( - block.name, - extra={"kv": {key: block.input.get(key, "") for key in keys}}, - ) - elif isinstance(block, claude_agent_sdk.ToolResultBlock): - log.info(f"ToolResultBlock {block}") - elif isinstance(block, claude_agent_sdk.TextBlock): - log.info(block.text, extra={"render_as": RenderAs.MARKDOWN}) - else: - log.info(block) +def log_tool(name: str, arguments: dict) -> None: + # Tools without a curated subset log their full input. + keys = _TOOL_KEYS.get(name, list(arguments)) + log.info( + name, extra={"kv": {key: arguments[key] for key in keys if key in arguments}} + ) diff --git a/tools/bfoptimizer/models.py b/tools/bfoptimizer/models.py index 66b3a33c4..0ffd2cdfe 100644 --- a/tools/bfoptimizer/models.py +++ b/tools/bfoptimizer/models.py @@ -16,15 +16,7 @@ from tests.benchmarks.bfbencher import Report -class Model(enum.StrEnum): - CLAUDE_FABLE_5_1 = "claude-fable-5-1" - CLAUDE_OPUS_5_5 = "claude-opus-5-5" - CLAUDE_SONNET_5_5 = "claude-sonnet-5-5" - CLAUDE_HAIKU_5_5 = "claude-haiku-5-5" - - def agent_options(self, effort: "Effort") -> dict: - """Return kwargs to unpack into ClaudeAgentOptions for this model and effort.""" - return {"effort": effort.value, "thinking": {"type": "adaptive"}} +DEFAULT_MODEL = "anthropic/claude-opus-5.5" class Effort(enum.StrEnum): @@ -43,8 +35,9 @@ class Config: build_dir: pathlib.Path iterations: int - proposal_model: Model - impl_model: Model + proposal_model: str + impl_model: str + commit_model: str effort: Effort hint: str | None benchmark_host: str | None @@ -194,9 +187,7 @@ def cumulative_progress(self, up_to: int | None = None) -> float: """ attempts = self.attempts if up_to is None else self.attempts[: up_to + 1] successes = [ - attempt - for attempt in attempts - if attempt.result == AttemptResult.SUCCESS + attempt for attempt in attempts if attempt.result == AttemptResult.SUCCESS ] if not successes: return 0.0 diff --git a/tools/bfoptimizer/templates/implementation.j2 b/tools/bfoptimizer/templates/implementation.j2 index 58b238e0f..156927113 100644 --- a/tools/bfoptimizer/templates/implementation.j2 +++ b/tools/bfoptimizer/templates/implementation.j2 @@ -13,7 +13,7 @@ Only modify files under {{ cgen_dir }}/. You may read anywhere in the tree (incl ## Style -See CLAUDE.md for the complete guide. Key points: 4 spaces, 80-char line limit, `//` for single-line comments, `/* */` with aligned asterisks for multi-line. +See AGENTS.md for the complete guide. Key points: 4 spaces, 80-char line limit, `//` for single-line comments, `/* */` with aligned asterisks for multi-line. {% if failures %} ## Failures