improve zarr-python skill + add skill-review CI#102
Conversation
- expand description with concrete actions + natural trigger terms - trim explanatory content LLMs already know - add verification step to cloud-native workflow - add .github/workflows/skill-review.yml for automated skill review on PRs
Gonzih
left a comment
There was a problem hiding this comment.
The zarr-python skill improvements are solid — the expanded description with concrete trigger terms is a genuine quality improvement that helps agents match user intent, and the verification assert in the cloud-native workflow is a good addition. However, the CI workflow included in this PR raises a concern that needs to be addressed before merging.
Blocking issue: pinned third-party action from an external vendor
- uses: tesslio/skill-review@14b47c8420ff9ac9a12c30ed4b89ce0e9d0355ae # mainThis action is pinned to a commit hash (good practice), but it comes from tesslio/skill-review — an external action maintained by the PR author's employer (tessl.io, disclosed in the PR description). Running this in pull_request context with pull-requests: write permission means every contributor's PR will execute code from a third-party service with write access to the repository's PR comments.
Before this can merge:
- The K-Dense team needs to audit what
tesslio/skill-review@14b47c8420ff9ac9a12c30ed4b89ce0e9d0355aeactually does — specifically whether it exfiltrates code or sends PR content to external servers. - Consider whether a simpler in-repo linting check (e.g., validating YAML frontmatter with a small Python/shell script) would meet the same need without the supply-chain dependency.
- At minimum, the action's source code should be reviewed and the team should be comfortable with the data-handling implications.
Skill changes — no blocking issues
- Removing the "Benefits" bullet lists is fine; the content was explanatory rather than actionable.
- The tightened description is better aligned with how agents match skills.
- The removed overview paragraph was redundant with the Quick Start.
- The verification assert (
assert z_read.shape == z.shape) is a small but useful addition to the cloud workflow example.
The skill-side changes can be merged independently if the CI workflow is separated into its own PR after proper review of the third-party action.
Gonzih
left a comment
There was a problem hiding this comment.
The zarr SKILL.md improvements are good — expanding the description with trigger terms helps with skill discovery, the prose reduction makes it more scannable, and adding the round-trip assert is a nice concrete verification example.
Blocker: tesslio/skill-review action
The CI workflow adds a third-party GitHub Action (tesslio/skill-review@14b47c8...) from tessl.io, which is the contributor's employer. This raises a supply-chain trust concern that needs to be resolved before merge:
- Self-referential trust: A contributor from the company that owns the action is adding that action to this repo's CI. Even with a pinned commit SHA, the maintainers have no way to audit what the action does without reviewing its source.
pull-requests: writepermission: This permission lets the action post comments, approve/request-changes, and modify PR metadata on every SKILL.md PR going forward. That's a significant ongoing trust grant.- No documentation: There's no README or comment explaining what
tesslio/skill-reviewactually does, why this specific action was chosen, or what its output looks like.
Suggestions:
- Either replace with an auditable in-repo script (e.g. a shell/Python linter that checks required SKILL.md sections), or
- Add a link to the action's source, an explanation of what it does, and get explicit sign-off from maintainers on the trust decision.
Please separate the zarr SKILL.md fix into its own PR so it can merge independently while the CI question is resolved.
|
thanks for the thorough review @Gonzih - really appreciate the detail! the action source is fully open at tesslio/skill-review - it runs |
|
Hi - following up on the above. |
hey @K-Dense-AI, thanks for building the scientific skills collection. really like the breadth of coverage across scientific computing domains. Kudos on passing
15kstars! I've just starred it.ran your zarr-python skill through agent evals and spotted a few quick wins that took it from
~49%to~83%performance:expanded the description with concrete actions + natural trigger terms like "Zarr", ".zarr files", "chunked arrays" so agents reliably match user requests
trimmed explanatory content LLMs already know (benefits lists, concept definitions)
added a verification step to the cloud-native workflow pattern
also added a lightweight GitHub Action that freely auto-reviews any skill.md changed in a PR. this means that it gives you and your contributors an instant quality signal before you have to review yourself (no signup, no tokens needed).
these were easy changes to bring the skill in line with what performs well against Anthropic's best practices. honest disclosure, I work at tessl.io where we build tooling around this. not a pitch, just fixes that were straightforward to make! happy to answer any questions on the changes.