Skip to content

examples: add auditable evaluation optimization loop#244

Open
Wsp030914 wants to merge 9 commits into
trpc-group:mainfrom
Wsp030914:feat/eval-optimize-loop
Open

examples: add auditable evaluation optimization loop#244
Wsp030914 wants to merge 9 commits into
trpc-group:mainfrom
Wsp030914:feat/eval-optimize-loop

Conversation

@Wsp030914

@Wsp030914 Wsp030914 commented Jul 26, 2026

Copy link
Copy Markdown

This change adds an Evaluation and Optimization closed-loop example based on AgentEvaluator, AgentOptimizer, and TargetPrompt.

The example evaluates training and validation datasets, attributes failed cases, optimizes a system prompt, compares baseline and candidate results per case, and applies configurable acceptance gates for score improvement, hard failures, critical regressions, overfitting, cost, and duration.

It provides fake-model, fake-judge, and trace modes so the complete pipeline can run without a real model API key. JSON and Markdown reports record baseline and candidate scores, per-case deltas, failure attribution, gate decisions, costs, timing, and reproducibility metadata.

The example includes three training cases and three validation cases covering successful optimization, ineffective optimization, and candidate regression. Source prompts are only updated when write-back is explicitly enabled and the candidate passes the gate.

Tests:
Docker Python 3.12 target tests: 18 passed.
Target coverage: 92%.
YAPF and flake8 checks passed.

Fixes #91

RELEASE NOTES: Added an auditable Evaluation and Optimization example with offline replay, prompt optimization, regression gates, and structured reports.

Add an auditable baseline, optimization, replay, and gate pipeline with offline fake and trace modes.\n\nFixes trpc-group#91
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

很好,预期内容已填充。现在我有足够的上下文了。让我把剩余特定的担忧整理一下。让我再重新检查一个关键问题:_maybe_write_back 使用了 bundle.prompt_path(源路径),并且 TargetPrompt().add_path(PROMPT_KEY, str(bundle.prompt_path))accepted && write_back 时写回。但是 bundle.prompt_pathoptions.paths.prompt_path.resolve()。对于默认的 run_pipeline,这是 agent/prompts/system.md(源文件)。如果传入 --write-back 且 gate 接受,它会覆盖源文件。这就是预期的行为。没问题。

现在让我检查一个真正的正确性问题:在 _run_pipeline 中,_evaluate_pair 是通过 _evaluate_pair(workspace, ...) 调用的,其中 workspace 是 prompt_path。但在 _run_pipeline 第 1318 行,它调用了 _evaluate_pair(workspace, bundle, options, "baseline")。签名是 _evaluate_pair(prompt_path, bundle, options, phase)。很好。

等等——在 _optimize_evaluate_candidate 中,prompt_path 被直接使用。但在 _run_pipeline 中,它也调用 _optimize(workspace, ...),其中 workspace 是工作 prompt 的副本。对于真实模式,_optimize 调用 TargetPrompt().add_path(PROMPT_KEY, str(prompt_path)),使用的是工作副本路径,而不是源路径。很好——优化器读取的是工作副本。并且 update_source=False。很好。

现在关键点是:_optimize 通过 prompt_path.read_text 读取 baseline_prompt。然后对于真实模式,在优化器运行后,candidate = result.best_prompts。优化器是从工作副本运行的,所以 best_prompts 是优化后的提示词。然后 _evaluate_candidate 将其写入工作副本并进行评估。很好。

现在让我检查一个隐蔽的 Bug:_evaluate_candidate 在 finally 块中从变量 baseline(这是工作副本的原始内容)恢复 prompt_path。很好。但是如果在 _evaluate_pair 内部发生异常,finally 会恢复。很好——这就是 test_candidate_prompt_is_restored_after_replay 所测试的内容。

现在让我看看 test_candidate_prompt_is_restored_after_replay 是否真正测试了源恢复与工作副本恢复。它读取的是源文件 ROOT/agent/prompts/system.md。但流水线操作的是工作副本(位于 output_dir/work/prompts/system.md),而不是源文件。所以源文件从未被修改过——这个测试实际上是同义反复(测试总是通过,因为流水线从没动过源文件)。真正的恢复测试应该是针对工作副本的。这是测试有效性问题。让我核实一下:_prepare_workspace 将源文件复制到 output_dir/work/prompts/system.md,所有操作都发生在工作副本上。源文件 agent/prompts/system.md 从未被 _run_pipeline 触碰(除非 --write-back)。所以测试 test_candidate_prompt_is_restored_after_replay 验证了源文件 == 源文件,实际上什么都没测到。值得发出警告。

让我再检查一下 _failure_resultstarted_wall 的计算逻辑——使用了 datetime.now()(脚本中允许使用)。

现在检查 agent.py 第 149 行的生成器优先级——已确认没问题。

让我再验证一下 _has_tool_mismatch_has_argument_mismatch 的顺序——参数检查仅在工具名称匹配时触发。但 _has_argument_mismatch 返回 bool(actual or expected) and actual != expected。如果工具名称匹配,但参数列表不同,则触发。没问题。但请注意 attribute_case 在工具检查之前调用了 _has_tool_mismatch_has_argument_mismatch,这需要 case.actual/expected 项拥有 .tool_calls_tool_names(invocation) 使用了 invocation.tool_callsInvocationSnapshot 具有 tool_calls: list[dict])。_has_argument_mismatch 中的 item.tool_calls 是字典列表。没问题。

现在,让我验证一下关于 compare_snapshotsSplitDelta.split = baseline.split 的具体担忧。在 _gate_input 测试中,train_deltavalidation_delta 都是由同一个比较构建的。这只是测试问题。在实际流水线中,没问题。

让我检查 _evaluate_pair 的签名与调用——在 _run_pipeline 中是 _evaluate_pair(workspace, bundle, options, "baseline")。但 _evaluate_candidate 调用的是 _evaluate_pair(prompt_path, bundle, options, "candidate")。签名 (prompt_path, bundle, options, phase)。是一致的。很好。

现在让我检查 _make_call_agent 是否为真实模式创建了新的闭包,但 real_call_agent 在每次调用时都会创建一个新的 agent/session——没问题。

关键审查项:_failure_result 报告了 started_at,但使用了 time.monotonic() 的差值来计算 started_wall——没问题。

我深入看一个潜在的真实 Bug:evaluate_split 捕获了 AssertionError 并仅在结果为 None 时重新抛出。但 _EvaluationCasesFailed 是在 self._result 设置之后引发的(第 221 行设置结果,第 231 行引发)。因此,当部分用例失败时,即使结果存在,也会引发 AssertionError。代码捕获并保留了结果——这是为了保留部分结果的预期行为。很好。但注意:如果结果为 None,它会重新抛出 AssertionError,这会传播到 run_pipelineexcept Exception_failure_result。没问题。

现在让我核实一下 README/DESIGN 中关于 TRPC_AGENT_API_KEY 等的说明——_create_agent 直接读取 os.environ["TRPC_AGENT_API_KEY"](如果缺失会抛出 KeyError)。对于真实模式,这是可以接受的。没问题。

现在看 optimizer.json${TRPC_AGENT_API_KEY} 等是通过优化器的 _expand_env 扩展的。所以真实模式优化使用了环境变量。很好。但 agent.py 中的 _create_agent 使用的是 os.environ[...](直接索引,会抛出 KeyError)。不一致,但不是 Bug。

让我看看一个真正令人担忧的区域:_snapshot_result 通过 result.results_by_eval_set_id.values() 进行分组,然后是 set_result.eval_results_by_eval_id.items()。在多个文件/集合的情况下,按 eval_id 分合可能会合并不同的集合。但此处每个拆分只使用一个数据集文件。没问题。

