Skip to content

feat: add functional evals for 7 phase-1 skills - #23

Merged
ether-moon merged 2 commits into
mainfrom
strengthen-skill-validation
May 6, 2026
Merged

feat: add functional evals for 7 phase-1 skills#23
ether-moon merged 2 commits into
mainfrom
strengthen-skill-validation

Conversation

@ether-moon

@ether-moon ether-moon commented May 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add functional eval suite (evals.json + input fixtures) for 7 phase-1 skills under each skill's evals/ directory.
  • Each skill has 2 test prompts with structural assertions plus DISCRIMINATING assertions targeting canonical wording, fixed Q1–Q5 labels, OBVIOUS/AMBIGUOUS severity classes, deletion-test framing, and other artifacts only the skill is known to produce consistently.
  • Establishes a reusable layout (skills/<name>/evals/evals.json + evals/files/<eval-name>/...) that future skill iterations can extend.

Iteration-1 results

Ran 28 parallel general-purpose subagents (7 skills × 2 evals × 2 configs) and graded with a deterministic per-skill grader. After adding DISCRIMINATING assertions, the with_skill vs without_skill differential becomes:

Skill with → without Δ
guarding-agent-directives 100% → 57% +43%
applying-coding-baseline 100% → 68% +32%
autofixing-and-escalating 100% → 85% +15%
building-shared-vocabulary 100% → 86% +14%
bumping-version 100% → 100% 0%
managing-git-workflow 94% → 94% 0%
improving-architecture 100% → 100% 0%

Pattern. Skills that codify novel canonical wording or named procedures show clear gain. Skills that codify well-known industry standards (Keep a Changelog, Conventional Commits, Ousterhout deep modules) show no measurable gain — the baseline already knows them.

Cost. with_skill averages +13% time/tokens (SKILL.md + reference reading).

Changes

  • plugins/skill-set/skills/applying-coding-baseline/evals/ — 2 evals, fixtures for greenfield CLAUDE.md and paraphrased-rule normalization.
  • plugins/skill-set/skills/autofixing-and-escalating/evals/ — 2 evals, fixtures for mixed lint output and PR review comments.
  • plugins/skill-set/skills/bumping-version/evals/ — 2 evals, fixtures for patch (package.json) and minor (plugin.json) bumps.
  • plugins/skill-set/skills/building-shared-vocabulary/evals/ — 2 evals, fixtures for first-term glossary and ADR-worthy decision.
  • plugins/skill-set/skills/guarding-agent-directives/evals/ — 2 evals, fixtures for vague-rule rejection and specific-rule acceptance.
  • plugins/skill-set/skills/managing-git-workflow/evals/ — 2 evals with setup.sh scripts that initialize fixture git repos for commit-message and PR-title generation.
  • plugins/skill-set/skills/improving-architecture/evals/ — 2 evals, fixtures for shallow-validator cluster and deletion-test on thin wrapper.

Transient artifacts (subagent runs, deterministic grader, benchmarks, static HTML viewers) live under .context/skill-evals/ and are excluded from this PR.

