Conversation
LLM-judge based evaluation tool that measures whether the broken-access-control skill actually improves SAST output. Runs with/without skill comparison using the student's existing AWS Bedrock session — no Claude Code or external tools required. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The new evaluator has a few confirmed runtime robustness issues (e.g., output typing/division-by-zero) and enables an SSRF-prone web-fetch tool by default, which is unsafe for a benchmarking script.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds a standalone evaluation harness for Exercise 08 that benchmarks the broken-access-control DeepAgents skill by running identical SAST prompts with and without the skill enabled, then scoring both outputs via an LLM judge and saving results to disk.
Changes:
- Introduces
scripts/exercise-08/evaluate_skill.pyto run a with-skill vs baseline analysis across multiple access-control-focused eval cases. - Adds an LLM-judge scoring step that produces per-criterion verdicts and an overall quality assessment, then persists outputs/grades under
skill-eval-results/. - Prints a benchmark summary including score deltas and runtime overhead.
File summaries
| File | Description |
|---|---|
| scripts/exercise-08/evaluate_skill.py | New CLI tool to benchmark the broken-access-control skill using with/without runs and an LLM-based grader, persisting comparable results. |
Review details
Suppressed comments (3)
scripts/exercise-08/evaluate_skill.py:176
- run_agent assigns final_output directly from msg.content, which may be non-string (e.g., list/dict for structured or multi-part content). That can later crash at f.write(run_with["output"]) which expects str. Also, tc["name"] will throw if tool call objects aren't dicts, causing the whole run to be treated as an exception. Coerce output to str and extract tool names defensively.
if hasattr(msg, "tool_calls") and msg.tool_calls:
for tc in msg.tool_calls:
tool_calls.append(tc["name"])
elif hasattr(msg, "content") and msg.content:
final_output = msg.content
scripts/exercise-08/evaluate_skill.py:231
- print_grades divides by max_score, which is len(scores). If the judge returns an empty scores array (valid JSON but no entries), this will raise ZeroDivisionError and stop the entire benchmark. Guard the empty case and avoid KeyError by using s.get("verdict").
scores = grades.get("scores", [])
total = sum(score_to_points(s["verdict"]) for s in scores)
max_score = len(scores)
print(f"\n [{label}] Score: {total}/{max_score} ({total/max_score*100:.0f}%)")
scripts/exercise-08/evaluate_skill.py:254
- The evaluator assumes the target repo already exists at REPO_PATH, but nothing verifies it before constructing the FilesystemBackend/agent. If ./repo is missing, the script will fail later with a less actionable error. Add an early existence check (or a clear instruction to run the demo/cloning step) and exit cleanly.
os.makedirs(OUTPUT_DIR, exist_ok=True)
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
|
||
| return create_deep_agent( | ||
| model=llm, | ||
| tools=[FetchURLTool()], |
|
No summary was generated for this pull request. All finding details can be found in the DryRun Security Dashboard. |
Summary
evaluate_skill.py— an LLM-judge based tool that measures whether thebroken-access-controlskill actually improves SAST outputUsage
Test plan
python evaluate_skill.py --skill-only --eval idorto verify it completes against Bedrockskill-eval-results/🤖 Generated with Claude Code