现在,_portable_dataset_path 返回 os.path.relpath(path, Path.cwd())。如果 path 不在当前工作目录(cwd)下,relpath 会返回类似 ../../... 的路径。SDK 随后会将相对于 cwd 的路径加载。评论说为了避免 Windows 冒号问题。在 Linux 上没问题。如果文件在 cwd 之外,relpath 仍然有效。OK。但存在一个微妙的问题:_dataset_for_sdk 首先尝试 _portable_dataset_path,只有在引发 OSError/ValueError 时才会回退到复制。对于 cwd 之外的路径,os.path.relpath 会成功并返回 ../ 路径——这没问题。没问题。

现在让我核实最具体的怀疑点——候选评估提示词的恢复以及测试。让我读一下流水线测试第 2781-2786 行。确实,它断言的是源文件未改变。由于源文件从未被修改,该测试是通过的,但没有验证恢复逻辑。中等价值的警告。

我还想检查 _collect_attributions 是否仅根据 baseline(而非候选者)进行归因。设计说明中写道“优化阶段... 归因”。实际上 attribute_cases 是在 baseline 快照上调用的(第 1335 行 _collect_attributions(baseline))。DESIGN.md 说“先分别评测... 再按... 进行确定性归因” —— 归因是基于 baseline 失败进行的。鉴于 baseline 是确定要修复的故障,这是说得通的。不是 Bug。

现在看 optimization_report.json 的产物文件——这是提交进仓库的预生成报告。它包含了时间戳和 python_version: 3.14.5 以及 git_sha。这是一个提交的产物,会导致 CI 产生差异,但测试并未对其进行比对。小问题。实际上,提交生成的报告是不良做法(陈旧、环境特定)。这是一个建议。

让我核实一下是否存在路径遍历/注入问题。run_pipeline.py 的参数是路径,但仅用于本地文件 I/O。_git_sha 运行了 subprocess.run(["git","rev-parse","HEAD"])——没有 shell,是固定的参数列表。安全。

现在让我看看报告的 markdown 文件权限 chmod 0o600——没问题。

让我最后检查一个正确性问题:evaluate_gateoverfitting 检查使用了 train_delta.score_deltavalidation_delta.score_delta,这些是拆分级别的 deltas(拆分平均分数的差值)。在 _score_check 中,value = delta.score_delta 即验证拆分的 delta。很好。过度拟合 = 训练提升且验证退化。在报告中,训练 delta=0(未提升),所以没有过度拟合。没问题。

现在看 test_gate_rejects_score_regression_and_overfitting:baseline 和 candidate 分数都是 0.0 → delta=0.0,min_score_delta=0.1 → _score_check 失败(0 + epsilon >= 0.1 为假)。已拒绝。reasons[0] 包含“validation delta”。很好。

我认为主要的发现是测试有效性的警告。我还要确认 test_validate_inputs_rejects_split_content_leakage —— 复制了带有重复 eval_id 的 train case(内容与 train case 相同),放入 validation 中。overlap 检查:train_ids 和 validation_ids —— duplicate 的 eval_id 是 "duplicate",与 train id 没有重叠。所以 id-overlap 不会捕获它。但是内容哈希重叠会捕获:重复的 case 内容 == train case 内容 → 哈希重叠 → ValueError("normalized case content overlaps")。测试匹配的是 "overlap"。很好。等等,测试创建 duplicate = train.eval_cases[0].model_copy(update={"eval_id": "duplicate"}),然后将其放入 validation 中。所以 train 有原始 case,validation 有重复项。内容哈希匹配 → 引发 "normalized case content overlaps across splits"。测试 match="overlap"。很好。

但等等——_case_hash 从 payload 中弹出 eval_id 等,所以哈希是基于内容的。train case 和重复项内容相同 → 哈希相同 → 重叠。很好。测试有效。

现在让我核实一下是否存在关键问题:validate_inputs 在第 716 行的 input_paths = tuple(paths.model_dump().values())InputPaths 有 5 个路径字段。model_dump() 返回字典;.values() 保留顺序。没问题。然后在第 729 行对 input_paths 进行哈希处理 → 哈希字典使用 path.name 作为键。如果有两个路径共享同一个文件名,哈希就会发生碰撞。在这里:system.md, train.evalset.json, val.evalset.json, optimizer.json, gate.json —— 全部唯一。没问题。但脆弱。不是 Bug。

让我看看一个真正可疑的地方:_failure_result 创建了一个 seed=91 和空 baseline/candidate/delta 的报告。OptimizationReport 要求 baseline: dict[SplitName, EvaluationSnapshot](默认为空字典)。空字典没问题。audit.input_hashes={}。没问题。测试 test_pipeline_failure_is_reported_without_prompt_write 期望状态为 REJECTED 且存在失败记录。很好。

现在,关于 return 0 if result.report.status in {"ACCEPTED", "REJECTED"} else 1 —— 失败也会返回 REJECTED 状态 → 退出代码 0。所以流水线失败在 CLI 中返回 0(成功)。这可能会掩盖 CI 中的失败。让我评估一下:run_pipeline 将异常转换为 REJECTED 报告,并且 main() 对 REJECTED 返回 0。因此,损坏的流水线(例如缺少文件、优化器崩溃)向 shell 返回退出代码 0。在 CI 中,测试 test_cli_fake_mode_runs_without_api_key 使用了 check=True,这要求返回代码 0 —— 它会通过的。但对于真实运行,崩溃的流水线会报告成功。这是一个关于正确性/CI 的担忧。让我检查一下 CLI 测试是否运行实际流水线。test_cli_fake_mode_runs_without_api_key 运行 fake 模式 → ACCEPTED/REJECTED → 退出 0。没问题。但对于 --mode real 出错时,它会静默返回 0。这是一个 Warning(CI/行为问题)。

实际上,失败运行返回 0 是有问题的:用户无法通过退出代码检测流水线是否失败。设计预期是报告驱动的,但从 CI/脚本角度来看,main 应该在出现意外失败时返回非零值。值得提出 Warning。

让我完成最终确认。## 发现的问题

🚨 Critical

  • examples/optimization/eval_optimize_loop/run_pipeline.py:2392:pipeline 失败时 CLI 仍返回 0,会掩盖 CI/脚本失败
    • run_pipeline 将所有异常吞掉转为 status="REJECTED" 报告(loop/pipeline.py:1310),而 main()ACCEPTED/REJECTEDreturn 0。因此真实模式崩溃、输入缺失、优化器异常等都会以退出码 0 退出,CI 中无法据退出码判定失败。建议对 REJECTED(尤其是含 failures/异常路径)或 _failure_result 产生的报告返回非零退出码,至少区分"评测拒绝"与"管线异常"。

⚠️ Warning

  • tests/evaluation/test_eval_optimize_loop_pipeline.py:2781-2786:恢复测试断言的是源文件,实际从未被修改,测试恒为真

    • _prepare_workspace 把源 prompt 复制到 output_dir/work/prompts/system.md,后续写/恢复都发生在工作副本上(loop/pipeline.py:1533-1537, 1525-1530),源文件 agent/prompts/system.md 从不被改动(除非 --write-back)。该测试读取源文件做前后比较,无法验证 _evaluate_candidatefinally 恢复逻辑真正生效。建议改为断言工作副本路径 output_dir/work/prompts/system.md 在运行后内容回到 baseline。
  • examples/optimization/eval_optimize_loop/optimization_report.json:1736-2281:提交预生成的报告产物,含环境相关字段

    • 该 545 行 JSON 是一次本地运行的结果,固化了 python_versiongit_shastarted_at/finished_atduration_seconds 等环境与时间相关字段,且不在任何测试断言中。它会随环境漂移造成无意义 diff,也容易被误当作权威产物。建议从仓库移除(加入 .gitignore)或改为由测试生成到临时目录。
  • examples/optimization/eval_optimize_loop/agent/agent.py:128-130:real 模式用 os.environ[...] 直接取 key,缺失时抛 KeyError 而非可读错误

    • optimizer.json${TRPC_AGENT_API_KEY}expandvars 展开的路径不一致;当环境变量缺失时 KeyError 会被 run_pipeline 吞为通用 REJECTED 报告,掩盖根因。建议用 os.getenv 并给出明确的报错信息。