Test Plan

  • evals.json files parse as valid JSON in each of the 7 skills.
  • evals/files/*/setup.sh scripts (managing-git-workflow) run without error in a fresh empty directory.
  • Re-running the deterministic grader against the existing .context/skill-evals/ outputs reproduces the iteration-1 numbers above.
  • No fixture file accidentally contains secrets (.env, credentials, keys).
  • Eval directories follow the shared layout (evals/evals.json + evals/files/<eval-name>/...).

Summary by CodeRabbit

  • New Features
    • Added seven new skill evaluations with test fixtures and scenarios for: applying coding baselines, autofixing and escalating issues, building shared vocabulary, bumping versions, guarding agent directives, improving architecture, and managing Git workflows.
    • Each skill includes sample project files, evaluation configuration, and expected outcomes to assess competency in software engineering practices.

Adds evals.json + input fixtures under evals/ for 7 skills:
applying-coding-baseline, autofixing-and-escalating, bumping-version,
building-shared-vocabulary, guarding-agent-directives,
managing-git-workflow, improving-architecture.

Each skill has 2 test prompts with structural assertions plus
DISCRIMINATING assertions targeting canonical wording, fixed
question labels, severity classifications, or other artifacts only
the skill is known to produce consistently.

Iteration-1 results (28 subagent runs, deterministic grading):
- Strong gain (delta >= 20%): guarding-agent-directives (+43%),
  applying-coding-baseline (+32%)
- Moderate gain (5-20%): autofixing-and-escalating (+15%),
  building-shared-vocabulary (+14%)
- No measurable gain: bumping-version, managing-git-workflow,
  improving-architecture (industry standards already known to baseline)

Cost: with_skill averages +13% time/tokens (SKILL.md reading).
@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@ether-moon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 39 minutes and 31 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc74c55d-1c49-4817-8c68-338e2c96b91c

📥 Commits

Reviewing files that changed from the base of the PR and between 973c5a8 and e0eab94.

📒 Files selected for processing (9)
  • plugins/skill-set/skills/EVALS.md
  • plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-lint-output/lint-report.txt
  • plugins/skill-set/skills/building-shared-vocabulary/evals/README.md
  • plugins/skill-set/skills/bumping-version/evals/README.md
  • plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/CHANGELOG.md
  • plugins/skill-set/skills/improving-architecture/evals/README.md
  • plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/admin.ts
  • plugins/skill-set/skills/managing-git-workflow/evals/files/commit-with-conventional-style/setup.sh
  • plugins/skill-set/skills/managing-git-workflow/evals/files/pr-title-and-body-generation/setup.sh
📝 Walkthrough

Walkthrough

Adds comprehensive evaluation configurations and fixture files across seven skills in the skill-set system: applying-coding-baseline, autofixing-and-escalating, building-shared-vocabulary, bumping-version, guarding-agent-directives, improving-architecture, and managing-git-workflow. Each skill receives an evals.json with scenario definitions and supporting fixture files (source code, documentation, or test data).

Changes

Skill Evaluation Configurations and Fixtures

Layer / File(s) Summary
Eval Definitions
plugins/skill-set/skills/*/evals/evals.json
Seven new evals.json files define evaluation scenarios for each skill, including prompts, expected outputs, target files, and verification expectations.
Applying Baseline
plugins/skill-set/skills/applying-coding-baseline/evals/files/*
Fixture files for greenfield and normalization scenarios: README.md and CLAUDE.md documenting baseline project setup and canonical rules.
Autofixing and Escalation
plugins/skill-set/skills/autofixing-and-escalating/evals/files/*
Lint report, source files (User, Order classes), and PR review comments for mixed-lint-output and mixed-pr-review-comments scenarios.
Building Shared Vocabulary
plugins/skill-set/skills/building-shared-vocabulary/evals/files/*
README files for first-domain-term and adr-worthy-decision scenarios documenting example project and event store service.
Bumping Versions
plugins/skill-set/skills/bumping-version/evals/files/*
Package.json, CHANGELOG.md entries, and commit logs for patch-bump and minor-bump scenarios in both npm and Claude plugin contexts.
Guarding Agent Directives
plugins/skill-set/skills/guarding-agent-directives/evals/files/*
CLAUDE.md files with project directives, build commands, and commit style guidelines for reject-vague and accept-specific rule scenarios.
Improving Architecture
plugins/skill-set/skills/improving-architecture/evals/files/*
TypeScript source files (feature-a, feature-b, string-helper utility, order validators) for deletion-test and shallow-validators-cluster scenarios.
Managing Git Workflow
plugins/skill-set/skills/managing-git-workflow/evals/files/*
Setup scripts initializing git fixtures with parser module updates (normalizeNestedArrays function) and rate-limiter feature branches for commit and PR scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ether-moon/skill-set#21: Adds core skill metadata and baseline rule definitions that correspond to the evaluation fixtures introduced in this PR.
  • ether-moon/skill-set#22: Adds applying-coding-baseline skill foundation that is complemented by the eval configurations and CLAUDE.md fixtures in this PR.

Poem

🐰 Six skills now have evals to test,
Each fixture and config put to the test,
From baseline rules to git workflow grace,
Building vocabulary, finding its place.
Version bumps and architecture dreams—
Skills are stronger than they seem! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding evaluation suites for 7 phase-1 skills, which is the core purpose of this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch strengthen-skill-validation

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (4)
plugins/skill-set/skills/building-shared-vocabulary/evals/evals.json (1)

17-17: ⚡ Quick win

Consider documenting the DISCRIMINATING: prefix convention.

The "DISCRIMINATING:" prefix is a magic sentinel string with special semantics for the deterministic grader (it identifies higher-stakes assertions used to compute the with_skill vs without_skill delta). Since JSON has no comment syntax, its meaning is entirely opaque to anyone reading this file without prior context.

A lightweight fix could be a sibling README.md in the evals/ directory explaining the eval schema, or a "$schema" reference if a JSON Schema is ever added. At minimum, a note in the top-level eval layout docs would help future contributors authoring new evals.

As per coding guidelines for **/*.json: "Document configuration values in scripts and avoid 'voodoo constants' - explain why each configuration value was chosen."

