Merge branch 'main' into experiment/code-review/inline-knowledge

gggdttt · web-flow · commit efbe8130e457 · 2026-07-02T15:28:56.000+02:00
diff --git a/dataset/codereview.jsonl b/dataset/codereview.jsonl
diff --git a/docs/_data/code-review.json b/docs/_data/code-review.json
diff --git a/pyproject.toml b/pyproject.toml
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "bcbench"
-version = "0.6.1"
+version = "0.7.0"
 description = "Benchmarking tool for Business Central (AL) ecosystem, inspired by SWE-Bench"
 readme = "README.md"
 requires-python = ">=3.13"
@@ -87,6 +87,11 @@ ignore = [
     "TRY301",  # raise-within-try: suggested "abstract to inner function" refactor adds indirection
 ]
 
+[tool.ruff.lint.flake8-tidy-imports.banned-api]
+# Force sandboxed rendering:
+"jinja2.Template".msg = "Use jinja2.sandbox.SandboxedEnvironment instead; the bare Template is not sandboxed."
+"jinja2.Environment".msg = "Use jinja2.sandbox.SandboxedEnvironment instead; the bare Environment is not sandboxed."
+
 [tool.ruff.lint.per-file-ignores]
 "tests/**" = ["ANN"]
 "notebooks/**" = ["ANN", "T20"]   # notebooks routinely print() for analysis output
diff --git a/src/bcbench/agent/bcal/bc_eval_capi_bridge.py b/src/bcbench/agent/bcal/bc_eval_capi_bridge.py
@@ -16,6 +16,10 @@
 _CERT_TENANT_ENV = "CAPI_TENANT_ID"
 _CERT_CLIENT_ENV = "CAPI_CLIENT_ID"
 
+# Reasoning effort (not all models support this parameter), different models might have different available values:
+# Set to None to omit the parameter entirely (lets the model/service use its own default).
+_DEFAULT_REASONING_EFFORT: str | None = None
+
 
 def _to_jsonable(value: object) -> dict[str, object]:
     model_dump = getattr(value, "model_dump", None)
@@ -148,6 +152,10 @@ def main() -> int:
     if request.get("tools"):
         kwargs["tools"] = request["tools"]
 
+    reasoning_effort = request.get("reasoning_effort", _DEFAULT_REASONING_EFFORT)
+    if reasoning_effort is not None:
+        kwargs["reasoning_effort"] = reasoning_effort
+
     response = _create_with_retry(client, kwargs)
     json.dump(_to_jsonable(response), sys.stdout)
     return 0
diff --git a/src/bcbench/agent/shared/mcp.py b/src/bcbench/agent/shared/mcp.py
@@ -4,7 +4,7 @@
 from pathlib import Path
 from typing import Any
 
-from jinja2 import Template
+from jinja2.sandbox import SandboxedEnvironment
 
 from bcbench.agent.shared.altool_paths import build_assembly_probing_paths, compiler_symbol_folder_for_container
 from bcbench.dataset import BaseDatasetEntry
@@ -13,6 +13,8 @@
 
 logger = get_logger(__name__)
 
+_jinja = SandboxedEnvironment(autoescape=False)
+
 
 def _build_server_entry(server: dict[str, Any], template_context: dict[str, Any]) -> tuple[str, dict[str, Any]]:
     server_type: str = server["type"]
@@ -26,7 +28,7 @@ def _build_server_entry(server: dict[str, Any], template_context: dict[str, Any]
             }
         case "stdio":
             args: list[str] = server["args"]