💡 Suggestion

  • examples/optimization/eval_optimize_loop/loop/evaluation.py:763-766:捕获 AssertionError 仅在 get_result()None 时重抛,依赖 SDK 用 AssertionError 子类(_EvaluationCasesFailed)表达"部分 case 失败但仍有结果"这一隐式契约。建议加注释说明该契约,避免后续 SDK 改用其他异常类型时静默丢失部分失败信息。

总结

整体设计严谨、边界处理(hard failure、过拟合、回写原子性)较完备;存在一处应修复的 Critical:CLI 对异常/拒绝一律返回 0 会掩盖 CI 失败。其余为测试有效性与产物提交相关 Warning,建议一并处理。

测试建议

  • 补充 CLI 退出码测试:构造必然失败(如 --mode real 缺 key 或缺失数据集)的运行,断言退出码非 0。
  • test_candidate_prompt_is_restored_after_replay 改为断言工作副本 prompt 被正确恢复,而非源文件。

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@6a2f7f9). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             main        #244   +/-   ##
==========================================
  Coverage        ?   88.18123%           
==========================================
  Files           ?         482           
  Lines           ?       45157           
  Branches        ?           0           
==========================================
  Hits            ?       39820           
  Misses          ?        5337           
  Partials        ?           0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

write_back=args.write_back,
)))
print(f"{result.report.status}: {result.json_path}")
print(result.markdown_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

管线失败时 CLI 仍返回退出码 0

run_pipeline 把所有异常吞为 status="REJECTED" 报告,而 main() 对 ACCEPTED/REJECTED 一律 return 0。真实模式崩溃、输入缺失、优化器异常等都以退出码 0 退出,CI 无法据退出码判定失败。建议对 _failure_result 产生的报告或含 failures 的 REJECTED 返回非零退出码。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我已经完成了审查,以下是我的审查结论。

发现的问题

🚨 Critical

  • examples/optimization/eval_optimize_loop/loop/pipeline.py:143-149:写入报告与回写源 prompt 顺序错误,导致审计报告失真
    • _run_pipeline 先调用 write_reports(report, ...)(此时 report.source_updated 为构造时传入的 False),之后才执行 _maybe_write_back(...) 并把结果赋给 report.source_updated。因此落盘的 optimization_report.json / .md 始终记录 source_updated=false,即使 --write-back 已实际改写源 prompt。该字段是审计闭环的关键凭据(DESIGN 明确要求报告记录是否回写),与实际行为不一致会造成误判。修复方向:先执行 _maybe_write_back 得到 source_updated,再 write_reports,或回写成功后重新写一次报告。

⚠️ Warning

  • examples/optimization/eval_optimize_loop/loop/pipeline.py:415-419_build_reportAuditInfo.num_runs 硬编码为 1,未读取 config.evaluate.num_runs

    • 审计信息应反映真实运行参数;当 optimizer.json 调整 num_runs 后,报告中的 num_runs 仍为 1,属于可审计性/可复现性缺陷。建议从加载的 config 取值(evaluate_split 已加载 config,可回传或重读)。
  • examples/optimization/eval_optimize_loop/loop/analysis.py:436attribute_case 中 RESPONSE 归因硬编码 "final_response_avg_score"

    • 该判定使用字面量指标名而非 case 所属 snapshot 的 primary_metric,一旦复用到其它主指标配置,RESPONSE 类别永远不会命中、统一落到 OTHER,归因失真。建议通过参数传入 primary_metric 或从 case 上下文获取。当前示例配套配置下不触发,但与“可复用、可审计”的设计目标相悖。

💡 Suggestion

  • examples/optimization/eval_optimize_loop/loop/evaluation.py:770-773_dataset_for_sdk 的 copy 回退仅在 os.path.relpath 抛错时触发,且拷贝文件落在 Path.cwd() 下而非临时目录;若 executor.evaluate() 抛非 AssertionError 异常,cleanup() 不会执行,会在 cwd 残留 .eval-optimize-* 文件。Linux 下 relpath 基本不抛错故难触发,但建议把拷贝文件放进已有的 tempfile.TemporaryDirectory 内,使其随 with 自动清理,消除残留风险。

总结

整体逻辑链(评测→归因→逐 case diff→gate→回写)设计严谨、失败即闭合(fail-closed)语义清晰,测试覆盖了过拟合、hard fail、critical 退化等关键路径。存在一个必须修复的审计正确性问题:报告写入早于源回写,导致 source_updated 恒为 false;另有审计字段硬编码与归因硬编码指标名两处可维护性隐患。

测试建议

  • 补充一条 --write-back 且 gate 接受的集成测试,断言落盘 optimization_report.jsonsource_updated 与源 prompt 是否被实际更新一致(覆盖当前顺序 bug 的回归)。
  • 补充一条 num_runs>1 的配置用例,校验报告中 audit.num_runs 与 config 一致。

model_name=options.model_name,
num_runs=1,
case_parallelism=options.case_parallelism,
python_version=platform.python_version(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

报告写入早于源回写,source_updated 恒为 false

_run_pipeline 先调用 write_reports(此时 source_updated 仍为构造时的 False),之后才执行 _maybe_write_back 并赋值给 report.source_updated。导致落盘的 optimization_report.json / .md 始终记录 source_updated=false,即使 --write-back 已实际改写源 prompt,审计闭环失真。修复方向:先执行 _maybe_write_back 得到 source_updatedwrite_reports,或回写成功后重写报告。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我已经审查了完整的 pr.diff,并对照了 SDK 源码(trpc_agent_sdk/evaluation/_agent_evaluator.py_target_prompt.py_eval_case.py)进行了验证。以下是我的审查结论。

发现的问题

⚠️ Warning

  • examples/optimization/eval_optimize_loop/loop/pipeline.py:1601-1611(以及 run_pipeline.py:2404):在 fake/trace 模式下开启 --write-back 会把合成标记写回真实源 prompt 文件,而非工作副本。
    • _maybe_write_back 写入的是 bundle.prompt_path(即 validate_inputspaths.prompt_path.resolve(),默认指向仓库内检入的 agent/prompts/system.md),不是 _prepare_workspace 生成的工作副本。在 fake-model 默认模式下 gate 会接受(baseline validation 1/3 → candidate 2/3,delta≈0.333 > 0.1),因此执行 run_pipeline.py --write-back 会把 "\n\nOPTIMIZED_CANDIDATE\n" 持久化写入检入的源文件,污染示例自身的 prompt 且不可通过 gate 含义恢复(合成的"优化"并非真实改进)。建议在 fake/trace 模式下直接拒绝 --write-back,或将写回目标限制为工作副本/输出目录内的文件。

💡 Suggestion

  • examples/optimization/eval_optimize_loop/loop/evaluation.py:772-778evaluate_split 用裸 except AssertionError 兜底以保留部分 case 结果。这确实是 SDK _EvaluationCasesFailed(AssertionError) 的契约,但当 get_result() 恰好非 None 时,也会吞掉 call_agent 或 SDK 内部抛出的无关 AssertionError。可在 re-raise 分支之外记录或限定捕获范围(例如仅在 result 非空时取结果、否则原样抛出),目前逻辑已做 result is None → raise 兜底,影响有限,可选优化。

总结

整体逻辑稳健:失败保留、归因、gate fail-closed、split 泄漏校验、trace 离线回放与 prompt 还原都覆盖到位,与 SDK 契约一致。未发现必须修复的阻塞性问题;主要风险是 fake/trace 模式下 --write-back 会把合成标记写回检入的源 prompt 文件,建议在该路径加模式守卫。

测试建议

  • 补充一条用例:在 fake-model 模式下传入 --write-back 并将 --prompt 指向临时副本,断言源 prompt 未被写入 OPTIMIZED_CANDIDATE(或断言在该模式下 --write-back 被拒绝),以锁定期望的写回边界。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

The sandbox is blocking these commands. Let me just review the longer lines I've already seen visually. I have enough context now. Let me check a couple of the longer lines I noticed:

  • Line 661 (analysis.py _contains_any with Iterable): def _contains_any(value: str, markers: Iterable[str]) -> bool: - short.
  • Line 230 evaluation _invocation_snapshots signature with trailing comma - short.
  • Line 154-155 hard_failure_added=bool(candidate is None ... - spans 2 lines, fine.

Let me verify the _fake_judge_score ordering issue more carefully — this is the most concrete correctness concern.

Looking at _fake_judge_score (evaluation.py:984-991):

def _fake_judge_score(runs):
    scores = []
    for run in runs:
        actual = _invocation_snapshots([run])[0]
        expected = _invocation_snapshots([run])[1]
        scores.append(float(bool(actual and expected and actual[0].final_text == expected[0].final_text)))
    return _mean_optional(scores)

_invocation_snapshots([run]) returns (actual_list, expected_list). So [0] = actual_list, [1] = expected_list. Then actual[0] and expected[0] — accessing first element. If a run has no eval_metric_result_per_invocation, both lists are empty, so actual[0] would IndexError. But bool(actual and expected and ...) — short-circuit: if actual is empty list, actual and ...actual (empty, falsy), so actual[0] is never evaluated. OK, this is safe due to short-circuit. But note _deduplicate_invocations is applied, so for a single run it's fine.

Now let me finalize the review. Let me verify the _failure_result started_wall calculation once more — it uses datetime.now() which is fine (not in workflow). Good.

Let me check one more potential issue: the _hard_failure_check metric comparison case.metric_statuses.get(metric) != "PASSED" — when metric_statuses doesn't contain the metric (None), None != "PASSED" is True → counts as failure. That's fail-closed, acceptable.

Now let me also verify: in _score_check, passed = value is not None and value + SCORE_EPSILON >= config.min_score_delta. If validation score_delta is None (e.g., all hard failures → primary_score None), gate fails. Good fail-closed.

Let me write the final review.

Based on my thorough analysis, here is my review:

发现的问题

🚨 Critical

无。

⚠️ Warning

  • examples/optimization/eval_optimize_loop/loop/evaluation.py:106-113:捕获 AssertionError 后仅在 get_result() is None 时重抛,会静默吞掉 SDK 用 AssertionError 表达的非“部分 case 失败”语义错误。
    • SDK 已将 case 失败专用异常细化为 _EvaluationCasesFailed(AssertionError) 子类(trpc_agent_sdk/evaluation/_agent_evaluator.py:78),其 docstring 明确建议调用方捕获该具体子类,否则会“误吞 numpy assert allclose 等无关 AssertionError”。本处用裸 except AssertionError 且依赖 get_result() 是否为 None 来判断,逻辑脆弱:一旦 _run 在赋值 self._result(line 221)之前因其他 AssertionError 抛出,get_result() 返回 None 才会重抛;但若异常发生在 self._result 赋值之后、raise 之前的其他断言路径,结果会被误当作“部分 case 失败”而吞掉。建议改用 except _EvaluationCasesFailed: 精确捕获,或至少 except _EvaluationCasesFailed as exc: if executor.get_result() is None: raise,避免把无关 AssertionError 当作预期失败。

💡 Suggestion

  • examples/optimization/eval_optimize_loop/loop/pipeline.py:132-145_write_back_and_report):回写失败时先 write_text(original)raise,与 TargetPrompt.write_all 自身的原子回滚(_target_prompt.py:135-160)职责重叠。
    • SDK 的 write_all 已对 path 字段做 tmp+os.replace 原子写并在失败时回滚到 pre-call 内容;此处再读一次原文并手动还原属于双重回滚,正常路径无副作用,但在 write_all 自身回滚也失败(抛 _RollbackError)时,bundle.prompt_path 的最终状态可能与 original 不一致而此处仍按成功回滚处理。建议信任 SDK 的原子性,或在注释中说明该兜底仅作为额外保险,并考虑记录 SDK 抛出的 _RollbackError 以便审计。

总结

整体实现严谨、fail-closed 语义到位,未发现必须修复的阻塞性问题。主要风险集中在 evaluation.py 对 SDK AssertionError 的宽泛捕获,建议按 SDK 推荐精确捕获 _EvaluationCasesFailed

测试建议

  • 建议补充一个用例:构造 call_agent 抛出 _EvaluationCasesFailedAssertionError(例如在评分阶段触发 assert allclose 失败),断言 evaluate_split 将其重抛而非吞掉并返回部分结果,以覆盖 evaluation.py:106-113 的异常筛选路径。
  • 建议为 _write_back_and_report 补充一个 write_all_RollbackError(回滚失败)场景的用例,验证 prompt 源文件的最终状态与报告 source_updated 字段是否符合预期。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查完成。已读取 pr.diff 全部 2985 行(含 17 个新增文件),并结合仓库上下文核对了关键调用链。下面是结论。

发现的问题

⚠️ Warning

  • examples/optimization/eval_optimize_loop/run_pipeline.py:46-56:real 模式下审计字段 model_name 始终为 fake-model

    • PipelineOptions.model_name 默认 "fake-model"main() 构造时未传入任何值,_run_pipeline 也只覆盖 num_runs/case_parallelism,而真正模型名来自 _create_agent 读取的 TRPC_AGENT_MODEL_NAME。因此 real 模式生成的报告 audit.model_name 仍是 fake-model,破坏可复现/溯源字段的正确性。建议在 real 模式下用 os.getenv("TRPC_AGENT_MODEL_NAME", "real") 填充,或新增 CLI 参数透传。
  • examples/optimization/eval_optimize_loop/loop/evaluation.py:104:临时目录强制建在 Path.cwd(),对只读 cwd 不健壮

    • tempfile.TemporaryDirectory(prefix="eval-optimize-", dir=Path.cwd()) 假定当前工作目录可写;CI 容器或只读仓库根时会直接失败,且运行期间会在仓库根留下临时目录。配合 _dataset_for_sdkos.path.relpath(path, Path.cwd()) 传给 SDK,整个链路隐式依赖 cwd=可写仓库根。建议改用 tempfile.gettempdir()options.output_dir,并让 _dataset_for_sdk 的拷贝兜底真正生效(当前 os.path.relpath 几乎不抛异常,拷贝分支不可达)。

💡 Suggestion

  • examples/optimization/eval_optimize_loop/optimization_report.json:534-539:提交了带环境相关字段的生成产物
    • 该 545 行示例报告内含 git_shapython_versionstarted_at/finished_at 等环境相关值,会随时间失效。虽然 DESIGN.md 声明其为示例输出、测试不依赖,但长期维护易产生误导。建议加入 .gitignore 或在文件首行注明“示例,运行时重新生成”。

总结

整体设计较稳健:fail-closed 的 gate、working-copy 隔离、write-back 仅限 real 模式且显式开关、SDK 异常按子类区分处理,均无明显阻塞性缺陷。存在 2 个建议修复项:real 模式审计 model_name 字段错误,以及临时目录依赖可写 cwd,均非 CI 阻塞但影响真实模式可用性与溯源正确性。

测试建议

  • 补充一条 real 模式 model_name 透传的测试:构造 PipelineOptions(mode="real", model_name=...) 并断言 report.audit.model_name 与传入值一致(当前 test_report_audit_uses_configured_num_runs 只覆盖了 num_runs)。
  • 补充 cwd 不可写或 dataset 在 cwd 之外时的 evaluate_split/_dataset_for_sdk 路径测试,验证拷贝兜底能正常生效。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

这确认了 EvalConfig.model_validate_json 是从 metrics 文件加载的,并且转储的 EvalConfig(包含 metrics)是有效的。很好 — 这很正确。

现在让我验证一下最重要的担忧:转储的 config 包含 num_runsmetrics,但 by_alias=True 将字段名转为了 camelCase。EvalConfig.metrics 项是 Any 字典(不是 pydantic 模型),所以 model_dump_json(by_alias=True) 会按原样转储它们 —— 但原始的 optimizer.json 使用的是 snake_case 键 metric_namefinal_response 等。等等 —— config.evaluate.metricsOptimizeConfigFile 中被解析为什么类型? 让我检查一下 OptimizeConfigFile.evaluate 是否为 EvalConfig,以及 metrics 是否为原始字典列表。如果是这样,转储时会保留 snake_case 键,并且 EvalConfig.get_eval_metrics 可以同时处理 metricNamemetric_name。没问题。

但这有一个实际的细微差别:metrics_path.write_text(config.evaluate.model_dump_json(by_alias=True)) 转储了 num_runs 和可能的 user_simulator_configEvalConfig 接受这些。没问题。这不是 bug。

现在让我检查一下 PR 没有暴露的 _sdk_version / __version__。没问题。

让我检查最后一个真正的担忧:_consume_final_text 中的生成器表达式结合 extend

output.extend(part.text or "" for part in event.content.parts or [] if not part.thought)

运算符优先级:for ... if not part.thoughtparts or [] 先求值,然后 for part in (parts or []) 使用 if not part.thought 进行过滤。所以 part 是正确的。part.text or ""。这可以正确过滤掉 thought。没问题。

现在让我看看是否有实际的 Bug。让我重新检查 _optimize 的真实模式参数:它传递了 verbose=0。SDK 签名接受 verbose: int = 1。没问题。update_source=False。很好 — PR 在之后通过 _maybe_write_back 进行了自己的回写。

现在有一个真正的问题:_run_pipeline 中,真实模式下的 _optimize 调用 _make_call_agent(prompt_path, options.mode),其中 prompt_path=workspace(工作副本)。 但是 SDK AgentOptimizer.optimize 也接收 target_prompt=TargetPrompt().add_path(PROMPT_KEY, str(prompt_path)) —— 同样的工作副本路径。优化器将提示词文件原地修改(因为 update_source=False,它不会持久化到源文件,但可能会在优化期间写入工作副本)。实际上 update_source=False 意味着它不会写回。但是优化器在内部读取提示词,使用候选词进行评估,并保持最佳状态。在 update_source=False 的情况下,它不应该修改工作副本文件。让我确认这是否安全。候选评估随后会写入 prompts[PROMPT_KEY](来自 result.best_prompts)并进行评估。没问题。

让我检查候选评估 + 优化器之间的交互:优化器以真实模式运行真实评估,该评估会修改提示词工作副本。 如果 update_source=False,优化器就不会触碰提示词文件。好的,问题不大。

现在让我汇总实际的审查发现。让我重新检查 _hard_failure_check 以查看一个潜在的实际问题:当 inputs.validation_delta.cases 不包含候选评估中的所有用例时(例如,候选数据集用例较少)。hard_failure_added 涵盖了这一点。没问题。

再检查一下 —— _collect_attributions 仅对基准快照进行归因baseline[split] 失败的用例)。测试 test_fake_pipeline_writes_auditable_json_and_markdown 检查 payload["audit"]["input_hashes"]。归因仅针对基准失败。根据设计,这是可以接受的。

现在让我仔细审视一个真正的正确性顾虑:_score_check 允许 min_score_delta 为负数,测试 test_gate_rejects_regression_even_with_negative_minimum_delta 证明了当验证集回归时,即使 min_score_delta=-1.0_regression_check 也会独立拒绝。这是设计使然(两个独立的检查)。没问题。

让我检查一下 evaluate_gate 返回 accepted=not reasons —— 如果没有失败的原因,则接受。但过拟合检查被附加到 checks 中,然后 reasons = [check.reason for check in checks if not check.passed]。所以过拟合确实被包含在拒绝原因中。并且 accepted = not reasons。很好 —— 过拟合会导致拒绝。

现在是我想要确认的真正 Bug:在 _write_back_and_report 中,当回写成功时,report.source_updated = True,然后调用 write_reports(report, ...)。但报告对象随后被返回给 _run_pipeline,后者在调用 write_reports 之后从 report 构建了…… 等等,不是。在 _run_pipeline 中,_build_reportsource_updated=False 构建报告,然后 _write_back_and_report 可能会在原地修改 report.source_updated = True。然后 PipelineResult(report=report, ...)。所以最终报告的 source_updated=True。但写入磁盘的 JSON 是在 write_reports(report) 调用期间写入的,它反映了修改后的 source_updated=True。很好 —— 磁盘和返回的报告中是一致的。

但有一个细微的问题:如果 _maybe_write_back 成功,但随后的 write_reports 失败(引发异常)怎么办?那么 report.source_updated=True,但 write_reports 引发了异常 → 被传播到 run_pipeline 的外层 try 块 → _failure_result。此时提示词源文件已被修改(回写成功),但失败报告显示 source_updated 未设置(新报告)。提示词已更新,但报告显示 REJECTED/失败。这是一个轻微的审计不一致,但 _failure_result 不会回滚提示词。然而,这只会影响报告写入失败的情况(例如,磁盘权限),这种情况极少。最多算作一个小的警告。

现在让我做最后的总结。让我重新检查一个看起来确实有问题的地方:_failure_result 计算了 started_wall,但使用了 datetime.now() 两次(隐式地 —— 在 _failure_result 中为一次)。实际上在 _failure_result 中:finished = datetime.now().astimezone() 一次,然后 started_wall = finished - timedelta(...)。是一致的。

让我重新考虑安全问题_git_sha 使用参数列表运行 subprocess.run(["git","rev-parse","HEAD"]) —— 没有注入风险。很好。没有用户输入流入 shell。路径来自 CLI,但使用 Path 类型处理 —— 没有用于执行命令的 shell 扩展。

现在,_trace_dataset_path 使用 json.loads 然后写入 —— 如果 trace 文件格式错误,会引发异常,并被捕获为流水线故障。没问题。

让我检查测试 test_cli_pipeline_failure_returns_nonzero —— 使用了丢失的训练文件。validate_inputs 引发 FileNotFoundError_failure_result → 报告失败 → main 返回 1。很好。

现在让我验证一个真正的问题:run_pipeline.py 中的 main() 使用 mode=args.mode 但没有传递 num_runs/case_parallelism —— 它们默认为 1 并被覆盖。没问题。但 model_name 是通过 _model_name 设置的。在 fake-model 模式下,返回 "fake-model"。PipelineOptions.model_name 默认为 "fake-model" 无论如何。没问题。

我已经进行了彻底的分析。现在让我识别出真正值得报告的问题。老实说,这段代码相当健壮。让我重新考虑一下到底有没有什么实际的 Bug。

让我重新检查 _snapshot_casehard_failurefake_judge 的顺序问题 —— 这里有一个真正的问题

hard_failure = not runs or scores.get(primary) is None or metric_not_evaluated or any(
    run.final_eval_status == EvalStatus.NOT_EVALUATED or run.error_message for run in runs)
if fake_judge and scores[primary] is not None:
    statuses[primary] = (...)
    passed = scores[primary] >= primary_threshold
passed = passed and not hard_failure

fake_judge=Truescores[primary]_fake_judge_score 中不为 None 时,hard_failurescores.get(primary) is None 子句为 False。但 hard_failure 仍然可以通过 metric_not_evaluatedrun.error_message 为 True。在那种情况下,passed = passed and not hard_failure = False。很好。但是 statuses[primary] 被设置为 PASSED/FAILED,而忽略了 hard_failure。所以一个 hard_failure 用例可能会将 statuses[primary]=PASSED 同时 passed=False。然后归因:case.hard_failure or case.error_message → 归因为 EXECUTION。没问题。

这感觉不是个 Bug。设计使然。

让我考虑一下是否存在并发/异步问题evaluate_split 使用了 tempfile.TemporaryDirectory 并且在 with 块内等待 executor.evaluate()。评估会读取数据集文件。文件在块内存在。很好。但是 _dataset_for_sdk 返回一个相对于 cwd 的路径。如果 cwd 不是仓库根目录,os.path.relpath 可能会产生错误的相对路径,但 SDK 接受相对路径或绝对路径。实际上它先返回 relpath,在回退时返回 str(local)(绝对路径)。两者都有效。

等等 —— 这里有一个潜在的真正问题_portable_dataset_path 执行了 resolved.relative_to(Path.cwd().resolve()),如果不在 cwd 下则引发 ValueError,然后回退将文件复制到临时目录。在正常流程中(从仓库根目录运行),数据集在 cwd 下,因此它返回一个相对路径。但 evaluate_split 在临时上下文中传递了 request.dataset_path —— 对于 trace 模式,数据集路径是 output_dir / "work" / "trace" / ...,它可能在 cwd 下也可能不在。如果不在,它会复制到 temp_dir(另一个临时目录)并返回相对路径。功能上没问题。

我现在相当有信心代码是正确的。让我寻找真正可以报告的项目 —— 顶级的真正问题。

再检查一次 —— _evaluate_pair 签名不匹配:让我非常仔细地重读。定义:

async def _evaluate_pair(prompt_path: Path, bundle: InputBundle, options: PipelineOptions, phase: str,)

调用:baseline = await _evaluate_pair(workspace, bundle, options, "baseline")。workspace 是一个 Path。✓。并且 _evaluate_candidate 调用 _evaluate_pair(prompt_path, bundle, options, "candidate")。✓。

现在 _optimize 签名(prompt_path: Path, bundle, options, primary_metric),调用方式为 _optimize(workspace, bundle, options, gate_config.primary_metric)。✓。del primary_metric —— 参数被接受但被删除了。没问题。

让我检查一下是否存在未使用的 primary_metric 意图问题 —— 它被删除了。这是一个小的代码坏味道,但无害。不是真正的问题。

让我看看**_build_reportdelta 参数类型为 dict[SplitName, Any]**,但传入了 deltasdict[SplitName, SplitDelta])。报告模型期望 delta: dict[SplitName, SplitDelta]。赋值是兼容的。没问题。

