Skip to content

feat(hook): tag skills loaded by reading their SKILL.md, not only Skill tool invokes - #82

Open
lilpacy wants to merge 1 commit into
langfuse:mainfrom
lilpacy:feat/read-skill-tags-upstream
Open

lilpacy wants to merge 1 commit into
langfuse:mainfrom
lilpacy:feat/read-skill-tags-upstream

Conversation

@lilpacy

@lilpacy lilpacy commented Sep 3, 2026

Copy link
Copy Markdown

Problem

collect_skill_tags covers two skill trails: a Skill tool_use block, and attributionSkill on assistant rows (slash commands). There is a third trail it misses: dependency sub-skills loaded by reading their SKILL.md directly with the Read tool, which is how a skill referenced from another skill's body ("REQUIRED SUB-SKILL: …", "See skills/foo/SKILL.md") typically gets loaded.

Measured on my local transcripts over 4 weeks: a commit-workflow skill that marks a shell-quoting sub-skill as required was invoked 33 times, while the sub-skill collected 0 skill: tags — it was only ever loaded via Read. A sibling Codex setup that counts raw SKILL.md reads saw the same sub-skill in 32 sessions over the same period. Tag-based usage analytics therefore undercount exactly the skills that only ever run as dependencies, and "which skills can I safely disable?" queries give a false zero for them.

Change

add_skill_tags_from_rows now recognizes the third trail: a Read tool_use whose file_path matches (^|/)skills/<name>/SKILL.md$ adds the same skill:<name> tag.

  • The regex is anchored on the SKILL.md filename, so reads of skills/<name>/references/*.md or other files inside a skill directory do not tag.
  • Dedup is unchanged (a turn that both invokes and reads the same skill yields one tag).
  • Subagent rows flow through the same helper, so the subagent-skill: namespace picks the trail up as well.
  • Still gated behind the existing CC_LANGFUSE_SKILL_TAGS option; no new configuration.

Verification

Three new unit tests in tests/unit/test_skill_tags.py:

Test Pins
test_skill_loaded_via_read_of_skill_md_is_tagged the new trail
test_read_of_non_skill_file_is_not_tagged ordinary reads and references/* reads stay untagged
test_read_and_invoke_of_same_skill_dedupes one tag per skill per turn

Full suite: 223 passed.

End to end: a headless claude -p session instructed to only Read a skill's SKILL.md (no Skill invoke) produced a trace tagged ['claude-code', 'skill:goal-first'], confirmed through the metrics API on the tags dimension against a live Langfuse project.

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant

CLAassistant commented Sep 3, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants