mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-10-21 01:15:11 +02:00
feat(cli): every crewai eval keeps the evaluation's full data in the project (#8326)
* feat(cli): every crewai eval keeps the evaluation's full data in the project
An evaluation's report opens for one five-minute sitting after it is first
read, and the verdict `crewai eval` waits for is that read. A coding agent
that wants a check's quotes, the outputs or the trace after its edits ran
`crewai eval export` and found it closed ("has been read already").
Right after the verdict, `crewai eval` now saves the export into
`.crewai/evaluations/<id>/` (report.md, run/checks.md, run/outputs.md,
run/trace.json, beside a `.gitignore` of `*`) and says where. Graded again
with nothing changed, it points at what it saved instead of asking AMP a
second time. Only inside a project; the ten newest are kept. A service that
does not export yet says nothing, and any other failure is one line: the
verdict and the exit code are never the save's.
The export writer now raises an error its callers decide about instead of
exiting, and writes into a nested folder the same way it wrote into one:
each folder opened inside the one before it, never through a link.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(cli): a saved evaluation is marked whole, pruned only by its owner, and fetched briefly
Review on #8326:
- the save asks AMP with its own short timeout (20 s to read) instead of
export's two minutes: the verdict is already printed, and CI waits on the
command (`PlusAPI.export_evaluation(..., timeout=)`);
- a marker is written last, and only when every file was; it, not report.md,
says an evaluation is saved, so a save cut short is tried again;
- a save that skipped a file says "Part of its data was saved … N skipped"
and leaves no marker, instead of claiming the full data;
- only a folder this command saved whole (the marker, a plain file) is ever
removed to keep the newest ten; never a link or a junction, and nothing at
all when `.crewai/` or `.crewai/evaluations/` is a link;
- the path writer (Windows) refuses a link anywhere on the folder's way
before making anything through it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(cli): AMP cannot send a saved evaluation's marker
Review on #8326: `_export_target` reserved the folder's `.gitignore` but not
the marker that says a save was whole. A payload carrying
`.crewai-eval-saved` and one file that is skipped would have left that
marker on a partial save: read as whole, never tried again, and kept over a
complete one. The marker is now reserved the same way, compared as a
case-insensitive filesystem that drops trailing dots sees it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
94508c971f
commit
a8877f0cdf
3 files changed
+425
-28
No files matched your search
@@ -40,6 +40,7 @@ import math
|
||||
import os
|
||||
from pathlib import Path
|
||||
import re
|
||||
import shutil
|
||||
import stat
|
||||
import sys
|
||||
import time
|
||||
@@ -396,6 +397,7 @@ def eval_crew(
|
||||
record_finished(finished)
|
||||
_print_verdict(finished, url)
|
||||
_print_brief(finished)
|
||||
save_evaluation(client, finished)
|
||||
_say_where_the_criteria_live(write_eval_config(finished))
|
||||
# The exit code is what a CI job reads, so it is the gate's: 0 only for a
|
||||
# run that PASSED. A failed gate, one without a verdict, and an evaluation
|
||||
@@ -2245,6 +2247,130 @@ _FOLDER_FLAGS = (
|
||||
_NEW_FILE_FLAGS = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0)
|
||||
|
||||
|
||||
# Every evaluation `crewai eval` reads keeps its full data in the project. The
|
||||
# report opens for one five-minute sitting after it is first read, and the
|
||||
# verdict this command waits for is that read: a coding agent that wants a
|
||||
# check's quotes or the trace an hour later, after its edits, would find it
|
||||
# closed. The newest few are kept; older ones are removed.
|
||||
SAVED_EVALUATIONS_DIR = "evaluations"
|
||||
SAVED_EVALUATIONS_KEPT = 10
|
||||
# Written last, and only when every file was: what says a folder was saved
|
||||
# whole by this command — so a save cut short is tried again, and only such a
|
||||
# folder is ever removed to keep the newest.
|
||||
SAVED_MARKER = ".crewai-eval-saved"
|
||||
# The verdict is already printed when the save starts, and one run's export is
|
||||
# small: a slow save must not hold a CI job or an agent for export's two minutes.
|
||||
SAVE_TIMEOUT = httpx.Timeout(10.0, read=20.0)
|
||||
|
||||
|
||||
def save_evaluation(client: PlusAPI, finished: dict[str, Any]) -> None:
|
||||
"""Save evaluation FINISHED's full data (the export) into
|
||||
`.crewai/evaluations/<id>/`, and say where.
|
||||
|
||||
Only inside a project, and never at the cost of the evaluation: a service
|
||||
that does not export yet says nothing, and any other failure is one line.
|
||||
A save cut short, or one that skipped a file, leaves no marker, says so,
|
||||
and is tried again the next time this evaluation is read.
|
||||
"""
|
||||
try:
|
||||
canonical = str(uuid.UUID(str(finished.get("id"))))
|
||||
except ValueError:
|
||||
return
|
||||
if not Path("pyproject.toml").is_file():
|
||||
return # `crewai eval --run ID` from anywhere: no project to keep it in
|
||||
from crewai.telemetry.tracing.last_run import LAST_RUN_DIR
|
||||
|
||||
folder = Path(LAST_RUN_DIR) / SAVED_EVALUATIONS_DIR / canonical
|
||||
if _saved_whole(folder):
|
||||
# Graded again with nothing changed: the same evaluation, saved when it
|
||||
# was first read — and its sitting may have closed since.
|
||||
console.print(Text(_saved_line(folder)), soft_wrap=True)
|
||||
return
|
||||
try:
|
||||
response = client.export_evaluation(canonical, timeout=SAVE_TIMEOUT)
|
||||
except Exception as error:
|
||||
console.print(Text(f"Its full data was not saved: {error}"), style="dim")
|
||||
return
|
||||
if response.status_code == 404:
|
||||
return # an evaluation service that does not export yet
|
||||
payload = _payload(response) or {}
|
||||
files = payload.get("files")
|
||||
if response.status_code != 200 or not isinstance(files, dict):
|
||||
said = payload.get("message")
|
||||
why = said if isinstance(said, str) and said.strip() else None
|
||||
why = why or f"AMP answered {response.status_code}."
|
||||
console.print(Text(f"Its full data was not saved: {why}"), style="dim")
|
||||
return
|
||||
try:
|
||||
_, skipped = _write_export(folder, files, marker=SAVED_MARKER)
|
||||
except ExportWriteError as error:
|
||||
console.print(Text(f"Its full data was not saved: {error}"), style="dim")
|
||||
return
|
||||
if skipped:
|
||||
plural = "" if skipped == 1 else "s"
|
||||
console.print(
|
||||
Text(
|
||||
f"Part of its data was saved in {folder}/: {skipped} file{plural} "
|
||||
"skipped (an unsafe path, or content that is not text)."
|
||||
),
|
||||
style="dim",
|
||||
soft_wrap=True,
|
||||
)
|
||||
return
|
||||
_keep_newest_saved_evaluations(folder.parent)
|
||||
console.print(Text(_saved_line(folder)), soft_wrap=True)
|
||||
|
||||
|
||||
def _saved_line(folder: Path) -> str:
|
||||
return f"Its full data is saved in {folder}/ — start with {EXPORT_START_HERE}."
|
||||
|
||||
|
||||
def _saved_whole(folder: Path) -> bool:
|
||||
"""Did this command save FOLDER whole? Its marker is there, a plain file."""
|
||||
try:
|
||||
return stat.S_ISREG((folder / SAVED_MARKER).lstat().st_mode)
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def _is_a_link(path: Path) -> bool:
|
||||
"""A symbolic link, or a Windows junction, which `is_symlink` misses."""
|
||||
if path.is_symlink():
|
||||
return True
|
||||
is_junction = getattr(path, "is_junction", None) # Python 3.12+
|
||||
return bool(is_junction and is_junction())
|
||||
|
||||
|
||||
def _keep_newest_saved_evaluations(folder: Path) -> None:
|
||||
"""Remove all but the SAVED_EVALUATIONS_KEPT newest evaluations this
|
||||
command saved whole: only a real folder named by an evaluation id with the
|
||||
marker in it — never a link, never a folder it did not write. Nothing is
|
||||
removed when FOLDER, or the `.crewai/` above it, is a link."""
|
||||
with contextlib.suppress(OSError):
|
||||
if _is_a_link(folder) or _is_a_link(folder.parent):
|
||||
return
|
||||
if os.path.normcase(folder.resolve()) != os.path.normcase(
|
||||
Path.cwd().resolve() / folder
|
||||
):
|
||||
return
|
||||
saved = []
|
||||
for entry in folder.iterdir():
|
||||
try:
|
||||
uuid.UUID(entry.name)
|
||||
except ValueError:
|
||||
continue
|
||||
if _is_a_link(entry) or not entry.is_dir() or not _saved_whole(entry):
|
||||
continue
|
||||
saved.append((entry.stat().st_mtime, entry))
|
||||
saved.sort(reverse=True)
|
||||
for _, old in saved[SAVED_EVALUATIONS_KEPT:]:
|
||||
shutil.rmtree(old, ignore_errors=True)
|
||||
|
||||
|
||||
class ExportWriteError(Exception):
|
||||
"""An export could not be written where it was going; the message says why."""
|
||||
|
||||
|
||||
def export_evaluation(evaluation_id: str) -> None:
|
||||
"""Write evaluation EVALUATION_ID's full data into `./crewai-eval-<id>/`.
|
||||
|
||||
@@ -2285,7 +2411,10 @@ def export_evaluation(evaluation_id: str) -> None:
|
||||
)
|
||||
|
||||
folder = Path(f"{EXPORT_DIR_PREFIX}{canonical}")
|
||||
written, skipped = _write_export(folder, files)
|
||||
try:
|
||||
written, skipped = _write_export(folder, files)
|
||||
except ExportWriteError as error:
|
||||
_fail(str(error))
|
||||
line = f"Wrote {len(written)} file{'' if len(written) == 1 else 's'} to {folder}/"
|
||||
if EXPORT_START_HERE in written:
|
||||
line += f" — start with {EXPORT_START_HERE}"
|
||||
@@ -2360,7 +2489,9 @@ def _export_target(name: str) -> list[str] | None:
|
||||
every segment a plain name (`[A-Za-z0-9._-]+`, never `.` or `..`). A `.git`
|
||||
segment is refused too: a `.git/config` there could name a command for the
|
||||
next `git status` in the folder to run, and a shell prompt runs one. The
|
||||
top `.gitignore` is this command's own. So is a name Windows keeps for a
|
||||
top `.gitignore` is this command's own, and so is the top SAVED_MARKER: a
|
||||
marker AMP sent would say a save was whole when it was not. So is a name
|
||||
Windows keeps for a
|
||||
device (`CON`, `PRN`, `AUX`, `NUL`, `COM0`-`COM9`, `LPT0`-`LPT9`, with any
|
||||
extension), on every platform, since the folder may be opened on Windows
|
||||
later or sit on a drive it shares: there `NUL.md` is the null device, and
|
||||
@@ -2379,18 +2510,26 @@ def _export_target(name: str) -> list[str] | None:
|
||||
or seen.split(".", 1)[0] in _WINDOWS_DEVICE_NAMES
|
||||
):
|
||||
return None
|
||||
if len(segments) == 1 and _as_a_filesystem_sees_it(segments[0]) == ".gitignore":
|
||||
if len(segments) == 1 and _as_a_filesystem_sees_it(segments[0]) in _RESERVED_TOP:
|
||||
return None
|
||||
return segments
|
||||
|
||||
|
||||
# The folder's own files, which no exported file may be: `.gitignore` keeps the
|
||||
# data out of a commit, the marker says the save was whole.
|
||||
_RESERVED_TOP = frozenset({".gitignore", SAVED_MARKER})
|
||||
|
||||
|
||||
def _as_a_filesystem_sees_it(segment: str) -> str:
|
||||
"""SEGMENT as macOS and Windows compare names: case ignored, trailing dots dropped."""
|
||||
return segment.rstrip(".").lower()
|
||||
|
||||
|
||||
def _write_export(folder: Path, files: dict[str, Any]) -> tuple[list[str], int]:
|
||||
def _write_export(
|
||||
folder: Path, files: dict[str, Any], *, marker: str | None = None
|
||||
) -> tuple[list[str], int]:
|
||||
"""Write FILES under FOLDER: the names written, and how many were skipped.
|
||||
MARKER, when given, is written last, and only when nothing was skipped.
|
||||
|
||||
The `.gitignore` goes first, so the data is never in the folder without it.
|
||||
A file whose path would go through a link, or through a file where a
|
||||
@@ -2404,10 +2543,12 @@ def _write_export(folder: Path, files: dict[str, Any]) -> tuple[list[str], int]:
|
||||
try:
|
||||
ignored = put([".gitignore"], b"*\n")
|
||||
except OSError as error:
|
||||
_fail(f"Could not write the export into {folder}/: {error}")
|
||||
raise ExportWriteError(
|
||||
f"Could not write the export into {folder}/: {error}"
|
||||
) from error
|
||||
if not ignored:
|
||||
# The data never goes into the folder without it.
|
||||
_fail(
|
||||
raise ExportWriteError(
|
||||
f"{folder}/.gitignore is a folder, not this command's file: remove it "
|
||||
"and export again, so the data is never committed by accident."
|
||||
)
|
||||
@@ -2424,11 +2565,20 @@ def _write_export(folder: Path, files: dict[str, Any]) -> tuple[list[str], int]:
|
||||
try:
|
||||
put_there = put(segments, data)
|
||||
except OSError as error:
|
||||
_fail(f"Could not write {folder}/{name}: {error}")
|
||||
raise ExportWriteError(
|
||||
f"Could not write {folder}/{name}: {error}"
|
||||
) from error
|
||||
if put_there:
|
||||
written.append(name)
|
||||
else:
|
||||
skipped += 1
|
||||
if marker is not None and not skipped:
|
||||
try:
|
||||
put([marker], b"Saved whole by `crewai eval`.\n")
|
||||
except OSError as error:
|
||||
raise ExportWriteError(
|
||||
f"Could not write {folder}/{marker}: {error}"
|
||||
) from error
|
||||
return written, skipped
|
||||
|
||||
|
||||
@@ -2454,24 +2604,38 @@ def _export_folder(folder: Path) -> Iterator[Callable[[list[str], bytes], bool]]
|
||||
|
||||
def _export_root_is_a_link(folder: Path) -> NoReturn:
|
||||
"""Stop: whoever made FOLDER a link chose where it leads, not this command."""
|
||||
_fail(f"{folder} is a link; the export writes only into a folder of its own.")
|
||||
raise ExportWriteError(
|
||||
f"{folder} is a link; the export writes only into a folder of its own."
|
||||
)
|
||||
|
||||
|
||||
def _export_root_handle(folder: Path) -> int:
|
||||
"""A handle on FOLDER, made when missing, opened inside the current
|
||||
directory and never through a link."""
|
||||
"""A handle on FOLDER, a path relative to the current directory: each of
|
||||
its folders made when missing and opened inside the one before it, never
|
||||
through a link."""
|
||||
try:
|
||||
here = os.open(".", _FOLDER_FLAGS)
|
||||
try:
|
||||
with contextlib.suppress(FileExistsError):
|
||||
os.mkdir(folder.name, dir_fd=here)
|
||||
return os.open(folder.name, _FOLDER_FLAGS, dir_fd=here)
|
||||
finally:
|
||||
os.close(here)
|
||||
handle = os.open(".", _FOLDER_FLAGS)
|
||||
except OSError as error:
|
||||
if folder.is_symlink():
|
||||
raise ExportWriteError(
|
||||
f"Could not write the export into {folder}/: {error}"
|
||||
) from error
|
||||
try:
|
||||
for part in folder.parts:
|
||||
with contextlib.suppress(FileExistsError):
|
||||
os.mkdir(part, dir_fd=handle)
|
||||
inner = os.open(part, _FOLDER_FLAGS, dir_fd=handle)
|
||||
os.close(handle)
|
||||
handle = inner
|
||||
except OSError as error:
|
||||
os.close(handle)
|
||||
if any(
|
||||
Path(*folder.parts[: n + 1]).is_symlink() for n in range(len(folder.parts))
|
||||
):
|
||||
_export_root_is_a_link(folder)
|
||||
_fail(f"Could not write the export into {folder}/: {error}")
|
||||
raise ExportWriteError(
|
||||
f"Could not write the export into {folder}/: {error}"
|
||||
) from error
|
||||
return handle
|
||||
|
||||
|
||||
def _put_through_handles(root: int, segments: list[str], data: bytes) -> bool:
|
||||
@@ -2528,14 +2692,18 @@ def _export_root_by_path(folder: Path) -> Path:
|
||||
`is_symlink` does not see, and which any account can make), would carry
|
||||
every file somewhere else.
|
||||
"""
|
||||
if folder.is_symlink():
|
||||
_export_root_is_a_link(folder)
|
||||
# Every folder on the way, before anything is made through one of them.
|
||||
for n in range(len(folder.parts)):
|
||||
if _is_a_link(Path(*folder.parts[: n + 1])):
|
||||
_export_root_is_a_link(folder)
|
||||
try:
|
||||
folder.mkdir(exist_ok=True)
|
||||
folder.mkdir(parents=True, exist_ok=True)
|
||||
root = folder.resolve()
|
||||
except OSError as error:
|
||||
_fail(f"Could not write the export into {folder}/: {error}")
|
||||
if os.path.normcase(root) != os.path.normcase(Path.cwd().resolve() / folder.name):
|
||||
raise ExportWriteError(
|
||||
f"Could not write the export into {folder}/: {error}"
|
||||
) from error
|
||||
if os.path.normcase(root) != os.path.normcase(Path.cwd().resolve() / folder):
|
||||
_export_root_is_a_link(folder)
|
||||
return root
|
||||
|
||||
|
||||
@@ -103,13 +103,16 @@ class PlusAPI(_CorePlusAPI):
|
||||
timeout=self.EVALUATION_POLL_TIMEOUT,
|
||||
)
|
||||
|
||||
def export_evaluation(self, evaluation_id: str) -> httpx.Response:
|
||||
def export_evaluation(
|
||||
self, evaluation_id: str, *, timeout: httpx.Timeout | None = None
|
||||
) -> httpx.Response:
|
||||
"""The evaluation's full data as files (crewai eval export):
|
||||
`{"id", "kind", "files": {relative path: text}}`."""
|
||||
`{"id", "kind", "files": {relative path: text}}`. TIMEOUT replaces the
|
||||
default, which allows the two minutes a large comparison may take."""
|
||||
return self._make_request(
|
||||
"GET",
|
||||
f"{self.EVALUATIONS_RESOURCE}/{evaluation_id}/export",
|
||||
timeout=self.EVALUATION_EXPORT_TIMEOUT,
|
||||
timeout=timeout or self.EVALUATION_EXPORT_TIMEOUT,
|
||||
)
|
||||
|
||||
def _make_multipart_request(
|
||||
|
||||
@@ -6,6 +6,7 @@ import contextlib
|
||||
from datetime import datetime, timedelta, timezone
|
||||
import json
|
||||
import os
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
@@ -2388,8 +2389,9 @@ class FakeExportAMP(FakeAMP):
|
||||
super().__init__()
|
||||
self.answer = answer if answer is not None else exported()
|
||||
|
||||
def export_evaluation(self, evaluation_id):
|
||||
def export_evaluation(self, evaluation_id, **kwargs):
|
||||
self.calls.append(("export", evaluation_id))
|
||||
self.export_kwargs = kwargs
|
||||
if isinstance(self.answer, Exception):
|
||||
raise self.answer
|
||||
return self.answer
|
||||
@@ -2546,6 +2548,8 @@ def test_exporting_again_overwrites_the_files(project, monkeypatch, capsys):
|
||||
".Git./config",
|
||||
".gitignore", # the folder's own
|
||||
".GITIGNORE",
|
||||
".crewai-eval-saved", # the saved evaluation's marker, never AMP's to send
|
||||
".CREWAI-EVAL-SAVED.",
|
||||
# names Windows keeps for its devices, with any extension
|
||||
"NUL.md",
|
||||
"nul",
|
||||
@@ -3460,6 +3464,208 @@ def test_a_project_that_turned_tracing_off_is_not_asked_to_trace_toward_an_untru
|
||||
assert "CREWAI_TRACING_ENABLED=false" in (directory / ".env").read_text()
|
||||
|
||||
|
||||
# ── every evaluation keeps its full data in the project ─────────────────────
|
||||
|
||||
SAVED = Path(".crewai") / "evaluations"
|
||||
|
||||
|
||||
def _verdict_of(evaluation_id: str, gate: str = "passed") -> httpx.Response:
|
||||
payload = done(gate=gate).json()
|
||||
return httpx.Response(200, json={**payload, "id": evaluation_id})
|
||||
|
||||
|
||||
def _graded(monkeypatch, directory: Path, answer=None, evaluation_id: str = EVALUATION_ID, gate: str = "passed"):
|
||||
(directory / "pyproject.toml").write_text("[project]\nname = 'demo'\n")
|
||||
amp = FakeExportAMP(answer=answer if answer is not None else exported(kind="trace"))
|
||||
amp.statuses = [_verdict_of(evaluation_id, gate)]
|
||||
return install(monkeypatch, amp)
|
||||
|
||||
|
||||
def test_the_verdict_leaves_the_evaluations_full_data_in_the_project(project, monkeypatch, capsys):
|
||||
"""The report opens for one five-minute sitting, and waiting for the verdict
|
||||
is that read: an agent that wants a check's quotes or the trace after its
|
||||
edits finds them here instead."""
|
||||
directory, _ = project
|
||||
amp = _graded(monkeypatch, directory)
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
saved = directory / SAVED / EVALUATION_ID
|
||||
assert files_in(saved) == {
|
||||
".gitignore": "*\n",
|
||||
**EXPORT_FILES,
|
||||
eval_module.SAVED_MARKER: "Saved whole by `crewai eval`.\n",
|
||||
}
|
||||
assert ("export", EVALUATION_ID) in amp.calls
|
||||
# its own short wait: the verdict is already out, and CI waits on the command
|
||||
assert amp.export_kwargs == {"timeout": eval_module.SAVE_TIMEOUT}
|
||||
said = " ".join(capsys.readouterr().out.split())
|
||||
assert f"Its full data is saved in {SAVED / EVALUATION_ID}/ — start with report.md." in said
|
||||
|
||||
|
||||
def test_a_service_that_does_not_export_yet_costs_nothing_and_says_nothing(project, monkeypatch, capsys):
|
||||
directory, _ = project
|
||||
_graded(monkeypatch, directory, answer=httpx.Response(404, json={"error": "not_found"}))
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
assert not (directory / SAVED).exists()
|
||||
assert "full data" not in capsys.readouterr().out
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"answer",
|
||||
[httpx.ConnectError("no route"), httpx.Response(500, json={"error": "boom"})],
|
||||
)
|
||||
def test_a_save_that_fails_is_one_line_and_never_the_verdicts(project, monkeypatch, capsys, answer):
|
||||
"""The exit code stays the gate's: a failed gate is still 1, a passed one 0."""
|
||||
directory, _ = project
|
||||
_graded(monkeypatch, directory, answer=answer, gate="failed")
|
||||
|
||||
with pytest.raises(SystemExit) as exited:
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
assert exited.value.code == 1 # the failed gate, not the save
|
||||
said = " ".join(capsys.readouterr().out.split())
|
||||
assert said.count("Its full data was not saved:") == 1
|
||||
assert "Goal gate: FAILED" in said
|
||||
|
||||
|
||||
def test_only_the_newest_saved_evaluations_are_kept(project, monkeypatch):
|
||||
directory, _ = project
|
||||
older = [str(uuid.UUID(int=n)) for n in range(1, 13)]
|
||||
for n, name in enumerate(older):
|
||||
folder = directory / SAVED / name
|
||||
folder.mkdir(parents=True)
|
||||
(folder / "report.md").write_text("old")
|
||||
(folder / eval_module.SAVED_MARKER).write_text("saved\n")
|
||||
os.utime(folder, (1_000_000 + n, 1_000_000 + n))
|
||||
(directory / SAVED / "notes").mkdir() # not an evaluation: never removed
|
||||
# named like an evaluation, but not saved by this command: never removed
|
||||
unmarked = str(uuid.UUID(int=99))
|
||||
(directory / SAVED / unmarked).mkdir()
|
||||
os.utime(directory / SAVED / unmarked, (1, 1))
|
||||
_graded(monkeypatch, directory)
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
kept = sorted(p.name for p in (directory / SAVED).iterdir())
|
||||
assert EVALUATION_ID in kept and "notes" in kept and unmarked in kept
|
||||
assert len([name for name in kept if name not in ("notes", unmarked)]) == eval_module.SAVED_EVALUATIONS_KEPT
|
||||
assert older[0] not in kept and older[-1] in kept # the oldest went first
|
||||
|
||||
|
||||
def test_outside_a_project_nothing_is_saved(project, monkeypatch):
|
||||
directory, _ = project
|
||||
amp = _graded(monkeypatch, directory)
|
||||
(directory / "pyproject.toml").unlink()
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
assert not (directory / ".crewai" / "evaluations").exists()
|
||||
assert ("export", EVALUATION_ID) not in amp.calls
|
||||
|
||||
|
||||
def test_graded_again_unchanged_it_points_at_what_it_saved_and_asks_nothing(project, monkeypatch, capsys):
|
||||
"""The same evaluation, read again after its sitting closed: what was saved
|
||||
the first time is where the data is, and AMP is not asked a second time."""
|
||||
directory, _ = project
|
||||
saved = directory / SAVED / EVALUATION_ID
|
||||
saved.mkdir(parents=True)
|
||||
(saved / "report.md").write_text("# Report\n")
|
||||
(saved / eval_module.SAVED_MARKER).write_text("saved\n")
|
||||
amp = _graded(monkeypatch, directory, answer=httpx.Response(410, json={"error": "report_closed", "message": "read already"}))
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
assert ("export", EVALUATION_ID) not in amp.calls
|
||||
assert "Its full data is saved in" in " ".join(capsys.readouterr().out.split())
|
||||
|
||||
|
||||
def test_a_refused_save_says_amps_own_words(project, monkeypatch, capsys):
|
||||
directory, _ = project
|
||||
message = f"Evaluation {EVALUATION_ID} has been read already. Evaluate the run again for a fresh report."
|
||||
_graded(monkeypatch, directory, answer=httpx.Response(410, json={"error": "report_closed", "message": message}))
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
assert f"Its full data was not saved: {message}" in " ".join(capsys.readouterr().out.split())
|
||||
|
||||
|
||||
def test_a_save_that_skipped_a_file_says_so_and_is_tried_again(project, monkeypatch, capsys):
|
||||
"""Missing checks or a missing trace must not read as the full data."""
|
||||
directory, _ = project
|
||||
files = {**EXPORT_FILES, "../outside.md": "no"}
|
||||
amp = _graded(monkeypatch, directory, answer=exported(files=files, kind="trace"))
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
said = " ".join(capsys.readouterr().out.split())
|
||||
assert f"Part of its data was saved in {SAVED / EVALUATION_ID}/: 1 file skipped" in said
|
||||
assert "Its full data is saved" not in said
|
||||
assert not (directory / SAVED / EVALUATION_ID / eval_module.SAVED_MARKER).exists()
|
||||
|
||||
amp.statuses = [_verdict_of(EVALUATION_ID)]
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
assert amp.calls.count(("export", EVALUATION_ID)) == 2 # not taken as saved
|
||||
|
||||
|
||||
def test_a_save_cut_short_after_the_report_is_tried_again(project, monkeypatch, capsys):
|
||||
"""report.md alone is not the evaluation saved: no marker, so the next
|
||||
read saves it again instead of pointing at half of it."""
|
||||
directory, _ = project
|
||||
amp = _graded(monkeypatch, directory)
|
||||
real = {name: getattr(eval_module, name) for name in ("_put_through_handles", "_put_by_path")}
|
||||
|
||||
def failing(name):
|
||||
def put(root, segments, data):
|
||||
if segments == ["run", "trace.json"]:
|
||||
raise OSError("disk full")
|
||||
return real[name](root, segments, data)
|
||||
return put
|
||||
|
||||
with monkeypatch.context() as patched:
|
||||
for name in real:
|
||||
patched.setattr(eval_module, name, failing(name))
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
said = " ".join(capsys.readouterr().out.split())
|
||||
assert "Its full data was not saved: Could not write" in said and "disk full" in said
|
||||
saved = directory / SAVED / EVALUATION_ID
|
||||
assert (saved / "report.md").is_file() and not (saved / eval_module.SAVED_MARKER).exists()
|
||||
|
||||
amp.statuses = [_verdict_of(EVALUATION_ID)]
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
assert amp.calls.count(("export", EVALUATION_ID)) == 2
|
||||
assert (saved / eval_module.SAVED_MARKER).is_file()
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name == "nt", reason="symbolic links need privileges on Windows")
|
||||
@pytest.mark.parametrize("linked", [".crewai", ".crewai/evaluations"])
|
||||
@pytest.mark.parametrize("through_handles", [True, False])
|
||||
def test_a_link_on_the_way_keeps_the_data_out_of_wherever_it_leads(
|
||||
project, monkeypatch, capsys, tmp_path_factory, linked, through_handles
|
||||
):
|
||||
"""Whoever made `.crewai` or `.crewai/evaluations` a link chose where it
|
||||
leads: nothing is written there, by either way of writing an export."""
|
||||
if through_handles and not eval_module._WRITES_THROUGH_HANDLES:
|
||||
pytest.skip("this platform writes by path")
|
||||
monkeypatch.setattr(eval_module, "_WRITES_THROUGH_HANDLES", through_handles)
|
||||
directory, _ = project
|
||||
elsewhere = tmp_path_factory.mktemp("elsewhere")
|
||||
if linked == ".crewai/evaluations":
|
||||
(directory / ".crewai").mkdir(exist_ok=True)
|
||||
(directory / linked).symlink_to(elsewhere, target_is_directory=True)
|
||||
_graded(monkeypatch, directory)
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
assert [p for p in elsewhere.rglob("*") if p.is_file()] == []
|
||||
said = " ".join(capsys.readouterr().out.split())
|
||||
assert "Its full data was not saved:" in said and "is a link" in said
|
||||
assert "Goal gate: PASSED" in said
|
||||
|
||||
|
||||
# ── with no terminal, the run's own output goes to a log, not the agent ────
|
||||
|
||||
|
||||
@@ -3652,3 +3858,23 @@ def test_putting_back_goes_on_past_a_failure_and_keeps_the_runs_own_error(tmp_pa
|
||||
finally:
|
||||
real_dup2(stdout_copy, 1) # the test puts stdout back itself
|
||||
os.close(stdout_copy)
|
||||
|
||||
|
||||
def test_a_marker_in_the_payload_never_marks_a_save_that_was_not_whole(project, monkeypatch, capsys):
|
||||
"""AMP cannot send the marker: with one file skipped, a forged marker would
|
||||
have made a partial save read as whole, never tried again and kept over a
|
||||
complete one."""
|
||||
directory, _ = project
|
||||
files = {**EXPORT_FILES, eval_module.SAVED_MARKER: "forged\n", "../outside.md": "no"}
|
||||
amp = _graded(monkeypatch, directory, answer=exported(files=files, kind="trace"))
|
||||
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
|
||||
saved = directory / SAVED / EVALUATION_ID
|
||||
assert not (saved / eval_module.SAVED_MARKER).exists()
|
||||
said = " ".join(capsys.readouterr().out.split())
|
||||
assert "Part of its data was saved" in said and "2 files skipped" in said
|
||||
|
||||
amp.statuses = [_verdict_of(EVALUATION_ID)]
|
||||
eval_module.eval_crew(run_id="named-run")
|
||||
assert amp.calls.count(("export", EVALUATION_ID)) == 2 # tried again
|
||||
Reference in new issue
Block a user