让我验证最后一个真正的担忧:示例 optimization_report.json 是一个包含 545 行的提交文件。设计文档说它是示例输出,而不是稳定的契约。但测试并没有断言其内容(它们重新运行流水线)。因此,如果它与实际输出不同步,提交的示例会产生误导,但不会破坏 CI。次要问题。

现在,让我做出最终决定。老实说,我没有发现明确的“Critical”问题。让我重新考虑值得提出的“Warning”级别的问题:

  1. _evaluate_candidate 仅在发生异常时恢复提示词,但如果进程被终止(SIGKILL),提示词工作副本将保留候选内容。 但这只是 output_dir/work/ 中的工作副本,而不是源文件。源文件仅由 _maybe_write_back(原子操作)触及。因此,即使工作副本被损坏,源文件也是安全的。不是真正的问题。

  2. _write_back_and_report 回写后的异常处理程序:如果 SDK write_all 成功,但随后的 write_reports 引发异常,处理程序会捕获该异常,将提示词恢复为 original(原始基准),重新引发异常。但 SDK write_all 已经原子地写入了候选词。因此,恢复操作会将源文件回滚到基准。这是预期的行为(故障安全)。但是 report.source_updated = False 是在重新引发之前设置的 —— 然而异常传播到 _failure_result,它会创建一个全新的报告。所以故障报告中 source_updated 为 False。但是源文件已被恢复。所以是一致的。不是 bug。

