refactor(me/evolve): fix path/flag bugs + simplify Phase 0 & 1 - #666
Conversation
…KILL.md
The slash-command shell does not guarantee ${CLAUDE_PLUGIN_ROOT} is set.
Use the "Base directory for this skill" value injected into the command
prompt header, which is always present.
Also documents the exit-2 case for unknown flags so the dry-run pitfall
is visible at the bun invocation line.
build-index.ts's parseArgs exits with code 2 on unknown flags, so forwarding --dry-run would crash Phase 0. SKILL.md now makes the ownership explicit: --dry-run is consumed by the main agent only.
The 사용법 block advertised --since and skill-name filters that build-index.ts has never implemented. Mark each variant explicitly and note the divergence so future Phase 1 subagents don't propose changes based on phantom features.
Previously detectSlashCommand returned only the command name, so when the user invoked /me:evolve with arguments (e.g. via <command-args>), the actual user intent was discarded — only the bare slash event survived. The Phase 1 subagent then had no signal about *what* the user asked. Now skill events carry an optional `args` field (≤200 chars) extracted from <command-args> tag or the prefix-form remainder.
build-index.ts emitted large_out events for tool_result bodies over 10KB, but SKILL.md's Phase 1 mapping table never consumed them — pure write-only signal. Removes: large_out EventKind, bytes field, LARGE_OUTPUT_THRESHOLD, and the now-unused ToolResultPayload.size field (orphan cleanup).
signal_positions duplicated cluster data in a flatter form. The LLM already has events[] for granular walks and clusters for dense-region detection — the per-kind turn list added no new information for the mapping table to consume. Simplifies Summary, removes buildSignalPositions, and tightens the SKILL.md guidance from three sub-fields to two.
The subagent was instructed to return a skipped[] array of low-confidence events, but Phase 2 never read it. Pure write-only output costing tokens on every run. classifications[] already labels noise/question events, and unproposed events are implicitly skipped.
The seven-row signal→file table over-specified what the LLM already infers from context. Every row's 1st-choice column resolved to one of three targets (SKILL.md / AGENTS.md / CLAUDE.md), so the discriminator that matters is "what kind of knowledge is missing," not "which signal pattern fired." Replaces the table with three target descriptions keyed on missing- knowledge type. The "Nearest" resolution rule — the actually load- bearing piece — is preserved verbatim.
|
Warning Review limit reached
More reviews will be available in 56 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesEvolve Skill Schema and Instruction Alignment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Three tests verified behavior that the refactor commits removed: - "summary has signal_positions" — field deleted in bd9d17a - "detects large_out event above 10KB" — kind deleted in 9b2287d - "signal_positions kinds appear in events" — same field Replace with: - positive assertion that summary has no signal_positions key - positive assertion that large_out events are never emitted - updated kind allowlists in two coverage tests Also drops large_out from the allowlist in the false-positive guard.
1 participant
Summary
Two phases applied to the
/me:evolveskill:Changes
Fixes
f7e21c6e— SKILL.md: use theBase directory for this skillvalue injected at the command-prompt header instead of${CLAUDE_PLUGIN_ROOT}(the env var is not guaranteed in the slash-command shell).91de3f04— SKILL.md:--dry-runis consumed by the main agent only; forwarding tobuild-index.tswould crash on the unknown-flag exit-2 path.4b26bca5— spec: mark each/me:evolveusage variant as implemented or unimplemented so future Phase 1 subagents don't propose changes for phantom features.99b09526— build-index.ts: preserve slash-command args (<command-args>tag or prefix-form remainder, ≤200 chars) on skill events so Phase 1 has the user's actual intent, not just the bare command name.Simplifications (subagent-proposed, F1–F4)
9b2287d5— droplarge_outEventKind,bytesfield,LARGE_OUTPUT_THRESHOLD, and the orphanedToolResultPayload.size. Pure write-only signal; Phase 1 mapping table never consumed it.bd9d17a8— dropsignal_positionsfrom Summary.clusters+headlinealready give the LLM dense-region detection and totals;events[]covers sparse cases.c796bc13— dropskippedfield from the Phase 1 proposal schema. Phase 2 never read it.ecb7f4f1— collapse the 7-row signal→file mapping table into 3 target rules keyed on missing-knowledge type (skill rules / repo navigation / project conventions). The "Nearest" resolution rule is preserved.Considered but rejected
repeatdetection into the mainbuildEventsloop) — separation is more readable than the merge would be; not a real perf win.Test plan
bun plugins/me/skills/evolve/scripts/build-index.tsruns against the current session transcript and emits valid JSONsummary.signal_positionsremoved;summarykeys are exactly{headline, clusters}/me:evolvein this very session)large_outevents in current-session outputgit revert <sha>if any individual change is badSummary by CodeRabbit
Documentation
/me:evolvecommand reference with clarified command variants, default behaviors, and argument specifications.Updates
/me:evolveskill with improved event detection and processing accuracy.