Also applies to: 33-33

🤖 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/skill-set/skills/building-shared-vocabulary/evals/evals.json` at line
17, Add explicit documentation for the magic sentinel "DISCRIMINATING:" used in
evals.json by creating a short evals/README.md that explains the prefix
semantics (what it signals to the deterministic grader, how it affects
with_skill vs without_skill scoring, and examples like the CONTEXT.md note about
'auth'), and also add an optional top-level "$schema" reference in evals.json
(or update the top-level eval layout docs) to point to a future JSON Schema;
ensure the README and any schema/doc update clearly call out the
"DISCRIMINATING:" sentinel and recommend contributors avoid undocumented voodoo
constants.
plugins/skill-set/skills/bumping-version/evals/evals.json (2)

9-13: ⚡ Quick win

Relative files paths lack a documented root.

Paths such as "evals/files/patch-bump-package-json/package.json" are relative, but the root they resolve against (skill directory? repo root? the evals/ directory itself?) is not declared in the schema or in any accompanying documentation. If the eval harness convention changes, these silently break.

Consider adding a base_dir field or documenting the resolution rule in a sibling README.md. As per coding guidelines, configuration values should be explained rather than left as implicit voodoo constants.

Also applies to: 30-34

🤖 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/skill-set/skills/bumping-version/evals/evals.json` around lines 9 -
13, The evals.json "files" array uses relative paths without a declared
resolution root causing brittle behavior; update the evals.json schema/instance
by adding a new "base_dir" (or "files_base") field alongside the existing
"files" array in the same JSON object and set it to the intended root (e.g.,
"evals/" or repo root), then update any code that consumes this file to join
base_dir with entries from "files"; alternatively, add a README.md next to
evals.json documenting the resolution rule (mentioning the "files" array and the
new "base_dir" key) so consumers and future maintainers know how paths are
resolved.

7-7: ⚡ Quick win

"outputs directory" is an undocumented implicit contract.

Both prompts instruct the agent to "Save the modified files to the outputs directory" / "Save modified files to the outputs directory", but evals.json has no outputs_dir field and no comment explaining where this directory is or how the harness resolves it. Any reader (or a future eval runner) has to guess the convention.

As per coding guidelines, configuration values should be documented and voodoo constants avoided. Consider adding a top-level or per-eval outputs_dir field, or at minimum add a comment block (e.g., in an accompanying README.md for the evals layout) documenting the convention.