等等,实际上这里有一个细微的问题:在 _write_back_and_report 中,如果 write_reports 引发异常,except 块会将 bundle.prompt_path.write_text(original) 恢复为基准。但 original 是在回写之前读取的。所以源文件被恢复。很好。但请注意 —— SDK write_all 已经是原子的;这个额外的恢复是双重保险。没问题。

  1. 真正值得提出的担忧:在 _failure_result 中,AuditInfo.input_hashes={}。但是 _run_pipeline 已经在失败前通过 validate_inputs 构建了 bundle —— 等等,不。如果失败发生在 validate_inputs 期间(例如缺少文件),则没有 bundle。但如果失败发生在 validate_inputs 之后(例如在评估期间),则存在 bundle,但 _failure_result 忽略了它并报告空的哈希。这是审计保真度方面的一个小缺陷。次要问题 —— 警告级别。

实际上,这是一个合理的警告:在早期输入验证之后发生的流水线故障会丢失已计算的输入哈希和部分审计跟踪。但报告仍然会被写入。问题不大。

  1. _restrict_permissions 在报告文件上执行 chmod 0o600 —— 但 output_dir 本身和中间工件(work/optimizer/)并没有受到限制。只有这两个报告文件是 0o600。报告可能包含候选提示词内容(在 audit.input_hashes.candidate_prompt 中 —— 那只是一个哈希,而不是内容)。实际上,报告确实包含了 optimization.best_prompts,即完整的候选提示词文本。因此,报告可能包含敏感的提示词 IP,并且文件是 0o600(好的),但 output_dir/work 下的工作副本(包含提示词)并没有受到限制。次要问题。

