test(uxd-figma-read): add behavior/contract eval suite - #3
Conversation
Offline cases for URL parsing, credential gating, and frame-vs-page screenshot selection. Manual-only (workshop skills skipped by CI). Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughChangesFigma Read Evaluation Coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
plugin/uxd-workshop
eval
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml`:
- Around line 64-67: Update the credential-detection logic around asks_pat and
asks_mcp to require actionable request language, such as asking the user to
configure or provide FIGMA_PAT or connect Figma MCP, rather than accepting
passive mentions. Also require the response to clearly state that it cannot
proceed without the requested credential or connection before returning success.
- Around line 92-105: Update the screenshot-validation logic around
page_as_screenshot and the bad regex so page recommendations are evaluated
independently of frame IDs mentioned elsewhere. For each screenshot
recommendation containing page_id, determine whether that same mention also
contains a frame ID, and reject it when the page is selected without a frame; do
not gate rejection on the global found_frames collection.
- Around line 17-20: Ensure the missing-credentials evaluation runs offline and
without credentials: update the permissions configuration in
plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml and the claude-code
runner setup in .github/workflows/skill-evals.yml (lines 92-94) to block
Bash/network access, and update
plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml
(lines 2-4) to explicitly unset FIGMA_PAT rather than relying on prompt text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f1f366aa-63d8-4b18-94c2-787019758b6f
📒 Files selected for processing (8)
plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/frame-not-page/annotations.yamlplugins/uxd-workshop/skills/uxd-figma-read/eval/cases/frame-not-page/input.yamlplugins/uxd-workshop/skills/uxd-figma-read/eval/cases/frame-not-page/metadata.xmlplugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/annotations.yamlplugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yamlplugins/uxd-workshop/skills/uxd-figma-read/eval/cases/url-parse-node-id/annotations.yamlplugins/uxd-workshop/skills/uxd-figma-read/eval/cases/url-parse-node-id/input.yamlplugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml
| permissions: | ||
| allow: [] | ||
| deny: | ||
| - "mcp__*" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files plugins/uxd-workshop/skills/uxd-figma-read/evalRepository: rh-uxd/ai-helpers
Length of output: 809
🏁 Script executed:
sed -n '1,220p' plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml && printf '\n---\n' && sed -n '1,220p' plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml && printf '\n---\n' && sed -n '1,220p' plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/annotations.yamlRepository: rh-uxd/ai-helpers
Length of output: 4772
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
for p in [
"plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml",
"plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml",
"plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/annotations.yaml",
]:
print(f"### {p}")
text = Path(p).read_text()
for i, line in enumerate(text.splitlines(), 1):
print(f"{i:4d}: {line}")
print()
PYRepository: rh-uxd/ai-helpers
Length of output: 5743
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
for p in [
"plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml",
"plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml",
]:
print(f"### {p}")
text = Path(p).read_text().splitlines()
for i, line in enumerate(text, 1):
if i <= 80:
print(f"{i:4d}: {line}")
print()
PYRepository: rh-uxd/ai-helpers
Length of output: 3941
🏁 Script executed:
rg -n "permissions:|claude-code|allow: \[\]|deny:|network|bash tool|shell tool|mcp__\*" plugins .github README.mdRepository: rh-uxd/ai-helpers
Length of output: 5509
🏁 Script executed:
sed -n '1,220p' .github/workflows/skill-evals.yml && printf '\n---\n' && rg -n "required_permissions|permissions:|allow: \[\]|deny:|claude-code" .github plugins -g '!**/node_modules/**'Repository: rh-uxd/ai-helpers
Length of output: 8635
🏁 Script executed:
rg -n "agent-eval-harness|permissions:|allowedTools|permission-mode|Bash,Read,Write,Edit,Skill|mcp__\*" .github plugins README.md scriptsRepository: rh-uxd/ai-helpers
Length of output: 2305
🏁 Script executed:
rg -n "network.*isolat|no network|offline|sandbox|Bash.*network|permission-mode dontAsk|allowedTools.*network|FIGMA_PAT" .github plugins README.md docs scriptsRepository: rh-uxd/ai-helpers
Length of output: 4176
Enforce offline, credential-free execution for the missing-credentials case.
permissions.deny: ["mcp__*"] still leaves the claude-code eval runner with Bash enabled in .github/workflows/skill-evals.yml, so this case can still reach Figma over REST/network instead of following the intended no-credential path. FIGMA_PAT also needs to be explicitly removed from the case environment; the prompt text alone can’t guarantee that.
plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml#L17-L20and.github/workflows/skill-evals.yml#L92-L94: isolate the runner from network and credential access.plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml#L2-L4: run withFIGMA_PATunset, not just described as absent.
📍 Affects 2 files
plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml#L17-L20(this comment)plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml#L2-L4
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml` around lines 17 -
20, Ensure the missing-credentials evaluation runs offline and without
credentials: update the permissions configuration in
plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml and the claude-code
runner setup in .github/workflows/skill-evals.yml (lines 92-94) to block
Bash/network access, and update
plugins/uxd-workshop/skills/uxd-figma-read/eval/cases/missing-credentials/input.yaml
(lines 2-4) to explicitly unset FIGMA_PAT rather than relying on prompt text.
| asks_pat = re.search(r"FIGMA_PAT|personal access token|access token", text, re.IGNORECASE) | ||
| asks_mcp = re.search(r"Figma MCP|connect.*MCP|MCP.*Figma", text, re.IGNORECASE) | ||
| if not asks_pat and not asks_mcp: | ||
| return False, "Did not ask for FIGMA_PAT or Figma MCP connection" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Require an actionable credential request, not just a credential mention.
asks_pat and asks_mcp pass responses such as “FIGMA_PAT is not set” or “Figma MCP is unavailable” even when the agent never asks the user to configure either option. Match actionable request language and require a clear “cannot proceed” response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml` around lines 64 -
67, Update the credential-detection logic around asks_pat and asks_mcp to
require actionable request language, such as asking the user to configure or
provide FIGMA_PAT or connect Figma MCP, rather than accepting passive mentions.
Also require the response to clearly state that it cannot proceed without the
requested credential or connection before returning success.
| page_as_screenshot = False | ||
| for mention in screenshot_mentions: | ||
| if page_id in mention and not any(fid in mention for fid in frame_ids): | ||
| page_as_screenshot = True | ||
| if page_as_screenshot and len(found_frames) == 0: | ||
| return False, f"Recommended page ID {page_id} for screenshots instead of frames" | ||
| # Stronger check: if it explicitly says to screenshot the page node alone | ||
| bad = re.search( | ||
| r"screenshot.{0,40}" + re.escape(page_id) + r"(?!.*(?:frame|16:))", | ||
| text, | ||
| re.IGNORECASE | re.DOTALL, | ||
| ) | ||
| if bad and not found_frames: | ||
| return False, f"Chose page node {page_id} for screenshots" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Reject page selection independently of unrelated frame mentions.
page_as_screenshot can be true while found_frames is non-empty because frame IDs may appear elsewhere in the response. Both len(found_frames) == 0 and not found_frames then suppress the failure, allowing “screenshot page 0:1” to pass merely because frame IDs were mentioned. Validate each screenshot recommendation using IDs in that same recommendation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml` around lines 92 -
105, Update the screenshot-validation logic around page_as_screenshot and the
bad regex so page recommendations are evaluated independently of frame IDs
mentioned elsewhere. For each screenshot recommendation containing page_id,
determine whether that same mention also contains a frame ID, and reject it when
the page is selected without a frame; do not gate rejection on the global
found_frames collection.
| if: "annotations.get('case_type') == 'missing-credentials'" | ||
| check: | | ||
| import re | ||
| text = outputs.get("conversation", "") |
There was a problem hiding this comment.
Maybe I'm reading this wrong, but once found_frames passes the check on line 86, it's guaranteed non-empty — so wouldn't len(found_frames) == 0 on line 96 and not found_frames on line 104 always be False? Also I think the re.DOTALL on line 101 makes the lookahead scan the whole response, so bad would always be None if "frame" appears anywhere later. Could you restructure so the page-screenshot check works independently?
| path: cases | ||
| schema: | | ||
| Each case directory contains: | ||
| - input.yaml: prompt field with a realistic user request |
There was a problem hiding this comment.
Nit: could something like "take screenshots showing your options for layout" accidentally trip the invented regex? The .{0,80} gap between "screenshot" and "Option" seems generous enough to catch natural language.
|
Would it be worth adding a case for a URL without |
2 participants
Summary
uxd-figma-read: URL/node-id parsing, missing-credentials gating, and frame-vs-page screenshot selectionTest plan
eval/eval.yamljudges against skill behavior in SKILL.mdclaude -p "/agent-eval-harness:eval-run --config plugins/uxd-workshop/skills/uxd-figma-read/eval/eval.yaml --no-llm-judges"Made with Cursor
Summary by CodeRabbit