📋 Example schema addition
 {
   "skill_name": "bumping-version",
+  "outputs_dir": "evals/outputs",
   "evals": [

Also applies to: 28-28

🤖 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/skill-set/skills/bumping-version/evals/evals.json` at line 7, The
evals.json prompts reference an undocumented "outputs directory"; update the
eval configuration to make this explicit by adding a top-level or per-eval
outputs_dir field (or an outputs_dir key next to the "prompt" entry) in
plugins/skill-set/skills/bumping-version/evals/evals.json and include a short
comment or README entry describing how the harness resolves that path so callers
don't rely on a hidden convention; ensure the outputs_dir value is used by the
eval harness and documented next to the "prompt" key so future readers see the
contract.
plugins/skill-set/skills/improving-architecture/evals/evals.json (1)

26-28: 💤 Low value

Consider documenting DISCRIMINATING pattern rationale.

The DISCRIMINATING expectations encode important design decisions about canonical vocabulary (depth/shallow, seam/interface, deletion test, "does not reappear"). While the choices are contextually clear from the skill name and PR objectives, the coding guideline suggests documenting configuration values.

Since JSON lacks native comments, consider one of:

  • Adding a "rationale" field to DISCRIMINATING expectations in the schema
  • Documenting the vocabulary choices in a skill-level README or evaluation guide
  • Accepting the format limitation, as the current design is reasonably self-documenting

This is a minor enhancement; the current structure is clear and functional.

As per coding guidelines: "Document configuration values in scripts and avoid 'voodoo constants' — explain why each configuration value was chosen" applies to JSON files, though JSON's lack of comment support creates a format constraint.

Also applies to: 47-49

🤖 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/skill-set/skills/improving-architecture/evals/evals.json` around
lines 26 - 28, The DISCRIMINATING expectation entries currently use
domain-specific constants (e.g., 'shallow'/'deep', 'seam'/'interface', "does not
reappear") without documented rationale; add a short "rationale" string to each
DISCRIMINATING expectation object explaining why those canonical vocabulary
choices were made (or alternatively create a skill-level README/evaluation guide
that documents the same vocabulary and the deletion/reappearance rule), ensuring
entries referencing candidates.md explicitly mention the expected canonical
terms and the deletion-test intent so reviewers and automated checks understand
the configuration.
🤖 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/skill-set/skills/autofixing-and-escalating/evals/files/mixed-lint-output/lint-report.txt`:
- Around line 3-5: The lint-report contains two incorrect location annotations:
update the entry for getDisplayName in src/user.js (currently "src/user.js:8:1")
to the correct position where the function is defined ("src/user.js:14:1"
referencing getDisplayName), and fix the getOrderTotal entry in src/order.js
(currently "src/order.js:25:1") to the actual start line of getOrderTotal
("src/order.js:18:1"); ensure the report lines reference the correct file and
function names (getDisplayName, getOrderTotal) so AMBIGUOUS items point to real
locations.

In
`@plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/CHANGELOG.md`:
- Around line 1-19: Add the standard "Keep a Changelog" preamble to CHANGELOG.md
by inserting the two-line header exactly as in the other fixture: "All notable
changes to this project will be documented in this file." and "The format is
based on [Keep a Changelog](https://keepachangelog.com/)." before the existing
release sections (preserve the existing entries for [1.5.2], [1.5.1], [1.5.0]);
ensure the bracket-link format matches exactly so the eval expecting the "Keep a
Changelog" reference will pass.

In
`@plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/admin.ts`:
- Around line 6-21: The function adminCreateOrder is marked async but performs
only synchronous calls (checkInventory, checkPromotion) so the async keyword is
unnecessary and changes error-handling/return semantics; remove async from the
adminCreateOrder declaration and update its return type to { ok: boolean;
errors?: Record<string,string> } (not Promise<...>) so callers get a plain
synchronous result, or alternatively keep async but add a clear comment above
adminCreateOrder stating it is intentionally async for future I/O and why;
ensure you update any call sites if you remove the Promise return to match the
synchronous signature.

In
`@plugins/skill-set/skills/managing-git-workflow/evals/files/commit-with-conventional-style/setup.sh`:
- Around line 4-10: Add an explicit empty-workdir preflight in setup.sh before
running git init: check for any files/dirs (e.g. using ls -A or checking git
status if repo exists) and if non-empty print a clear error like "expected empty
directory for fixture" to stderr and exit non-zero; place this check immediately
before the git init -q -b main line so the script fails fast with a helpful
message rather than seeding a flaky repo.
- Around line 65-66: The current pattern reassigns module.exports to an object
with parse and normalizeNestedArrays and then sets exports.version, which
mutates a stale reference and won’t surface on the exported object; consolidate
into a single export so the version property is part of the exported object
(e.g. include version alongside parse and normalizeNestedArrays in the
module.exports assignment) or ensure you mutate the exact exported object
reference rather than using the stale exports alias; update the export around
the parse and normalizeNestedArrays symbols and the version value accordingly.

In
`@plugins/skill-set/skills/managing-git-workflow/evals/files/pr-title-and-body-generation/setup.sh`:
- Around line 4-10: The script assumes an empty working directory but doesn't
guard against being run inside an existing repo or non-empty dir; add a
preflight check before git init (and alongside set -euo pipefail) that fails
fast with a clear message if the cwd is non-empty or already a git repo (use git
rev-parse --is-inside-work-tree or test for existing .git and for any files
besides expected fixture markers), and provide a safe alternative in the message
(e.g., instruct to run in an empty directory, remove/backup files, or set a
FORCE_ENV var to override); ensure the failing path prints the helpful message
and exits non-zero.

---

Nitpick comments:
In `@plugins/skill-set/skills/building-shared-vocabulary/evals/evals.json`:
- Line 17: Add explicit documentation for the magic sentinel "DISCRIMINATING:"
used in evals.json by creating a short evals/README.md that explains the prefix
semantics (what it signals to the deterministic grader, how it affects
with_skill vs without_skill scoring, and examples like the CONTEXT.md note about
'auth'), and also add an optional top-level "$schema" reference in evals.json
(or update the top-level eval layout docs) to point to a future JSON Schema;
ensure the README and any schema/doc update clearly call out the
"DISCRIMINATING:" sentinel and recommend contributors avoid undocumented voodoo
constants.

In `@plugins/skill-set/skills/bumping-version/evals/evals.json`:
- Around line 9-13: The evals.json "files" array uses relative paths without a
declared resolution root causing brittle behavior; update the evals.json
schema/instance by adding a new "base_dir" (or "files_base") field alongside the
existing "files" array in the same JSON object and set it to the intended root
(e.g., "evals/" or repo root), then update any code that consumes this file to
join base_dir with entries from "files"; alternatively, add a README.md next to
evals.json documenting the resolution rule (mentioning the "files" array and the
new "base_dir" key) so consumers and future maintainers know how paths are
resolved.
- Line 7: The evals.json prompts reference an undocumented "outputs directory";
update the eval configuration to make this explicit by adding a top-level or
per-eval outputs_dir field (or an outputs_dir key next to the "prompt" entry) in
plugins/skill-set/skills/bumping-version/evals/evals.json and include a short
comment or README entry describing how the harness resolves that path so callers
don't rely on a hidden convention; ensure the outputs_dir value is used by the
eval harness and documented next to the "prompt" key so future readers see the
contract.

In `@plugins/skill-set/skills/improving-architecture/evals/evals.json`:
- Around line 26-28: The DISCRIMINATING expectation entries currently use
domain-specific constants (e.g., 'shallow'/'deep', 'seam'/'interface', "does not
reappear") without documented rationale; add a short "rationale" string to each
DISCRIMINATING expectation object explaining why those canonical vocabulary
choices were made (or alternatively create a skill-level README/evaluation guide
that documents the same vocabulary and the deletion/reappearance rule), ensuring
entries referencing candidates.md explicitly mention the expected canonical
terms and the deletion-test intent so reviewers and automated checks understand
the configuration.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: db4a674e-7607-4ad6-be71-f642044c5601

📥 Commits

Reviewing files that changed from the base of the PR and between f8dcc0e and 973c5a8.

📒 Files selected for processing (34)
  • plugins/skill-set/skills/applying-coding-baseline/evals/evals.json
  • plugins/skill-set/skills/applying-coding-baseline/evals/files/greenfield-no-claude-md/README.md
  • plugins/skill-set/skills/applying-coding-baseline/evals/files/normalize-paraphrased-rules/CLAUDE.md
  • plugins/skill-set/skills/autofixing-and-escalating/evals/evals.json
  • plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-lint-output/lint-report.txt
  • plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-lint-output/src/order.js
  • plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-lint-output/src/user.js
  • plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-pr-review-comments/review-comments.md
  • plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-pr-review-comments/src/auth.js
  • plugins/skill-set/skills/building-shared-vocabulary/evals/evals.json
  • plugins/skill-set/skills/building-shared-vocabulary/evals/files/adr-worthy-decision/README.md
  • plugins/skill-set/skills/building-shared-vocabulary/evals/files/first-domain-term/README.md
  • plugins/skill-set/skills/bumping-version/evals/evals.json
  • plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/.claude-plugin/plugin.json
  • plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/CHANGELOG.md
  • plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/recent-commits.txt
  • plugins/skill-set/skills/bumping-version/evals/files/patch-bump-package-json/CHANGELOG.md
  • plugins/skill-set/skills/bumping-version/evals/files/patch-bump-package-json/package.json
  • plugins/skill-set/skills/bumping-version/evals/files/patch-bump-package-json/recent-commits.txt
  • plugins/skill-set/skills/guarding-agent-directives/evals/evals.json
  • plugins/skill-set/skills/guarding-agent-directives/evals/files/accept-specific-rule/CLAUDE.md
  • plugins/skill-set/skills/guarding-agent-directives/evals/files/reject-vague-rule/CLAUDE.md
  • plugins/skill-set/skills/improving-architecture/evals/evals.json
  • plugins/skill-set/skills/improving-architecture/evals/files/deletion-test-on-thin-wrapper/src/feature-a.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/deletion-test-on-thin-wrapper/src/feature-b.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/deletion-test-on-thin-wrapper/src/utils/string-helper.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/admin.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/credit-check.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/inventory-check.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/promotion-check.ts
  • plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/route.ts
  • plugins/skill-set/skills/managing-git-workflow/evals/evals.json
  • plugins/skill-set/skills/managing-git-workflow/evals/files/commit-with-conventional-style/setup.sh
  • plugins/skill-set/skills/managing-git-workflow/evals/files/pr-title-and-body-generation/setup.sh

Apply OBVIOUS fixture corrections and approved AMBIGUOUS items from
CodeRabbit's review of the phase-1 evals PR.

OBVIOUS (auto-applied):
- mixed-lint-output/lint-report.txt: correct two stale line numbers
  (getDisplayName 8:1 -> 14:3, getOrderTotal 25:1 -> 18:3)
- minor-bump-plugin-json/CHANGELOG.md: add Keep a Changelog preamble
  for parity with patch-bump fixture (eval expects bracket-link format)
- commit-with-conventional-style/setup.sh: add empty-dir preflight guard
- pr-title-and-body-generation/setup.sh: add empty-dir preflight guard

AMBIGUOUS (escalated, user-approved):
- shallow-validators-cluster/admin.ts: drop unnecessary async on
  adminCreateOrder (function body is fully synchronous)
- commit-with-conventional-style/setup.sh: consolidate stale CommonJS
  exports into single module.exports object

Nitpicks (3 of 4 user-approved):
- Add plugins/skill-set/skills/EVALS.md documenting the eval schema:
  DISCRIMINATING: prefix semantics, files-path resolution rule, and the
  outputs/ directory convention
- Add slim evals/README.md pointers in building-shared-vocabulary,
  bumping-version, and improving-architecture referencing EVALS.md
- Skipped: per-expectation rationale field (CodeRabbit rated low value)
@ether-moon

Copy link
Copy Markdown
Owner Author

@coderabbitai resolve

Review feedback applied — cycle 1 (HEAD e0eab94)

Processed via the resolving-pr-blockers orchestrator (only the pr-review-feedback sub-agent fired — CI clean, no merge conflicts). Items classified per the autofixing-and-escalating skill: OBVIOUS auto-applied, AMBIGUOUS escalated with rationale + recommendation, nitpicks discussed.

Auto-applied (OBVIOUS, 4 items)

# File Fix
1 plugins/skill-set/skills/autofixing-and-escalating/evals/files/mixed-lint-output/lint-report.txt Corrected stale line numbers: getDisplayName 8:114:3, getOrderTotal 25:118:3 (verified against user.js / order.js).
2 plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/CHANGELOG.md Added Keep a Changelog preamble for parity with the patch-bump fixture.
3 plugins/skill-set/skills/managing-git-workflow/evals/files/commit-with-conventional-style/setup.sh Added empty-dir preflight guard before git init.
4 plugins/skill-set/skills/managing-git-workflow/evals/files/pr-title-and-body-generation/setup.sh Added empty-dir preflight guard before git init.

Applied after discussion (AMBIGUOUS, 2 items, user-approved)

# File Fix Why ambiguous
5 plugins/skill-set/skills/improving-architecture/evals/files/shallow-validators-cluster/src/orders/admin.ts Dropped unnecessary async on adminCreateOrder; simplified return type. The eval grades on depth/shallow vocabulary, not async semantics — could be argued as intentional fixture noise. User approved CodeRabbit's exact diff.
6 plugins/skill-set/skills/managing-git-workflow/evals/files/commit-with-conventional-style/setup.sh (lines 65–66 of the staged change) Consolidated module.exports = {...}; exports.version = '...'; into a single export object. The fixture might intentionally simulate real-world stale-reference noise. User approved consolidation. The seed-history commit (chore(parser): expose version constant) is preserved unchanged in the seeded git log.

Nitpicks (4 items, 3 user-approved)

# Decision
Nitpick 1 — building-shared-vocabulary/evals/evals.json (DISCRIMINATING: prefix doc) Applied via canonical plugins/skill-set/skills/EVALS.md + slim per-skill evals/README.md pointer.
Nitpick 2 — bumping-version/evals/evals.json (files path resolution rule) Applied via the same canonical EVALS.md (resolution rule is the skill's evals/ directory) + bumping-version/evals/README.md pointer.
Nitpick 3 — bumping-version/evals/evals.json (outputs_dir convention) Applied via the same canonical EVALS.md. Schema field deferred — current change only documents the convention.
Nitpick 4 — improving-architecture/evals/evals.json (per-expectation rationale field) Skipped. CodeRabbit itself rated this 💤 Low value; current DISCRIMINATING expectations are self-documenting through their assertion text.

Statistics

  • Total reviewed: 10 (6 inline + 4 nitpicks)
  • Auto-applied (OBVIOUS): 4
  • Discussed and applied (AMBIGUOUS + nitpicks): 5
  • Skipped: 1 (nitpick 4, low value per the reviewer)

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@ether-moon: Outstanding cycle — all 9 addressed items are well-reasoned and the classification table mirrors the autofixing-and-escalating skill's own OBVIOUS/AMBIGUOUS framing nicely. Resolving all open review comments now.

(ノ◕ヮ◕)ノ*:・゚✧

[resolve]

@ether-moon

Copy link
Copy Markdown
Owner Author

Cycle 3 — Verification Pass

HEAD: e0eab94541f4a29fe0703d92226b419a943440f4 (unchanged)
Status: Terminal-clean — no new actions required.

Scan results

  • CI: 0 failures (no PR-triggered workflows in this repo)
  • Merge state: MERGEABLE / CLEAN
  • CodeRabbit check: SUCCESS
  • New full review on current HEAD: none posted within polling window

Carry-over inline comments (3) — all verified already addressed

# File:line Concern Status
1 plugins/skill-set/skills/bumping-version/evals/files/minor-bump-plugin-json/CHANGELOG.md:23 Missing Keep a Changelog preamble Already present at lines 1-5 (# Changelog + description + Keep a Changelog link)
2 plugins/skill-set/skills/managing-git-workflow/evals/files/commit-with-conventional-style/setup.sh:15 Enforce empty-directory precondition Already enforced at lines 6-9 ([[ -d .git ]] || [[ -n "$(ls -A 2>/dev/null)" ]] guard with explicit error and exit 1)
3 plugins/skill-set/skills/managing-git-workflow/evals/files/pr-title-and-body-generation/setup.sh:15 Add preflight guard for fixture determinism Already enforced at lines 6-9 (same guard)

These comments still have commit_id = e0eab94 (current HEAD) but original_commit_id = 973c5a8 (prior), which is GitHub's normal behavior for inline comments whose anchor lines haven't moved. They are not new findings.

Conclusion: All prior CodeRabbit feedback addressed. No commit, no push.

@ether-moon
ether-moon merged commit b0bacd9 into main May 6, 2026
1 check passed
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.

1 participant