让我重新考虑什么才是真正有影响力的。让我检查**_failure_reason 是否泄露敏感信息** —— 对于 _RollbackError,它包含了 error(回滚失败细节)。对于其他异常,只包含类型名。所以没有凭据泄露。很好。

现在让我检查最后一个真正的问题 —— OptimizationReport.baseline: dict[SplitName, EvaluationSnapshot] 在失败情况下为空render_markdown 遍历 report.delta.items(),对于失败情况,它是空的 → 没有 Scores 行。没问题。_pass_rate 返回 "-"。没问题。

好的。让我做决定。我认为没有明确的 Critical 问题。有几个值得提出的 Warning 级别问题。让我把最重要的几个整理出来。

实际上,让我重新审视一个看起来确实是真正 Bug 的问题:_change_kind 中 IMPROVED/REGRESSED 与 UNCHANGED 在 None 分数情况下的逻辑。

baseline_passed==candidate_passed==False(两者都失败)且 baseline_score=None, candidate_score=None(两者都严重失败)时:delta = _score_delta(None,None) = Nonedelta is None → UNCHANGED。但是 hard_failure_added 会单独跟踪。这没问题 —— 这是一个没有分数变化的失败用例。很好。

baseline_passed=True, candidate_passed=True,且分数相同 → UNCHANGED。很好。

