Skip to content

Commit bb03d3e

Browse files
committed
Harden evaluation harnesses
- isolate nested uv template invocations from the root virtualenv - checkpoint eval steps so chained commands start from a clean tree - handle byte output from timed-out subprocesses - ignore XML formatting tails in duplicate-registration detection - prune dependency dirs while walking generated projects - fail skill eval runs on process errors and baseline skill leaks - add fast unit tests for both harnesses
1 parent 447280e commit bb03d3e

8 files changed

Lines changed: 259 additions & 49 deletions

File tree

CHANGES.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@
33
## 7.0.0b15 (unreleased)
44

55

6-
- Nothing changed yet.
6+
- Allow the plonecli skill to start the development server when the user
7+
explicitly requests it.
8+
[MrTango]
9+
10+
- Harden scaffolding evaluation and remove false template Git warnings.
11+
[MrTango]
712

813

914
## 7.0.0b14 (2026-08-13)

evals/scaffolding/EVALUATION.md

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
# Scaffolding evaluation findings
22

3-
> Historical baseline from before the fixes. The current full report passes
4-
> 245/245 cases; see the ignored `results/report.md` generated on 2026-08-13.
3+
> Historical baseline from before the fixes. A full verification run passed
4+
> 245/245 cases with no warnings on 2026-08-13. The ignored
5+
> `results/report.md` is mutable and may instead contain the latest quick or
6+
> CI-validation run.
57
68
## Scope
79

@@ -61,7 +63,24 @@ Update the assertion and pytest target to the generated `src/<package>/tests/` l
6163
- Feature generation inside these nested, `--no-git` workspaces reports the outer plonecli repository as dirty. Git cleanliness checks should be scoped to the detected generated project rather than walking into an unrelated parent repository.
6264
- Keep the generated TOML/XML/Python validators as CI checks. They found failures that successful Copier exit codes did not detect.
6365

64-
## Test receipts
66+
## Resolution
67+
68+
All findings above have been addressed:
69+
70+
- free-text TOML values use serialization filters;
71+
- standalone Zope projects are detected correctly;
72+
- chained `create``setup` refreshes project context;
73+
- theme variants reject conflicting overlays;
74+
- the Barceloneta integration test uses the generated package test path;
75+
- context hooks use the current in-place API;
76+
- the deprecated command-alias dependency was removed;
77+
- Git checks are scoped to the generated project;
78+
- subtemplate validation tasks use Copier's `_copier_operation` value and no
79+
longer report files generated earlier in the same copy as pre-existing
80+
changes;
81+
- generated TOML/XML/Python validation runs in CI.
82+
83+
## Baseline test receipts
6584

6685
- Root unit suite: **209 passed, 16 skipped**.
6786
- Copier-template unit suite: **386 passed, 2 integration tests deselected**.

evals/scaffolding/run_evals.py

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -948,20 +948,32 @@ def build_cases(quick: bool) -> tuple[list[Case], dict[str, int]]:
948948
return cases, planned
949949

950950

