Skip to content

perf(mentoring): trim always-on routing metadata of the family's skills - #1563

Merged
potiuk merged 4 commits into
apache:mainfrom
liwenjie200543:perf/mentoring-frontmatter-budget
Oct 9, 2026
Merged

potiuk merged 4 commits into
apache:mainfrom
liwenjie200543:perf/mentoring-frontmatter-budget

Conversation

@liwenjie200543

Copy link
Copy Markdown
Contributor

Fixes #1351 (part of the #1342 family budget umbrella).

What

Trims the always-on routing metadata (frontmatter description + when_to_use) of three mentoring-family skills that exceeded the 200-token family budget, and syncs measured_tokens accordingly. Skill bodies are untouched, so behavior is unchanged:

Skill always-on before → after
good-first-issue-sweep 224 → 162 tok
newcomer-issue-explainer 233 → 164 tok
welcome 203 → 180 tok

good-first-issue-author already measured 168 tokens and is left as-is. The docs/setup/marketplace.md family aggregate stays at ~0.4k, so no doc-table change is needed.

Verification

  • Re-counted chars÷4 for every edited frontmatter after trimming: all three now under the 200-token budget while keeping trigger phrases, sibling-skill disambiguation, and guardrails.
  • Ran the repo's own skill-and-tool-validator over the full tree: no findings attributable to the three edited files.
  • Follows the pattern of the merged trim PRs in this family (e.g. perf(contributor-growth): trim activity-sweep skill routing metadata #1483).

Per the repo AI-contribution policy, the commit carries the Generated-by: trailer.

@liwenjie200543
liwenjie200543 force-pushed the perf/mentoring-frontmatter-budget branch from d9338f8 to 94d08b2 Compare October 9, 2026 07:27
@github-actions github-actions Bot added capability:triage Sweep + classify + propose disposition capability:review Deep per-item code review or contributor mentoring family:mentoring mentoring skills labels Oct 9, 2026

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this. The trims themselves look reasonable, but the second commit (9a8591c1f, "sync measured_tokens") converted all three SKILL.md files from LF to CRLF line endings, which needs to be undone before this can merge.

Blocking — CRLF line endings in all three files

The first commit (94d08b21e) keeps LF line endings like main. The second one changes three measured_tokens values but rewrites every line as CRLF, so the diff reads as +954/−962 instead of the actual +24/−32 and every line's blame history is lost. (CI did not catch it: the mixed-line-ending hook only fails files that mix the two styles.)

Please convert the files back to LF, for example:

sed -i 's/\r$//' plugins/magpie-mentoring/skills/{good-first-issue-sweep,newcomer-issue-explainer,welcome}/SKILL.md
uv run --project tools/skill-token-count skill-token-count --write

and re-stamp measured_tokens afterwards, since the current values were measured on the CRLF files. Setting git config core.autocrlf input (or making your editor save with LF) avoids it recurring. git diff --stat main...HEAD should then show only the frontmatter lines changing.

Smaller observations

  • The PR body says trigger phrases were kept, but several were dropped. In particular welcome was only 3 tokens over budget and now has ~20 tokens of headroom; please consider keeping "send the first-time contributor message on NNN" there, and "curate the backlog for newcomers" in good-first-issue-sweep, which has ~38 tokens of headroom. (Inline comments below.)
  • newcomer-issue-explainer: "Read-only until confirmed." reads as a contradiction. (Inline comment below.)

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. After you've
addressed the points above and pushed an update, an Apache Magpie
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Apache Magpie handles maintainer review:
Contributing guide.

Comment thread plugins/magpie-mentoring/skills/welcome/SKILL.md Outdated
Comment thread plugins/magpie-mentoring/skills/good-first-issue-sweep/SKILL.md Outdated
Comment thread plugins/magpie-mentoring/skills/newcomer-issue-explainer/SKILL.md Outdated
potiuk added a commit that referenced this pull request Oct 9, 2026
The mixed-line-ending hook ran with its default --fix=auto, which only
fails a file that mixes LF and CRLF. A file converted wholesale to CRLF
(an editor on Windows, core.autocrlf=true) passed every check and landed
as a whole-file rewrite, as on #1563, where a three-number change showed
up as +954/-962.

--fix=lf converts any CRLF to LF and fails the run, so such a change is
caught locally and in CI. No file in the tree uses CRLF today, so the
whole-tree run on main is unaffected.

Generated-by: Claude Opus 5
@liwenjie200543

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — all three points are addressed in commit 39430af.

CRLF line endings (blocking): Found and fixed. The root cause: the second commit was generated by rewriting the files with Python's text mode on Windows, which silently translated every LF to CRLF. All three SKILL.md files are now converted back to LF, and measured_tokens was re-stamped on the LF files with tools/skill-token-count (tiktoken 0.14.0, cl100k_base): sweep 4319 / newcomer 3563 / welcome 3450. git diff --stat main...HEAD now shows only the frontmatter lines changing (+22/−29).

Trigger phrases and wording: Restored "curate the backlog for newcomers" in good-first-issue-sweep and "send the first-time contributor message on NNN" in welcome, as suggested. Applied the suggested newcomer-issue-explainer wording: "Posts nothing without explicit maintainer confirmation."

CI is green on the new head (11/11 checks, including measure and prek).

Three mentoring skills exceeded the 200-token always-on budget from
apache#1351: good-first-issue-sweep (224), newcomer-issue-explainer (233)
and welcome (203). Condense their frontmatter description and
when_to_use while keeping trigger phrases, sibling disambiguation and
safety guardrails, then sync measured_tokens. Bodies are untouched.

Generated-by: liwenjie200543 (with AI assistance, per docs/ai-contribution-policy.md)
CI's skill-token-count (tiktoken) reported small deltas from the
chars/4 estimates: sweep 4302->4310, welcome 3439->3437,
newcomer-issue-explainer 3550->3560.

Generated-by: liwenjie200543 (with AI assistance, per docs/ai-contribution-policy.md)
The previous commit re-encoded the three SKILL.md files as CRLF; this
restores LF so blame and the diff stay clean. Also restores two
trigger phrases dropped in the trim ("curate the backlog for
newcomers" in good-first-issue-sweep, "send the first-time contributor
message on NNN" in welcome) and rewords the explainer guardrail to
"Posts nothing without explicit maintainer confirmation." per review.

measured_tokens re-stamped with tools/skill-token-count
(tiktoken 0.14.0, cl100k_base): sweep 4319, newcomer 3563, welcome 3450.

Generated-by: liwenjie200543 (with AI assistance, per docs/ai-contribution-policy.md)
@potiuk
potiuk force-pushed the perf/mentoring-frontmatter-budget branch from 39430af to 47317d5 Compare October 9, 2026 13:59
Wrap the newcomer-issue-explainer guardrail sentence and the welcome
skip clause at the same width as the surrounding frontmatter.

Generated-by: Claude Opus 5

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. All three points from the last review are addressed: the files are back to LF and the diff is now just the frontmatter (+22/−29), the two trigger phrases are restored, and the explainer guardrail reads cleanly. Thanks for the quick turnaround.

I pushed one small fixup on top that rewraps two lines the trim left uneven (and re-stamps measured_tokens to match).


This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Magpie handles maintainer review:
Contributing guide.

@potiuk
potiuk merged commit bdbd66d into apache:main Oct 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

capability:review Deep per-item code review or contributor mentoring capability:triage Sweep + classify + propose disposition family:mentoring mentoring skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize the mentoring skill family

2 participants