-            rendered_args = [Template(arg).render(**template_context) for arg in args]
+            rendered_args = [_jinja.from_string(arg).render(**template_context) for arg in args]
             command: str = shutil.which(server["command"]) or server["command"]
             entry: dict[str, Any] = {
                 "type": server_type,
diff --git a/src/bcbench/agent/shared/prompt.py b/src/bcbench/agent/shared/prompt.py
@@ -1,14 +1,16 @@
 import re
 from pathlib import Path
 
-from jinja2 import Template
+from jinja2.sandbox import SandboxedEnvironment
 
 from bcbench.config import get_config
 from bcbench.dataset import BaseDatasetEntry
 from bcbench.types import EvaluationCategory
 
 _config = get_config()
 
+_jinja = SandboxedEnvironment(autoescape=False)
+
 
 def _transform_image_paths(content: str) -> str:
     dest_dir = _config.file_patterns.problem_statement_dest_dir
@@ -26,8 +28,7 @@ def build_prompt(entry: BaseDatasetEntry, repo_path: Path, config: dict, categor
 
     task = _transform_image_paths(entry.get_task())
 
-    template = Template(template_str)
-    return template.render(
+    return _jinja.from_string(template_str).render(
         repo_path=repo_path,
         task=task,
         project_paths=", ".join(entry.project_paths),
diff --git a/src/bcbench/cli_options.py b/src/bcbench/cli_options.py
@@ -11,7 +11,7 @@
 # Note: Defaults are provided in function signatures, not here
 RepoPath = Annotated[Path, typer.Option(help="Path to repository")]
 
-OutputDir = Annotated[Path, typer.Option(help="Directory to save evaluation results")]
+OutputDir = Annotated[Path, typer.Option(help="Directory to save evaluation results", file_okay=False, dir_okay=True)]
 
 RunId = Annotated[str, typer.Option(envvar="GITHUB_RUN_ID", help="Unique identifier for this evaluation run")]
 
diff --git a/src/bcbench/dataset/codereview.py b/src/bcbench/dataset/codereview.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from enum import StrEnum
+from typing import Annotated
 
 from pydantic import BaseModel, ConfigDict, Field, field_validator
 
@@ -46,11 +47,11 @@ def from_input(cls, value: str) -> Severity:
 class ReviewComment(BaseModel):
     model_config = ConfigDict(frozen=True)
 
-    file: str
-    line_start: int
-    line_end: int | None = None
+    file: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9./_-]*\.(al|json)$")]
+    line_start: Annotated[int, Field(ge=1)]
+    line_end: Annotated[int, Field(ge=1)] | None = None
     domain: str | None = None
-    body: str
+    body: Annotated[str, Field(min_length=1)]
     severity: Severity | None = None
 
     @field_validator("severity", mode="before")
diff --git a/src/bcbench/dataset/dataset_entry.py b/src/bcbench/dataset/dataset_entry.py
@@ -20,15 +20,15 @@
 class TestEntry(BaseModel):
     model_config = ConfigDict(frozen=True)
 
-    codeunitID: int
+    codeunitID: Annotated[int, Field(gt=0)]
     functionName: Annotated[frozenset[str], Field(min_length=1)]
 
 
 class EntryMetadata(BaseModel):
     model_config = ConfigDict(frozen=True)
 
     area: str | None = None
-    image_count: int | None = None
+    image_count: Annotated[int, Field(ge=0)] | None = None
     persona: str | None = None
 
 
@@ -44,8 +44,8 @@ class BaseDatasetEntry(BaseModel):
     base_commit: str = Field(pattern=r"^[a-fA-F0-9]{40}$")
     created_at: Annotated[str, Field(min_length=1)]
     environment_setup_version: str = Field(pattern=r"^[0-9]{2}\.[0-9]{1}$")
-    project_paths: list[str] = []
-    patch: Annotated[str, Field(min_length=1)]
+    project_paths: list[Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9 \\/-]*$")]] = []
+    patch: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")]
 
     @classmethod
     def load(cls, dataset_path: Path, entry_id: str | None = None, random: int | None = None) -> list[Self]:
@@ -112,7 +112,7 @@ class _BugFixTestGenBase(BaseDatasetEntry):
 
     fail_to_pass: Annotated[list[TestEntry], Field(alias="FAIL_TO_PASS", min_length=1)]
     pass_to_pass: Annotated[list[TestEntry], Field(alias="PASS_TO_PASS")] = []
-    test_patch: Annotated[str, Field(min_length=1)]
+    test_patch: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")]
 
     @property
     def problem_statement_dir(self) -> Path:
@@ -158,9 +158,9 @@ class NL2ALEntry(BaseDatasetEntry):
     """Dataset entry for NL2AL category — generate AL code from natural language."""
 
     base_commit: str | None = None
-    nl_prompt: Annotated[str, Field(min_length=1)]
+    nl_prompt: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")]
     expected: Annotated[list[ChecklistAssertion], Field(min_length=1)]
-    page: str
+    page: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9 ./]*$")]
     audience: Literal["Business", "Technical", "Both"]
 
     def get_task(self) -> str:
diff --git a/src/bcbench/operations/hooks_operations.py b/src/bcbench/operations/hooks_operations.py
@@ -1,5 +1,6 @@
 import contextlib
 import json
+import shlex
 from pathlib import Path
 
 from bcbench.config import get_config
@@ -60,15 +61,16 @@ def _setup_claude_hooks(repo_path: Path, script_path: str, tool_log_path: Path)
         with contextlib.suppress(json.JSONDecodeError):
             existing_settings = json.loads(settings_file.read_text(encoding="utf-8"))
 
-    tool_log_resolved = str(tool_log_path.resolve())
+    tool_log_quoted = shlex.quote(str(tool_log_path.resolve()))
+    script_path_quoted = shlex.quote(script_path)
     existing_settings["hooks"] = {
         "PreToolUse": [
             {
                 "matcher": "",
                 "hooks": [
                     {
                         "type": "command",
-                        "command": f'BCBENCH_TOOL_LOG="{tool_log_resolved}" pwsh -ExecutionPolicy Bypass -File "{script_path}"',
+                        "command": f"BCBENCH_TOOL_LOG={tool_log_quoted} pwsh -ExecutionPolicy Bypass -File {script_path_quoted}",
                     }
                 ],
             }
diff --git a/src/bcbench/results/codereview.py b/src/bcbench/results/codereview.py
@@ -19,7 +19,7 @@
 <summary>📖 How to read these metrics</summary>
 
 - **Micro** — sums matched/generated/expected across all tasks and computes one score; tasks with many comments dominate.
-- **Macro** — computes P/R/F1 per task and averages the scores; every task counts equally regardless of comment volume. Macro precision averages only over tasks where the agent left at least one comment, so staying silent is not scored as perfect precision (recall and F1 still span every task).
+- **Macro** — computes P/R/F1 per task and averages the scores; every task counts equally regardless of comment volume. Macro precision averages over tasks where the agent commented plus negative tasks (no expected findings), where correct silence scores as perfect precision; silence on a positive task is still not rewarded. Macro recall averages only over positive tasks, since negative tasks have nothing to recall.
 - **Matched comment** — a generated comment paired with an expected one by file and line proximity (within the configured tolerance), then confirmed by an LLM judge to describe the same underlying issue.
 - **F1** — harmonic mean of precision and recall; balances both equally. (Special case of Fβ at β=1.)
 - **Fβ** — generalized F-score with a tunable precision/recall trade-off:
@@ -40,7 +40,7 @@
 
 _CONSOLE_METRIC_EXPLANATIONS = (
     "[bold]Micro[/bold] — volume-weighted across all comments; tasks with many comments dominate.\n"
-    "[bold]Macro[/bold] — per-task P/R/F1 averaged equally; every task counts the same. Macro precision skips tasks with no comments so silence is not scored as perfect precision.\n"
+    "[bold]Macro[/bold] — per-task P/R/F1 averaged equally; every task counts the same. Macro precision rewards correct silence on negative tasks but not on positive ones; macro recall averages only over positive tasks.\n"
     "[bold]Matched comment[/bold] — paired by file + line proximity, then confirmed by an LLM judge to describe the same underlying issue.\n"
     "[bold]F1[/bold] — harmonic mean of precision and recall (special case of Fβ at β=1).\n"
     "[bold]Fβ[/bold] — F_β = (1 + β²) · (P · R) / (β² · P + R); β<1 favors precision, β>1 favors recall.\n"
@@ -361,12 +361,17 @@ def from_results(cls, results: Sequence[BaseEvaluationResult], run_id: str) -> "
         f_beta_05: float = f_beta_score(precision, recall, beta=0.5)
         f_beta_2: float = f_beta_score(precision, recall, beta=2.0)
 
-        # Precision is only defined where the agent actually commented. Silent tasks are assigned
-        # precision=1.0 by convention; averaging those in would reward saying nothing, so they are
-        # excluded from macro precision. Recall/F1 still span all tasks, so silence is penalized there.
-        results_with_comments = [r for r in code_review_results if r.generated_comments]
-        macro_precision: float = sum(r.precision for r in results_with_comments) / len(results_with_comments) if results_with_comments else 0.0
-        macro_recall: float = sum(r.recall for r in code_review_results) / total_results
+        # Precision is only defined where the agent actually commented, so silent tasks are normally
+        # excluded from macro precision (rewarding silence would inflate precision on positive tasks).
+        # Exception: negative tasks (no expected findings) ARE scored when silent — staying quiet is the
+        # correct answer and earns precision=1.0, while any comment is a false positive (precision=0).
+        results_for_macro_precision = [r for r in code_review_results if r.generated_comments or not r.expected_comments]
+        macro_precision: float = sum(r.precision for r in results_for_macro_precision) / len(results_for_macro_precision) if results_for_macro_precision else 0.0
+        # Recall measures the ability to find planted issues, so it only averages over positive tasks.
+        # Negative tasks (no expected findings) have a vacuous recall of 1.0 that would otherwise inflate
+        # the metric; when a run is all-negative there is nothing to recall, so recall defaults to 1.0.
+        positive_results = [r for r in code_review_results if r.expected_comments]
+        macro_recall: float = sum(r.recall for r in positive_results) / len(positive_results) if positive_results else 1.0
         macro_f1: float = sum(r.f1 for r in code_review_results) / total_results
         macro_f_beta_05: float = sum(r.f_beta_05 for r in code_review_results) / total_results
         macro_f_beta_2: float = sum(r.f_beta_2 for r in code_review_results) / total_results
diff --git a/tests/test_codereview.py b/tests/test_codereview.py
@@ -435,6 +435,56 @@ def test_macro_precision_zero_when_all_tasks_silent(self):
 
         assert summary.macro_precision == 0.0
 
+    def test_negative_task_silent_gets_full_score(self):
+        # A negative task (no expected findings) where the agent correctly stays silent scores perfectly.
+        silent_negative = create_codereview_result(instance_id="test__negative-1", output="[]", expected_comments=[])
+
+        assert silent_negative.precision == 1.0
+        assert silent_negative.recall == 1.0
+        assert silent_negative.f1 == 1.0
+
+        summary = CodeReviewResultSummary.from_results([silent_negative], run_id="run-1")
+
+        # Correct silence on a negative task is rewarded, not excluded, so macro precision is full.
+        assert summary.macro_precision == 1.0
+        assert summary.macro_f1 == 1.0
+
+    def test_negative_task_false_positive_penalized(self):
+        # Any comment on a negative task is a false positive and drives precision to zero.
+        noisy_negative = create_codereview_result(
+            instance_id="test__negative-2",
+            output=json.dumps([{"file": "src/app.al", "line_start": 10, "body": "Imagined issue", "severity": "warning"}]),
+            expected_comments=[],
+        )
+
+        assert noisy_negative.precision == 0.0
+        assert noisy_negative.f1 == 0.0
+
+        summary = CodeReviewResultSummary.from_results([noisy_negative], run_id="run-1")
+
+        assert summary.macro_precision == 0.0
+        assert summary.macro_f1 == 0.0
+
+    def test_macro_precision_full_when_all_negatives_silent(self):
+        # A batch of purely negative tasks with no false positives should report perfect macro precision,
+        # not the degenerate 0.0 that silent-positive exclusion would produce.
+        negatives = [create_codereview_result(instance_id=f"test__negative-batch-{i}", output="[]", expected_comments=[]) for i in range(3)]
+
+        summary = CodeReviewResultSummary.from_results(negatives, run_id="run-1")
+
+        assert summary.macro_precision == 1.0
+
+    def test_macro_recall_excludes_negative_tasks(self):
+        # Recall averages only over positive tasks; silent negatives must not dilute or inflate it.
+        expected_comments = [ReviewComment(file="src/app.al", line_start=10, body="Fix null check", severity=Severity.MEDIUM)]
+        positive_miss = create_codereview_result(instance_id="test__mix-pos-1", output="[]", expected_comments=expected_comments)
+        negatives = [create_codereview_result(instance_id=f"test__mix-neg-{i}", output="[]", expected_comments=[]) for i in range(3)]
+
+        summary = CodeReviewResultSummary.from_results([positive_miss, *negatives], run_id="run-1")
+
+        # Only the single positive task (a miss, recall 0.0) counts — the three negatives are ignored.
+        assert summary.macro_recall == 0.0
+
     def test_render_github_metrics_markdown_has_grouped_sections(self):
         expected_comments = [
             ReviewComment(file="src/app.al", line_start=10, body="Fix null check", severity=Severity.MEDIUM),
diff --git a/tests/test_hooks_operations.py b/tests/test_hooks_operations.py
@@ -1,4 +1,5 @@
 import json
+import shlex
 from pathlib import Path
 
 from bcbench.operations.hooks_operations import setup_hooks
@@ -102,3 +103,19 @@ def test_tool_log_path_is_absolute_in_env(self, tmp_path: Path):
         log_path = hooks_config["hooks"]["preToolUse"][0]["env"]["BCBENCH_TOOL_LOG"]
 
         assert Path(log_path).is_absolute()
+
+    def test_claude_command_escapes_shell_metacharacters(self, tmp_path: Path):
+        repo_path = tmp_path / "repo"
+        repo_path.mkdir()
+        output_dir = tmp_path / "results$(id)`whoami`"
+        output_dir.mkdir()
+
+        setup_hooks(repo_path, AgentType.CLAUDE, output_dir)
+
+        settings = json.loads((repo_path / ".claude" / "settings.local.json").read_text(encoding="utf-8"))
+        command = settings["hooks"]["PreToolUse"][0]["hooks"][0]["command"]
+
+        # shlex.quote wraps the path in single quotes so $() and backticks are inert.
+        assert command.startswith("BCBENCH_TOOL_LOG='")
+        assert 'BCBENCH_TOOL_LOG="' not in command
+        assert shlex.quote(str((output_dir / "tool_usage.jsonl").resolve())) in command
diff --git a/uv.lock b/uv.lock

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`import contextlib`
`2`	`2`	`import json`
	`3`	`+import shlex`
`3`	`4`	`from pathlib import Path`
`4`	`5`
`5`	`6`	`from bcbench.config import get_config`
`@@ -60,15 +61,16 @@ def _setup_claude_hooks(repo_path: Path, script_path: str, tool_log_path: Path)`
`60`	`61`	`with contextlib.suppress(json.JSONDecodeError):`
`61`	`62`	`existing_settings = json.loads(settings_file.read_text(encoding="utf-8"))`
`62`	`63`
`63`		`- tool_log_resolved = str(tool_log_path.resolve())`
	`64`	`+ tool_log_quoted = shlex.quote(str(tool_log_path.resolve()))`
	`65`	`+ script_path_quoted = shlex.quote(script_path)`
`64`	`66`	`existing_settings["hooks"] = {`
`65`	`67`	`"PreToolUse": [`
`66`	`68`	`{`
`67`	`69`	`"matcher": "",`
`68`	`70`	`"hooks": [`
`69`	`71`	`{`
`70`	`72`	`"type": "command",`
`71`		`- "command": f'BCBENCH_TOOL_LOG="{tool_log_resolved}" pwsh -ExecutionPolicy Bypass -File "{script_path}"',`
	`73`	`+ "command": f"BCBENCH_TOOL_LOG={tool_log_quoted} pwsh -ExecutionPolicy Bypass -File {script_path_quoted}",`
`72`	`74`	`}`
`73`	`75`	`],`
`74`	`76`	`}`