951+
def _subprocess_text(value: str | bytes | None) -> str:
952+
"""Normalize subprocess output, including TimeoutExpired byte payloads."""
953+
if value is None:
954+
return ""
955+
if isinstance(value, bytes):
956+
return value.decode(errors="replace")
957+
return value
958+
959+
951960
def run_command(
952961
step: Step, env: dict[str, str], timeout_seconds: int
953962
) -> dict[str, Any]:
954-
command = (
955-
["uv", *step.args[1:]]
956-
if step.args and step.args[0] == "__uv__"
957-
else [*CLI, *step.args]
958-
)
963+
is_direct_uv = bool(step.args and step.args[0] == "__uv__")
964+
command = ["uv", *step.args[1:]] if is_direct_uv else [*CLI, *step.args]
965+
command_env = env
966+
if is_direct_uv:
967+
# The template checkout has its own uv project. Do not leak the root
968+
# project's active environment into that nested invocation.
969+
command_env = dict(env)
970+
command_env.pop("VIRTUAL_ENV", None)
959971
started = time.monotonic()
960972
try:
961973
completed = subprocess.run(
962974
command,
963975
cwd=step.cwd,
964-
env=env,
976+
env=command_env,
965977
text=True,
966978
capture_output=True,
967979
stdin=subprocess.DEVNULL,
@@ -973,8 +985,10 @@ def run_command(
973985
timed_out = False
974986
except subprocess.TimeoutExpired as exc:
975987
exit_code = 124
976-
stdout = exc.stdout or ""
977-
stderr = (exc.stderr or "") + f"\nTimed out after {timeout_seconds}s\n"
988+
stdout = _subprocess_text(exc.stdout)
989+
stderr = (
990+
_subprocess_text(exc.stderr) + f"\nTimed out after {timeout_seconds}s\n"
991+
)
978992
timed_out = True
979993
return {
980994
"command": command,
@@ -987,6 +1001,41 @@ def run_command(
9871001
}
9881002

9891003

1004+
def _checkpoint_project(project: Path, step_number: int) -> None:
1005+
"""Commit an intermediate eval step so the next command starts clean."""
1006+
status = subprocess.run(
1007+
["git", "status", "--porcelain"],
1008+
cwd=project,
1009+
check=True,
1010+
capture_output=True,
1011+
text=True,
1012+
stdin=subprocess.DEVNULL,
1013+
)
1014+
if not status.stdout.strip():
1015+
return
1016+
subprocess.run(
1017+
["git", "add", "-A"],
1018+
cwd=project,
1019+
check=True,
1020+
stdin=subprocess.DEVNULL,
1021+
)
1022+
subprocess.run(
1023+
[
1024+
"git",
1025+
"-c",
1026+
"user.name=Evaluation Runner",
1027+
"-c",
1028+
"user.email=eval@example.invalid",
1029+
"commit",
1030+
"-qm",
1031+
f"Evaluation step {step_number}",
1032+
],
1033+
cwd=project,
1034+
check=True,
1035+
stdin=subprocess.DEVNULL,
1036+
)
1037+
1038+
9901039
def execute_case(
9911040
case: Case, env: dict[str, str], timeout_seconds: int
9921041
) -> dict[str, Any]:
@@ -1048,7 +1097,7 @@ def execute_case(
10481097
commands = []
10491098
errors = []
10501099
observations: list[str] = []
1051-
for step in case.steps:
1100+
for step_number, step in enumerate(case.steps, 1):
10521101
step.cwd.mkdir(parents=True, exist_ok=True)
10531102
result = run_command(step, env, timeout_seconds)
10541103
commands.append(result)
@@ -1062,6 +1111,8 @@ def execute_case(
10621111
if result["exit_code"] != 0:
10631112
errors.append(f"command {len(commands)} exited {result['exit_code']}")
10641113
break
1114+
if case.project and step_number < len(case.steps):
1115+
_checkpoint_project(case.project, step_number)
10651116

10661117
validation: dict[str, list[str]] = {}
10671118
if case.project and case.project.exists():

evals/scaffolding/validators.py

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,28 @@
22

33
from __future__ import annotations
44

5+
import os
56
import tomllib
67
import xml.etree.ElementTree as ET
78
from collections import Counter
9+
from collections.abc import Iterable
810
from pathlib import Path
911

1012
IGNORED_PARTS = {".git", ".venv", "node_modules", "__pycache__"}
1113
XML_SUFFIXES = {".xml", ".zcml"}
1214

1315

1416
def _files(root: Path):
15-
for path in root.rglob("*"):
16-
if path.is_file() and not (set(path.parts) & IGNORED_PARTS):
17-
yield path
17+
"""Yield project files while pruning generated dependency directories."""
18+
for directory, dirnames, filenames in os.walk(root):
19+
dirnames[:] = [name for name in dirnames if name not in IGNORED_PARTS]
20+
base = Path(directory)
21+
yield from (base / name for name in filenames)
1822

1923

20-
def validate_toml(root: Path) -> list[str]:
24+
def validate_toml(root: Path, files: Iterable[Path] | None = None) -> list[str]:
2125
errors: list[str] = []
22-
for path in _files(root):
26+
for path in files if files is not None else _files(root):
2327
if path.suffix != ".toml":
2428
continue
2529
try:
@@ -30,9 +34,9 @@ def validate_toml(root: Path) -> list[str]:
3034
return errors
3135

3236

33-
def validate_xml(root: Path) -> list[str]:
37+
def validate_xml(root: Path, files: Iterable[Path] | None = None) -> list[str]:
3438
errors: list[str] = []
35-
for path in _files(root):
39+
for path in files if files is not None else _files(root):
3640
if path.suffix not in XML_SUFFIXES:
3741
continue
3842
try:
@@ -42,9 +46,9 @@ def validate_xml(root: Path) -> list[str]:
4246
return errors
4347

4448

45-
def validate_python(root: Path) -> list[str]:
49+
def validate_python(root: Path, files: Iterable[Path] | None = None) -> list[str]:
4650
errors: list[str] = []
47-
for path in _files(root):
51+
for path in files if files is not None else _files(root):
4852
if path.suffix != ".py":
4953
continue
5054
try:
@@ -55,25 +59,47 @@ def validate_python(root: Path) -> list[str]:
5559
return errors
5660

5761

58-
def detect_duplicate_xml_registrations(root: Path) -> list[str]:
62+
def _element_identity(element: ET.Element, cache: dict[int, tuple]) -> tuple:
63+
"""Build a hashable subtree identity once, excluding formatting tails."""
64+
key = id(element)
65+
if key not in cache:
66+
cache[key] = (
67+
element.tag,
68+
tuple(sorted(element.attrib.items())),
69+
element.text,
70+
tuple(_element_identity(child, cache) for child in element),
71+
)
72+
return cache[key]
73+
74+
75+
def detect_duplicate_xml_registrations(
76+
root: Path, files: Iterable[Path] | None = None
77+
) -> list[str]:
5978
"""Find exact repeated direct-child registrations in generated XML.
6079
6180
Exact element identity is deliberately conservative: it catches hooks that
6281
append the same registration twice without treating similar, valid
6382
registrations as duplicates.
6483
"""
6584
errors: list[str] = []
66-
for path in _files(root):
85+
for path in files if files is not None else _files(root):
6786
if path.suffix not in XML_SUFFIXES:
6887
continue
6988
try:
7089
tree = ET.parse(path)
7190
except (OSError, ET.ParseError):
7291
continue
92+
cache: dict[int, tuple] = {}
7393
for parent in tree.iter():
74-
serialized = [ET.tostring(child, encoding="unicode") for child in parent]
75-
for element, count in Counter(serialized).items():
94+
children_by_identity = {
95+
_element_identity(child, cache): child for child in parent
96+
}
97+
counts = Counter(_element_identity(child, cache) for child in parent)
98+
for identity, count in counts.items():
7699
if count > 1:
100+
element = ET.tostring(
101+
children_by_identity[identity], encoding="unicode"
102+
)
77103
preview = " ".join(element.split())[:160]
78104
errors.append(
79105
f"duplicate XML registration ({count}x) "
@@ -84,9 +110,10 @@ def detect_duplicate_xml_registrations(root: Path) -> list[str]:
84110

85111
def validate_project(root: Path) -> dict[str, list[str]]:
86112
"""Run all deterministic, install-free generated-project checks."""
113+
files = tuple(_files(root))
87114
return {
88-
"toml": validate_toml(root),
89-
"xml": validate_xml(root),
90-
"python": validate_python(root),
91-
"duplicate_xml": detect_duplicate_xml_registrations(root),
115+
"toml": validate_toml(root, files),
116+
"xml": validate_xml(root, files),
117+
"python": validate_python(root, files),
118+
"duplicate_xml": detect_duplicate_xml_registrations(root, files),
92119
}

evals/skill/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ python evals/skill/run_evals.py --mode both --cases restapi-implicit,upgrade-ste
4141
python evals/skill/run_evals.py --model haiku # cheaper smoke run
4242
```
4343

44-
Requires the `claude` CLI logged in. Runs bill real model usage — a full
45-
`--mode both` sweep is ~16 agent runs. Sandboxes and transcripts land in
44+
Requires the `claude` CLI logged in. Runs bill real model usage. Sandboxes and transcripts land in
4645
`<tmpdir>/plonecli-skill-evals/<timestamp>/` (outside the repo on purpose:
4746
a sandbox inside this repo lets the baseline agent *find* the skill by
48-
searching the project); `results.json` there summarizes. Each run prints
47+
searching the project); `results.json` there summarizes. A full `--mode both`
48+
sweep is 24 agent runs. Each run prints
4949
`skills fired: [...]` — in `noskill` mode it must be `none`, anything else
5050
means a skill leaked into the baseline.
5151

0 commit comments

Comments
 (0)