Repository navigation
Conversation
Five EvaluationCards covering the reuse decision surface: use-as-is, extend, remix on stated ownership, unstated ownership intent, and a multi-part brief where every part has a catalog answer. Each prompt is anchored on a definition that exists in the catalog realm and asks for an instance to be created and shown — the runner's verdict requires a rendered card independent of successCriteria, so a definition-only reply scores a failure whatever it reused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each of the five graded one reuse decision on a brief engineered so that decision was nearly forced. That measures isolated choices and misses the failure the skill itself names: reusing one part of a brief and hand-building the rest, which only appears at scale. One service-desk brief carries eight reuse opportunities across card, field, command and component, graded on the ratio taken plus a must-not-hand-roll list, with criteria phrased conditionally so a different-but-sound design still passes. A follow-up asks for a divergence, so the run also covers what happens when the catalog answer no longer fits. Service-desk carries no Listing, so install and remix of a whole bundle are unmeasured here — recorded in the card's notes along with why the timing targets do not apply to a build this size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FadhlanR
changed the title
Reference and install look equally successful from the outside: both leave a working blog on screen. The difference is what the user is left holding — definitions still wired to the catalog, or a copy that will not receive upstream fixes. Two turns. The first describes a need the blog Listing matches and says nothing about owning anything, so the definitions should be referenced. The second asks for a version of their own, so the copy is now what was asked for. The binding has to move between them, which a single-turn card cannot observe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FadhlanR
changed the title
…reuse-evaluation-cards
…he scores, and move the ownership eval to an installable Listing An EvaluationCard can name the skill features a run needs (skillFeatures); the runner enables each in the room before the first prompt, and EVAL_SKILL_FEATURES adds more for an A/B run. Host, bot and runner failures land in an `unscored` tier the report mean leaves out, and every applied write counts, not only run-realm-code. The runner reopens the evaluation's room when a tool such as listing-remix moves the panel, clicks a pill still spinning past the host's tool timeout once and records it, flags a run that never read an enabled feature's skill, and no longer counts never-run invalid calls as a loop. reuse-listing-ownership now uses the Wine Cellar Listing, which installs and remixes cleanly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The host now marks a message finished only after its tools are built, so a tool is no longer left unclaimed for the runner to click; a pill still stuck past the host's tool timeout ends the run as a host failure, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…om failing a run, and mark unscored results WRITE_TOOL covers the remaining host tools and catalog commands that write into a realm, and the summary and result card report that total. Room return reads the current room without waiting and retries on the next poll when a switch fails. The feature-enable wait matches the no-reply limit and stops at once on an unknown feature. Unscored results get their own style, and the service-desk summary counts the seven opportunities its criteria grade. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s that tier Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da8f1a9b2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const WRITE_TOOL = | ||
| /^(copy-card|copy-card-to-stack|copy-and-edit|copy-source|copy-file-to-realm|download-file-to-realm|patch-card-instance|patch-fields|patch-code|patch-theme|save-card|store-add|write-text-file|write-binary-file|apply-markdown-edit|add-field-to-card-definition|execute-atomic-operations|transform-cards|create-specs|generate-readme-spec|generate-example-cards|generate-thumbnail|migrate-skill|listing-create|listing-generate-example|listing-install|listing-remix|listing-use)(_|$)/; |
There was a problem hiding this comment.
GenerateThemeExampleTool.run persists a new card through createExampleInstanceFromPayload, and because it is a default export its tool request is named generate-theme-example_<hash>. Since that prefix is absent here, a model that creates its only card with this tool and then renders it is classified as model-failure for making no write, corrupting the evaluation result despite successfully creating the requested card.
Useful? React with 👍 / 👎.
| if (name === 'readRealmFile') { | ||
| let args = parseArguments(request.arguments); | ||
| let urls = args.attributes?.urls ?? args.urls; | ||
| if (Array.isArray(urls)) { | ||
| result.filesRead.push( |
There was a problem hiding this comment.
This records every URL requested from readRealmFile regardless of the corresponding result. If the request is invalid, or a batched request succeeds for another URL while the feature's SKILL.md fails, that skill URL still enters filesRead; the check in runModel therefore omits the warning and reports that the feature was exercised even though its instructions never entered the model context. Derive successful reads from the result's attached files rather than the request arguments.
Useful? React with 👍 / 👎.
|
Can you show the |
| } else if (outcome.key !== 'applied') { | ||
| result.failedToolCalls.push({ name, reason: outcome.reason }); | ||
| } | ||
| if (name === 'readRealmFile') { |
There was a problem hiding this comment.
[Claude Code 🤖] filesRead counts reads that never happened. URLs are recorded from every readRealmFile request regardless of its outcome. A read that failed, or a URL dropped past the per-call cap (the bot reports these as "not read"), still lands in filesRead. The skill-read check then reports the feature's SKILL.md as read when the model never had it in context. Gate the push on outcome?.key === 'applied' (and ideally on the URLs the result says were actually returned).
Generated by Claude Code
| return false; | ||
| } | ||
| try { | ||
| await page.locator('[data-test-past-sessions-button]').click(); |
There was a problem hiding this comment.
[Claude Code 🤖] returnToRoom can block the poll loop and read the wrong room. The clicks rely on the 30s actionTimeout, then two 60s waits, so one failed attempt holds a poll for up to about 150s. During that time the stuck, stalled and deadline checks in waitForIdle don't run. A failed attempt can also leave the past-sessions popover open, so the next poll's click closes it again. If the eval room never shows in the list (for example, many rooms on one shared account from parallel workers), every poll fails silently. readActivity then keeps reading the remix room's DOM, and idle or no-reply is decided on the wrong conversation. Use short explicit timeouts. After N failed attempts, record it as an irregularity or runner failure instead of carrying on.
Generated by Claude Code
| // feature's skill lives in a skills/<feature name>/SKILL.md directory. | ||
| for (let feature of result.skillFeatures) { | ||
| let read = result.analysis.filesRead.some((url) => | ||
| url.endsWith(`/skills/${feature}/SKILL.md`), |
There was a problem hiding this comment.
[Claude Code 🤖] The skill-read check assumes skills/<feature>/SKILL.md, which may not exist. Per the ai-bot README, the catalog-reuse markers sit in the skills index ("turns on the mandatory catalog search in the skills index"), not necessarily in a skills/catalog-reuse/ directory. If the feature's text lives elsewhere, this note fires on every run. If the model gets the gated text from the always-loaded index, the note fires even though the feature was exercised. Consider mapping each feature to the file(s) that carry its markers rather than deriving a path from the name.
Generated by Claude Code
| successCriteria: attributes.successCriteria ?? '', | ||
| initialCards: linkedIds(doc, 'initialCards', id), | ||
| initialFiles: linkedIds(doc, 'initialFiles', id), | ||
| skillFeatures: Array.isArray(attributes.skillFeatures) |
There was a problem hiding this comment.
[Claude Code 🤖] Feature names are filtered on trim() but kept untrimmed. An entry like " catalog-reuse" passes the filter. The runner then sends boxel-debug:feature:enable: catalog-reuse, which fails the bot's FEATURE_COMMAND_RE ([\\w-]+ right after the colon). The bot replies with only the generic status, so neither the is now enabled locator nor the There is no feature named locator matches. The run waits the full 120s NO_REPLY_MS and ends as a confusing runner-failure. Map to f.trim(), and consider validating against [\\w-]+ here.
Generated by Claude Code
| // carries no usage, so it is not counted as a turn. A name the bot | ||
| // does not know gets "There is no feature named …" instead, which ends | ||
| // the run straight away rather than at the timeout. | ||
| let enabled = page.getByText(`${feature} is now enabled`).first(); |
There was a problem hiding this comment.
[Claude Code 🤖] The enabled check can match an earlier reply. getByText does a substring match across the whole page. A feature whose name is a suffix of one enabled earlier (for example reuse after catalog-reuse) matches the earlier "catalog-reuse is now enabled" reply, so the loop moves on before the bot has confirmed this feature. Scope the locator to the newest bot message, or use exact/a regex anchored on the code element.
Generated by Claude Code
| } | ||
| if (analysis.realmCodeWrites === 0) { | ||
| reasons.push('no file was written (no applied run-realm-code write)'); | ||
| let writes = analysis.realmCodeWrites + analysis.toolWrites; |
There was a problem hiding this comment.
[Claude Code 🤖] Altitude: the pass gate now depends on a hand-maintained tool-name allowlist. WRITE_TOOL in room-analysis.ts has 28 hard-coded prefixes, kept in sync by hand with host tools and catalog commands. When a new writing tool is added or one is renamed, a real write reads as "no file was written" and the run becomes a model-failure. The runner already captures the workspace (captureWorkspace) and knows initialCards/initialFiles. Deriving "did the model write anything" from the workspace diff is general and can't drift.
Generated by Claude Code
| if (result.roomId && (await returnToRoom(page, result.roomId))) { | ||
| roomReturns.count++; | ||
| } | ||
| result.roomReturns = roomReturns.count; |
There was a problem hiding this comment.
[Claude Code 🤖] roomReturns is recorded but never surfaced. The table in afterAll, the result card notes in run-eval.ts and judge-result.ts never read it, so the count only shows up in the raw JSON. Add it to the notes, for example returned to the eval room N time(s), or drop the field and the counter object threaded through waitForIdle.
Generated by Claude Code
| // current room never waits, and a switch that fails (the past-sessions button | ||
| // disabled, the host reloading) returns false for the next poll to retry | ||
| // rather than failing the run. | ||
| async function returnToRoom(page: Page, roomId: string): Promise<boolean> { |
There was a problem hiding this comment.
[Claude Code 🤖] Efficiency: returnToRoom adds a second page.evaluate round-trip to every 2s poll, just to read [data-test-room]. readActivityOnce already evaluates in the page each poll. Returning the current room id from there and calling returnToRoom only on a mismatch saves one CDP round-trip per poll. It also gets the same lost-context retry handling.
Generated by Claude Code
| // Verdicts where the host, ai-bot or runner broke first: the run says nothing | ||
| // about the model. Such a result is left unscored rather than counted as a | ||
| // zero, and the report's mean leaves it out. | ||
| const UNSCORED_VERDICTS = ['host-failure', 'bot-failure', 'runner-failure']; |
There was a problem hiding this comment.
[Claude Code 🤖] The report's Passed stat still counts unscored runs. The mean now leaves host, bot and runner failures out, but passText (unchanged) divides by this.results.length. A session with 2 passes and 1 host-failure shows 2/3 passed next to a mean computed over 2 runs. That mixes two denominators, and the platform failure reads as a model miss in the pass rate. Leave UNSCORED_VERDICTS out of the denominator, or show it separately (2/2 · 1 unscored).
Generated by Claude Code
|
[Claude Code 🤖] Code review report (high effort): 9 findings. The first 5 are correctness issues and the last 4 are cleanups. Each one also has an inline comment. Correctness
Cleanups
Note: two of the inline comments show the regex as Generated by Claude Code |
jurgenwerk
left a comment
jurgenwerk
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review looked at how the new skillFeatures path, the unscored tier and the two evaluation cards hold up against the runner's documented flow and the catalog definitions the criteria name. It did not run the evaluations or re-check the threads already open from the earlier review.
No blocking issues. The catalog definitions both cards name exist in the catalog, the feature command matches what the ai-bot accepts and answers, and CI is green. The branch merges into main with no conflicts.
- The skill doc tells evaluation authors to set
EVAL_SKILL_FEATURESforcatalog-reuse, and the card's ownskillFeaturesfield is not documented. See the comment in.claude/skills/ai-assistant-evals/SKILL.md. - The service-desk card's notes give the judge the wrong timing targets. See the comment in
reuse-service-desk-build.json. - An A/B run over these two cards cannot turn the feature off, and the result cards do not record which features were on. This is a follow-up. See the comment on
skillFeaturesinassistant-eval.spec.ts.
The earlier review's threads (the filesRead gate, returnToRoom timeouts, the feature-name trim, the confirmation locator, the passText denominator) have no reply yet. This approval does not settle them. Please answer each one before merge.
| | `EVAL_HOST_URL` / `EVAL_MATRIX_URL` | `https://localhost:4200` / `http://localhost:8008` | stack endpoints | | ||
| | `EVAL_BOT_USER` | `@aibot:localhost` | the ai-bot's matrix id | | ||
| | `EVAL_WORKERS` / `EVAL_STAGGER_SECONDS` | 5 / 40 | parallel workers, and the gap between their starts | | ||
| | `EVAL_SKILL_FEATURES` | none | comma-separated skill features to switch on in each room before the first prompt (`boxel-debug:feature:enable:<name>`). A skill section between `<!-- feature:<name> -->` markers is left out of the prompt otherwise, so an evaluation of `catalog-reuse` needs `EVAL_SKILL_FEATURES=catalog-reuse` | |
There was a problem hiding this comment.
[Claude Code 🤖] This row says an evaluation of catalog-reuse needs EVAL_SKILL_FEATURES=catalog-reuse. But both new cards enable it with their own skillFeatures, and the runner merges the two lists. Someone who follows this row sets a variable that does nothing. And nothing tells a new evaluation author that the card field exists: the README (where it lists initialCards / initialFiles) and this skill's "Changing the runner" section do not name skillFeatures. Please say that the variable is for a bare pnpm eval:models run, or to add a feature on top of the card's own. Then add skillFeatures next to initialCards / initialFiles in both places.
Regression (docs), non-blocking.
| "cardInfo": { | ||
| "name": "Reuse: service desk build", | ||
| "summary": "One realistic brief with seven catalog reuse opportunities across card, field and command \u2014 graded on the ratio taken, not a single decision.", | ||
| "notes": "Timing targets do not apply to this card. It is a multi-part build with a follow-up, so the <=8 turn and <=120s targets will report ROUGH as a matter of course; that is not a finding. Judge it on the reuse ratio and the must-not-hand-roll list.\n\nThe service-desk domain carries no Listing, so this card exercises the Spec/parts path only. Install and remix of a whole bundle are deliberately unmeasured here.\n\nDomain chosen because no other evaluation touches it, keeping eval vocabulary disjoint from the skill's own examples.", |
There was a problem hiding this comment.
[Claude Code 🤖] The notes give "<=8 turn and <=120s targets". The runner's BENCHMARKS in run-result.ts are 10 turns and 200 s, and gradeResult multiplies them by the number of prompts. For this two-prompt card that makes 20 turns and 400 s, so a ROUGH grade is not certain either. The judge reads these notes, so please remove the numbers. For example: "Timing benchmarks are not part of the quality judgment for this card; judge it on the reuse ratio and the must-not-hand-roll list."
Regression, non-blocking.
| let skillFeatures = [ | ||
| ...new Set([...(evaluation?.skillFeatures ?? []), ...SKILL_FEATURES]), |
There was a problem hiding this comment.
[Claude Code 🤖] The PR describes EVAL_SKILL_FEATURES as the way to do an A/B run. Two gaps prevent a clean A/B over these cards. (a) The variable can only add features. Both cards include catalog-reuse in their own skillFeatures, so to get the feature-off arm someone must edit the card. (b) result.skillFeatures gets to session.json but createResultCard in run-eval.ts does not write it to the EvaluationResultCard. So the result cards and report cards of the two arms look the same in the evaluations workspace. A small fix: an opt-out (for example EVAL_SKILL_FEATURES=-catalog-reuse, or a separate EVAL_SKILL_FEATURES_OFF), plus a skillFeatures note or field on the result card.
Follow-up, not this PR. Non-blocking.
|
Would still like to see the EvaluationReportCard with at least a couple of llm models evaluated. |
This branch has not been deployed
3 participants
Background and Goal
Two EvaluationCards that measure whether the AI assistant reuses the catalog instead of rebuilding, and the runner changes needed to measure that reliably.
reuse-service-desk-buildis a support-desk brief with eight reuse opportunities across card, field, command and component, followed by a change request (P0–P4 priorities, auto-escalation). It is graded on:reuse-listing-ownershipopens with "I want to keep track of our wine collection", which the catalog's Wine Cellar Listing answers, with no ownership language. It then follows with "we'd like our own version". The assistant should reference the catalog definitions first, and install or remix the Listing only once ownership is asked for. The Wine Cellar Listing is used because it installs and remixes cleanly.Related PRs:
Where to start
eval-realm/Evaluation/*.jsonholds the two evaluations.skillFeatures: ["catalog-reuse"]switches that feature on in each room, since the skill's sections sit behind a feature flag.assistant-eval.spec.tsholds the runner changes described below.Key decisions and non-obvious mechanics
skillFeatureson EvaluationCard. The runner sendsboxel-debug:feature:enable:<name>before the first prompt.EVAL_SKILL_FEATURESadds more for an A/B run.host-failure,bot-failureorrunner-failuregets theunscoredtier and is left out of the report mean, so a broken run never reads as a model zero.copy-card,patch-fields, the listing commands and the rest, not onlyrun-realm-code.listing-remixmoves the assistant panel to a room of its own, so the runner reopens the evaluation's room before reading the page.SKILL.mdwas never read is flagged.Latest results
Claude Sonnet 4.6 and GPT-6 Luna, run sequentially with #6560 and boxel-skills#176 applied. Quality is scored against each evaluation's criteria, out of 10.
Ticketwith apriorityFieldP0–P4 scale.🤖 Generated with Claude Code