现在确实有一个微妙的情况:_case_delta 使用 baseline.metric_scores.get(primary_metric),其中 primary_metric 是基准的 primary_metric。但候选对象可能使用不同的 primary_metric 吗?不会 —— primary_metric 是固定的。在 compare_snapshots 中,baseline.primary_metric 被使用。但 candidate.primary_metric 应该是相同的(相同的配置)。没问题。

好的,让我检查最具体的一个问题:compare_snapshots 使用 baseline.split 作为结果 SplitDelta.split,并假设候选对象具有相同的 split。 如果候选对象有不同的 split,那就不对了。但它们是相同的 split(两者都针对同一个 split 进行评估)。没问题。

让我再验证一个 —— _hard_failure_checkconfigured 列表也捕获了 hard_case_ids,这些 id 可能通过 validate_inputs 进行了验证,要求属于验证集。 很好。

我相信没有 Critical 问题。让我检查测试 test_evaluate_split_keeps_failed_case_result 断言 snapshot.primary_score == pytest.approx(0.0)。对于 VALIDATION 上的失败代理,所有用例都失败了(fake_judge=False),所以 primary_score = 指标平均值。_failing_agent 返回 {"queue":"unknown"},没有任何预期匹配 → 所有分数=0 → 平均值=0。但是 hard_failurescores.get(primary) = 0(不是 None),metric_not_evaluated:指标是以 exact match 评估的 → 状态为 FAILED 而不是 NOT_EVALUATED。run.final_eval_status — 将是 FAILED 而不是 NOT_EVALUATED。run.error_message — 无。所以 hard_failure=False。因此 primary_score = metrics.get(primary) = 0.0。✓ 符合断言。很好。

很好 —— 证实了正确性。现在让我写下审查意见。我将报告几个 Warning 级别的项目,它们是真实存在且可定位的。

让我再重新考虑一下 _failure_result 丢失 bundle 哈希的问题是否真的是 diff 中的问题 —— 是的,_failure_result 在 pipeline.py 的 diff 中,并且总是设置 input_hashes={},即使在失败前 bundle 可用时也是如此。这是一个真正的审计缺失。但可以说对于一个示例来说是可以接受的。Warning。

让我再重新考虑一下 _write_back_and_report 异常处理程序的正确性:它捕获 Exception(而不是 BaseException)。target.write_all 引发 _RollbackError(RuntimeError → Exception 的子类)。所以会被捕获。但 KeyboardInterrupt 不会。没问题。

还有一件事 —— 在 _write_back_and_report 中,恢复操作执行 bundle.prompt_path.write_text(original)。但 original 是在 _maybe_write_back 之前读取的。如果 _maybe_write_back 部分写入(但 SDK 是原子的),则不需要恢复。但额外的恢复操作直接写入(非原子)—— 可能使文件处于半写入状态。但这是在异常路径上。次要问题 —— 可接受的故障保险。

让我重新考虑是否存在重复回写风险_write_back_and_report 首先检查 if not report.gate.accepted or not options.write_back: return write_reports(...)。然后在 _maybe_write_back 中再次检查。没问题,只是多余而已。

最终决定。让我再验证一个潜在的实际问题 —— models.pyOptimizationReport.baseline: dict[SplitName, EvaluationSnapshot] 没有 default_factory —— 这是一个必填字段。在 _failure_result 中,传递了 baseline={}dict[SplitName, EvaluationSnapshot] 接受 {}。没问题(空的 dict 是有效的)。✓

好的,让我写下审查意见。我将报告:

  • 一个关于 _failure_result 丢弃可用输入哈希/部分审计的 Warning(审计保真度)。
  • 可能有一个关于工作副本工件没有像报告文件那样受到权限限制的 Warning(提示词 IP 存储在 output_dir/work/ 下,且为 0o644)。
  • 一个关于提交的 optimization_report.json 示例可能不同步的 Suggestion。

实际上,让我重新考虑权限那个问题 —— 提示词“IP”仅仅是系统提示词“你是一个支持队列分类器”(3 行)。并不敏感。报告中的 best_prompts 包含了候选词。工作副本是临时的。这是一个演示。权限警告很薄弱。我将其降级或省略。

让我重新考虑在上下文中什么才是真正有价值的。查看指令:仅报告可验证的问题。不要堆砌低价值的建议。要直接。

让我重新考虑 _failure_result 哈希丢失是否值得报告。在失败路径中,_run_pipeline 在 try 块内有 bundle,但异常传播到 run_pipeline 的外层 try 块,该块调用 _failure_result(options, started, exc) —— 注意它只传递了 options,而不是 bundle。所以 bundle 在作用域内但被丢弃了。这确实是已计算的输入哈希的审计丢失。对于一个明确关注审计性(DESIGN.md 中强调了“可审计”)的示例来说,失败报告丢失输入哈希是一个真正的缺陷。我会将其报告为 Warning。

让我验证在失败路径中 bundle 是否确实可用 —— 在 _run_pipeline 中,bundle, optimizer_config, gate_config = validate_inputs(...)。如果失败发生在评估期间,则存在 bundle。但是 run_pipeline_run_pipeline 外层捕获异常,并且只传递 options。所以是的,bundle/哈希丢失了。已确认。

这确实是 diff 中的一个真正问题。Warning 级别。

让我也重新考虑一下:_failure_resultaudit.num_runs=1case_parallelism=options.case_parallelism(在覆盖之前是默认值 1)。在失败时,这些不会反映配置。次要问题 —— 与哈希丢失属于同一个审计保真度问题。合并。

让我再检查一次,看看是否有我可能遗漏的真正正确性 bug。让我看看 _optimize 的真实模式:candidate = result.best_prompts; if not candidate or PROMPT_KEY not in candidate: candidate = {PROMPT_KEY: baseline_prompt}。然后 _evaluate_candidate 使用 prompts[PROMPT_KEY]best_prompts 是否保证有 PROMPT_KEY?SDK 的 target_prompt 注册了 PROMPT_KEY,所以 best_prompts 应该有。后备处理没问题。

现在确实有一个潜在的真正问题:在真实模式下,_optimize 调用 AgentOptimizer.optimize 时使用了 call_agent=_make_call_agent(prompt_path, options.mode),但优化器在内部使用目标提示词运行其自己的评估。优化器的评估使用提示词文件的当前内容。_evaluate_candidate 随后用 best_prompts 覆盖了提示词文件并重新评估。所以有两轮评估。这是设计使然(优化然后重放候选)。没问题。

我认为代码很稳健。让我最后确定警告事项。为了彻底起见,让我通过检查 SDK 对 final_response 精确匹配指标的处理来验证测试 test_evaluate_split_keeps_passing_result 的断言 primary_score == 1.0 —— 但测试通过了(按预期设计)。我相信它。

让我写下审查意见。

实际上,让我重新考虑一个我想确认的真正问题:_snapshot_result 计算每个指标的 metrics 平均值,包括 primary。然后 primary_score = None if any(hard_failure) else metrics.get(primary)。但是 metrics 是所有用例的平均值,包括 hard_failure 的用例。 对于 hard_failure 用例,scores.get(primary) 可能为 None(因为该用例的指标未被评估) → 该用例对 _mean_optional 的贡献为 None → 从平均值中排除。所以 metrics[primary] 是非 hard_failure 用例的平均值。但是 primary_score 被强制设为 None(如果有任何 hard_failure)。这是故障关闭机制。没问题。

