Conversation
- Add tools/run-output-evals.sh, a local runner that plays scripted multi-turn cases through the Claude Code CLI with only the plugin under test loaded, then grades the transcript and the generated files - Add plugins/solace-messaging-skills/evals/output-evals.json with 11 cases across the three skills, 5 marked must_pass - Grade with deterministic checks (assistant text, tool use, generated files, mvn compile, live sol-jcsmp release match) and a fixed-model LLM judge for routing and grounding criteria - Add an opt-in live_verify grader that runs the generated project's verify.sh against a broker named by OUTPUT_EVAL_BROKER_* variables and skips loudly when they are unset - Document the corpus format, run instructions, gate, and local-only status in the evals README - Model the runner on tools/run-trigger-evals.sh (isolation via --plugin-dir and a scratch CLAUDE_CONFIG_DIR, 90% gate, must_pass)
- Harden the Debug row in solace-application-development/references/jcsmp.md so the documentation redirect is the entire debug answer; memory-derived hypotheses, interim checks, and fixes are forbidden - State in solace-topic-best-practices/SKILL.md that the skill answers in chat only and never writes files - Both gaps surfaced as failing output-eval cases (appdev-debug-redirect-negative and topics-no-files-negative); both cases pass after these edits
aelsammak
changed the title
- Remove exact case counts from the evals README so the prose does not drift from the corpus - Remove the "No CI wiring" section from the evals README - Take broker credentials for live_verify from exported OUTPUT_EVAL_BROKER_* variables only; drop the optional env file - Report in the run summary whether live_verify executed, skipped, or never ran because the case failed before grading - Drop the run.sh and port details from two application-development case prompts; no grader depended on them
- Replace the Quickstart failover word check with a code grep for the reconnect block; the Step 6 checklist report legitimately names HA failover - Run the LLM judge tool-free (--tools "") from the run directory so a tool call cannot spend its only turn and project settings stay out of the judge session - Add a compile grader to the Solace Suggested case and make compile and maven_release_match iterate every generated pom for two-project layouts - Align the debug-redirect judge with the Debug row: fail troubleshooting from any source, treat a restated symptom as the acknowledgement - Accept bold field labels in the design summary greps (design mode leaves formatting to the model) - Validate OUTPUT_EVAL_RUNS as a positive integer (a zero value crashed bash 3.2 on an empty array expansion) - Classify a timeout kill (exit 124) as INFRA in compile and live_verify - Carry the cause onto turn-level INFRA lines and print a RETRY line before the second attempt - Show the live-verify skip tag on FAIL lines; document that a max_turns hit fails the case - Drop the run-duration claim from the evals README; mention output evals in the root README Verified locally: claude-sonnet-5 and claude-opus-5 legs over the six affected cases pass, with live verify green against a real broker on both models.
| skill="$(jq -r '.skill' <<<"$case_json")" | ||
| must="$(jq -r '.must_pass // false' <<<"$case_json")" | ||
| max_turns="$(jq -r '.max_turns // 25' <<<"$case_json")" | ||
| NTURNS="$(jq -r '.turns | length' <<<"$case_json")" |
There was a problem hiding this comment.
jq is not standard on windows bash shell even for the claude git bash requirement. And there is a lot of jq. Makes me wonder if the run evals should be in python or for better performance go.
There was a problem hiding this comment.
I will be opening a separate ticket to migrate this to python or go
mtite-sol
left a comment
mtite-sol
left a comment
There was a problem hiding this comment.
Added some non blocking suggestions but overall code LGTM!
| "max_turns": 80, | ||
| "graders": [ | ||
| { "type": "assistant_grep", "pattern": "[Ff]ailover", "expect": "present", "turn": 3 }, | ||
| { "type": "java_disclaimer" }, |
There was a problem hiding this comment.
Non blocking suggestion: java_disclaimer and maven_release_match are rules that apply to any generated code, but right now each case that generates code has to remember to include them, and if someone forgets nothing fails. Could the runner just run them on every case instead? They'd pass when there's no .java or pom.xml to check. Then they can be removed from the two cases that have them and new cases pick them up automatically. compile should stay explicit since it's expensive.
full disclosure, I've never used evals before so just suggesting this based on experience with other test frameworks 😆
There was a problem hiding this comment.
I do see your point but, in the future when we expand the scope of this skill to handle more of our APIs in other languages we wouldn't want to enforce language specific rules to them. So having them optional for now will make more sense in the future!
There was a problem hiding this comment.
Thinking about it more - maven_release_match global rule doesn't make sense but I think my point about the disclaimer is still valid. I meant for it to be api agnostic - the check could look at all generated source files (*.java today, *.py / *.go etc. as other API skills land) and pass when either the source files contain the disclaimer, or when no source files are generated. It would fail when files are generated and any do not contain the disclaimer. That way nobody has to remember to add it to each test per API, and a new code-generating case can't silently skip the disclaimer check.
That being said, I don't think this needs to happen as part of this PR, just something to think about.
There was a problem hiding this comment.
Yes that is a good point! Something to definitely remember for when we go to add more languages 😄
- Add tools/output-evals.schema.json (JSON Schema draft 2020-12) that defines the output-eval corpus with per-grader field rules and a description for every field, in response to review feedback - Rename the corpus fields turns to user_turns and max_turns to max_agent_turns to separate the scripted user messages from the CLI's agent-turn budget; update the runner and the corpus accordingly - Point the evals README and the runner header at the schema; the runner keeps enforcing the structural subset at start-up - Add a prerequisites list and a transcript-to-corpus mapping table (observed with Claude Code 2.1.266) to the evals README
- Resolve GNU timeout with a gtimeout fallback for the mvn compile and verify.sh bounds, and warn once when neither is on PATH and a selected case needs Maven, instead of running unbounded silently - List timeout as an optional prerequisite in the evals README with the unbounded fallback and the macOS install hint - State in both README credential notes that an existing claude login does not satisfy the runner, because the throwaway config directory carries no login
Local run of the output evals on WindowsRan both legs from this branch unmodified, on Windows 11 (Cygwin bash 5.3, JDK 11.0.31 Temurin, Maven 3.6.3,
ᴹ = The suite works well and the docs are accurate — I got it running with no changes to the runner. Both 1. The runner ignores
|
| leg | denials |
|---|---|
claude-sonnet-5 |
5 × Bash |
claude-opus-5 |
2 × Bash, 1 × PowerShell |
On appdev-quickstart-implement-full, sonnet issued a compound command:
cp '...\references\jcsmp\scripts\verify.sh' './verify.sh' && chmod +x ./verify.sh && ls -la
which came back denied:
This Bash command contains multiple operations. The following parts require approval:
cp '...',chmod +x ./verify.sh,ls -laClaude requested permissions to edit
...\references\jcsmp\scripts\verify.shwhich is a sensitive file.
It then fell back to Read → Write, re-emitting all 55 KB of verify.sh through the model. Opus used a single-command cp with forward slashes, which was allowed, and passed.
So --allowedTools Bash does not grant unrestricted Bash: the multi-operation splitter still gates compound commands, and a sensitive-file guard fires on the plugin's own reference path. The measured behavior was distorted exactly as the header says it should not be.
Suggestion: every result event already carries permission_denials. Treating a non-empty array as INFRA would have surfaced this as an infrastructure problem rather than scoring it as a skill failure. Worth considering regardless of platform, since a denial can flip any case.
2. No .gitattributes — the identical_to grader can't pass on a Windows checkout
With core.autocrlf=true (the Git for Windows installer default) the whole tree checks out CRLF. The reference verify.sh is then 56,406 bytes with 1,017 CRs, while the model writes LF (55,389 bytes). Strip the CRs and the MD5s are identical (63f3c5bf…), so the content was right — only the byte comparison failed. This is the only identical_to grader in the corpus, so it's a single point of failure for a must_pass case.
Suggestion: add .gitattributes with *.sh text eol=lf (or * text=auto eol=lf repo-wide) and renormalize.
Worth noting the Windows behavior is arguably more correct here. implement-mode.md:18 and :117 require "a copy of the bundled scripts/verify.sh". On a Unix checkout, sonnet's retyped LF output would have matched byte-for-byte and passed — silently hiding that it never copied the file, which is precisely the drift this grader exists to catch. On Windows the grader accidentally distinguishes a real copy from a retype. If catching that is intended, it may deserve a grader that doesn't depend on line-ending luck.
3. feedback-support-email-positive — a real sonnet-specific contract gap
Not environmental. Sonnet's draft omitted Steps to reproduce entirely, though the feedback is bug-shaped and SKILL.md:151 requires the section in that case. It also emitted Notes / None, where SKILL.md:152 says Notes is omitted when empty.
Opus, on the identical session, produced usable steps and correctly dropped Notes:
Steps to reproduce
1. Ask the solace-application-development skill to generate a JCSMP subscriber
consuming from a queue with guaranteed messaging.
2. Inspect the generated onReceive handler.
3. Observe ackMessage() is invoked before the processing call rather than after
it returns.
So the case is well formed — the steps are derivable from the reported bug, and there's no need to seed the prompt with a real invocation history. This looks like a genuine model-dependent gap in the skill. The Notes padding isn't currently graded and might be worth an assistant_grep absent-check.
Doc nits
timeoutis an undocumented dependency. It bounds themvnandverify.shcalls; without it both run unbounded and the prerequisite list doesn't mention it.- An existing
claudelogin does not satisfy the credential requirement, because the scratchCLAUDE_CONFIG_DIRdiscards it by design. That's the first thing anyone hits — one sentence in the README would save the confusion. mvnon PATH is documented, but note the compile cases fail closed before any case runs, which is good; just worth knowing a Maven wrapper dist works fine as a substitute.
Local caveats
Two things specific to my machine that may not generalize: an rtk shim rewrites commands (ls → rtk ls), which added a part to sonnet's compound command and may have contributed to that split; and the sensitive-file guard on the plugin reference path may come from my local settings. Neither changes finding #1 — the runner currently can't distinguish "the skill behaved badly" from "a denial forced a fallback."
…ripts - Read permission_denials from every turn's result event: a denial of a tool the runner grants is INFRA at once, with the tool and command on the case line and no retry, because the same policy would deny the retry; a denial of any other tool is tagged on the case line - Correct the header claim that the allowlist prevents denials; managed settings, hooks, and command shims can still deny a granted tool, and a denied cp became a lossy retype in a Windows run - Add .gitattributes with *.sh text eol=lf so Windows checkouts keep shell scripts LF for bash and for the byte-identity grader on verify.sh - Document Git Bash and LF on Windows, and denials as INFRA, in the evals README
…edback skill - Make Steps to reproduce mandatory for bug-shaped feedback and derive it from the report when the session holds no skill invocation; a Windows sonnet run dropped the section under the old wording - Drop the Notes heading when empty and forbid None or N/A filler under it - State that Read is the skill's only tool and that the plugin manifest is read at its announced path, never located with a shell command; three-run checks showed sonnet running find for the manifest - Add a grader to feedback-support-email-positive that forbids a line consisting only of None - Three sonnet runs of the case pass by majority vote after these edits
- Prescribe the copy command in implement-mode Step 4: cat the bundled script into the project root and chmod it, and forbid cp on the bundled file - Claude Code protects the plugin directory and blocks a cp that names a file inside it, even with Bash allowed; the blocked copy fell back to a retype of the script that must stay byte-identical, and the runner now reports that denial as INFRA - Reproduced on macOS with the runner's flags: cp denied, cat redirect allowed and byte-identical - Two sonnet runs of appdev-quickstart-implement-full with this wording used the cat form with zero denials; the second passed every grader
- Add exclude_pattern to the four bare Bash-absent graders so a bare echo, true, or : (plus ls and pwd on the topics negative) no longer fails a case; a redirect, a chained command, a search, or a transport call still does - Sonnet ended feedback turns with echo done in about a third of runs and opus listed the directory once before it refused to write the file, while the graded content was correct every time - Tell the grounding judge that deriving the consequences of the fetched rules for the scenario is applying the guidance, and tell every judge that WebFetch results are the tool's summaries, not the page; a summary remark that guidance was absent had failed a correct opus answer - Correct the README Windows note: git checkout -- . leaves a stale CRLF checkout in place because git treats the files as unchanged; document the delete-and-checkout recipe
- Add one sentence to step 3: when the user's scenario needs a decision the fetched page does not cover, say so before suggesting one - The grounding judge already accepts flagged extrapolation, and sonnet folded command topics into the page's template unflagged in 2 of 4 runs - Fifteen single sonnet runs of topics-grounded-answer pass after the edit; on the final tree the full sonnet leg is 11/11 and the opus leg 10/11, live verify skipped
cjwmorgan-sol
left a comment
cjwmorgan-sol
left a comment
There was a problem hiding this comment.
Tried running the evals locally again and passed with the 90% gate.
Had some offline discussions with @aelsammak as I'm not sure the pass fail gate measure is accurate/reflective enough, but that would be out of scope for this change.
Really appreciate the output-evals.schema.json to document the structure of output-evals.json. I hope this can be used in verification of output-eval.json additions/substractions later as well!
Change look ready for merge to me!
3 participants
What is the purpose of this change?
Trigger evals only check that the right skill fires. Nothing checks that a skill's output honors its contract (design before code, the door question, scrubbed feedback drafts, grounded answers). This PR adds a local output-eval suite for
solace-messaging-skillsand fixes two skill contract gaps that the suite found.How is this accomplished?
tools/run-output-evals.sh, modeled ontools/run-trigger-evals.sh. It runs oneclaude -pinvocation per scripted user turn and continues the session with--resume, under--plugin-dirand a scratchCLAUDE_CONFIG_DIR. The subject gets the full tool allowlist, including bare Bash, so headless denials do not distort the measured behavior.plugins/solace-messaging-skills/evals/output-evals.json: 11 cases (5 application development, 4 feedback, 2 topic best practices), 5 markedmust_pass(the four forbidden-behavior negatives plus the compile anchor). A case passes only when the target skill fired, every grader passed, and no turn hit themax_turnscap.assistant_grep,tool_use,file_exists,file_grep,java_disclaimer,compile(a realmvn compileof every generated pom, so the two-project Solace Suggested layout is proven whole),maven_release_match(every generated pom carries the live sol-jcsmp release),llm_judge(tool-free, fixed judge model, defaultclaude-sonnet-5), andlive_verify(opt-in: runs the generated project'sverify.sh roundtripagainst a broker named by fourOUTPUT_EVAL_BROKER_*variables, skips loudly when none is set, errors on a partial set, and keeps the values out of the subject model's environment). Both the Quickstart case and the Solace Suggested case compile their output.must_passfailure or infrastructure failure fails the run. Atimeoutkill counts as infrastructure, a turn-level infrastructure failure prints its cause, and a retry is announced.solace-application-development/references/jcsmp.md: the documentation redirect is the entire debug answer. The suite caught the model appending memory-derived checks after the redirect.solace-topic-best-practices/SKILL.mdthat the skill answers in chat only and never writes files. The suite caught it writing atopic-conventions.mdwhen asked to.Anything reviews should focus on/be aware of?
claude-sonnet-5. Initial full legs, one run per case:claude-sonnet-510 of 11 (a case-script ambiguity, fixed and re-run green) andclaude-opus-510 of 11 (a grader false positive, scoped and re-run green). Both skill fixes were validated by re-running their cases green.claude-sonnet-56 of 6;claude-opus-55 of 6, where the one failure was the tightened debug judge misreading a restated symptom as a hypothesis. The criteria now say a restated symptom is the acknowledgement, and the case re-ran green on both models.live_verifyhas run green against a real broker on both models: the generated Quickstart project'sverify.sh roundtripcompleted end to end. Each run leaves one durable queue on the broker.SSL_TRUST_STORE, the solrsearch text in the pom and checklist), so absent-checks are scoped to a tool or construct, never to a bare substring. A word check on prose fails when the skill itself requires a sentence that contains the word (the Step 6 checklist report names HA failover on every door), so the Quickstart posture is graded on the generated code instead. A judge criteria that bans hypotheses must say that restating the reported symptom is the acknowledgement.topics-no-files-negativeis notmust_pass.caffeinate -i; machine sleep surfaces as INFRA failures.