但这里有一个细微之处:pass_rate = passed / len(cases)。在 fake-model 通过的情况下,所有用例都通过 → 1.0。在 _snapshot 测试助手中,pass_rate=sum(passed)/len。没问题。

好的,我很有信心。让我写审查意见。

鉴于稳健性,我将报告:

  • Warning:_failure_result 丢弃了已计算的 bundle/输入哈希和配置的 num_runs → 失败的审计报告缺少可重复性字段(DESIGN.md 中强调了审计性)。修复:将 bundle 和已解析的配置传入 _failure_result,或者在失败时从 _run_pipeline 内部填充它们。
  • Suggestion:提交的 optimization_report.json 示例如果不重新生成可能会不同步 —— 但 DESIGN.md 已经说明它是示例,不是契约,且测试也不会断言它。所以价值较低。根据“最多 2 个 Suggestion,且只有在确实有维护价值时输出”的规则,我将省略它。

让我重新考虑一下是否还有第二个真正的问题。让我重新检查 _restrict_permissions 仅应用于报告文件,而不应用于 work/ 提示词副本。工作副本 output_dir/work/prompts/system.md 包含提示词(最终是候选词),并且是使用默认 umask(0o644)写入的。报告 optimization_report.json 也包含 optimization.best_prompts(完整候选提示词文本),并且是 0o600。因此,敏感的候选提示词被写入了 0o644 的 work/prompts/system.md 和 0o600 的报告 JSON 中。不一致 —— 工作副本更具可读性。对于一个演示来说,这很次要,但这是一个真正的一致性问题。我将其作为 Suggestion 提及。实际上提示词并不敏感。为了保持低噪声,我将跳过它。

让我重新考虑指令:“如果没有发现明确问题,请直接说明”。但我确实发现了一个真正的问题(_failure_result 哈希)。让我将其报告为 Warning。

让我再重新考虑一下这是否真的是一个“问题”,还是可接受的示例行为。DESIGN.md 明确指出审计字段(时间戳、git sha、python 版本)仅用于展示。输入哈希也是如此吗?DESIGN.md 说“报告 JSON/Markdown 保存输入 hash”。因此,输入哈希是预期交付的一部分。在失败路径中,它们被清空了。对于一个强调审计性和可重复性的示例,失败报告应该仍然包含输入哈希(在失败前已计算)。这确实是一个缺陷。Warning。

我将最终确定。I have thoroughly analyzed the diff and verified the key SDK call sites (TargetPrompt.write_all, AgentEvaluator.get_executer, AgentOptimizer.optimize, EvalConfig, EvalCase/EvalSet schemas). Let me record one verified finding and write the review.

发现的问题

⚠️ Warning

  • examples/optimization/eval_optimize_loop/loop/pipeline.py:181-204:失败路径丢失已计算的输入哈希与配置项,审计报告不完整

    • run_pipeline 捕获异常后只把 options 传给 _failure_result,而 _run_pipeline 中已经通过 validate_inputs 得到 bundle(含 hashes)以及 optimizer_config(含 num_runs/case_parallelism),但失败报告里 audit.input_hashes={}num_runs=1,均为硬编码默认值。DESIGN.md 明确把“输入 hash”列为必备审计字段,一旦失败发生在输入校验之后(如评估、优化、回写阶段),失败报告就丢失了可复现性依据,与示例“可审计闭环”的定位不一致。
    • 修复方向:将 bundle 与解析后的 optimizer config 传入 _failure_result,在可用时填充 input_hashesnum_runs/case_parallelism,仅在 validate_inputs 本身失败时才退回默认空值。
      def _failure_result(options, started, error, bundle=None, optimizer_config=None):
          ...
          audit=AuditInfo(
              seed=91,
              input_hashes=bundle.hashes if bundle else {},
              num_runs=optimizer_config.evaluate.num_runs if optimizer_config else 1,
              ...
          )
  • examples/optimization/eval_optimize_loop/loop/pipeline.py:291-303_write_back_and_report 在报告写入失败时会把已成功回写的源 prompt 回滚,但回滚用裸 write_text 非原子

    • TargetPrompt.write_all 本身已是 tmp+os.replace 原子写;这里的兜底 bundle.prompt_path.write_text(original) 在异常路径上以非原子方式覆盖源文件,若此时再次中断会留下半写内容。影响面有限(仅 write_reports 在回写成功后抛错时触发),但与 SDK 的原子性保证不一致。建议复用 TargetPrompt 的原子写原语,或记录“需人工核对”而非裸写恢复。

总结

整体实现稳健:fake/trace/real 三模式状态流转、candidate prompt 的 try/finally 恢复、gate 的 fail-closed 语义、split 泄漏校验均与 SDK 契约一致,未发现安全漏洞或核心逻辑错误。主要风险是失败路径审计字段(输入哈希、num_runs)被硬编码丢弃,与该示例强调的可审计目标不符,建议修复但不构成阻塞。

测试建议

  • 建议补充一个用例:在 validate_inputs 通过后(如构造评估阶段抛错)触发 _failure_result,断言失败报告中 audit.input_hashes 非空且 num_runs 来自 optimizer 配置,覆盖当前未验证的失败审计路径。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

发现的问题

⚠️ Warning

  • examples/optimization/eval_optimize_loop/run_pipeline.py:2495:gate 拒绝时进程退出码为 0

    • main() 仅以 result.report.failures 决定退出码,而正常完成路径(_build_report)即使 decision.accepted=False(status=REJECTED)也不会写入 failures,只有异常路径 _failure_result 才会设置。结果是"gate 拒绝"与"gate 接受"都返回 0,仅异常返回非零。若该脚本被 CI/上游脚本复用(README 鼓励直接运行 run_pipeline.py),拒绝的候选会被当成成功放行,与设计宣称的 fail-closed 语义冲突。建议改为按 result.report.gate.accepted 决定退出码(如 return 0 if result.report.gate.accepted and not result.report.failures else 1)。
  • examples/optimization/eval_optimize_loop/loop/pipeline.py:1730-1732_sdk_version 恒为 "unknown",审计字段失效

    • getattr(trpc_agent_sdk, "__version__", "unknown") 依赖 trpc_agent_sdk/__init__.py 暴露 __version__,但该 __init__.py 并未 import 或定义该属性(版本实际定义在 trpc_agent_sdk/version.py__version__ = '1.1.14'),已提交的 optimization_report.json 也确认为 "sdk_version": "unknown"。这使 DESIGN.md 宣称用于复现审计的 sdk_version 字段永远无信息量。建议改为 from trpc_agent_sdk.version import __version__importlib.metadata.version("trpc-agent-py")

💡 Suggestion

总结

整体结构清晰、fail-closed 语义与回写保护实现得当,未发现安全或核心逻辑层面的阻塞问题。两处 Warning 值得修复:CLI 在 gate 拒绝时返回退出码 0 会误导 CI/复用方,以及 sdk_version 审计字段恒为 unknown 削弱了可复现性审计。

测试建议

  • 补充一条 CLI/集成测试:构造 gate 拒绝(如候选验证集退化)的正常完成场景,断言 main() 返回非零退出码,覆盖当前仅靠 failures 判定退出码的盲区。
  • 补充断言 _sdk_version() 返回非 "unknown" 的真实版本号,防止审计字段静默退化。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

构建 Evaluation + Optimization 的自动回归与提示词优化闭环

2 participants