lint: add skillsaw job that lints#231
Conversation
Reviewer's GuideAdds a Skillsaw linting workflow to CI with strict mode and token limits, fixes broken internal references in skill docs, and adjusts documentation links and Skillsaw configuration to ignore or tune specific rules. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider pinning
stbenjam/skillsawto a specific commit SHA or at least a stable major tag (e.g.@v0.15.0if available) instead of@v0to avoid unexpected behavior from upstream changes. - The
.skillsaw.yamlhard-codesversion: "0.15.0"while the action is floated at@v0; it may be worth aligning these (or documenting the expected update process) so the workflow and config don’t silently drift apart.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider pinning `stbenjam/skillsaw` to a specific commit SHA or at least a stable major tag (e.g. `@v0.15.0` if available) instead of `@v0` to avoid unexpected behavior from upstream changes.
- The `.skillsaw.yaml` hard-codes `version: "0.15.0"` while the action is floated at `@v0`; it may be worth aligning these (or documenting the expected update process) so the workflow and config don’t silently drift apart.
## Individual Comments
### Comment 1
<location path=".github/workflows/skillsaw.yml" line_range="18" />
<code_context>
+ uses: actions/checkout@v4
+
+ - name: Run skillsaw
+ uses: stbenjam/skillsaw@v0
+ with:
+ strict: true
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider pinning the skillsaw action to a specific commit SHA instead of a floating tag.
Floating tags like `@v0` can change over time, introducing unexpected behavior or breaking changes. Pinning to a specific commit SHA (and optionally noting the corresponding release) improves supply-chain security and ensures the workflow remains stable until you intentionally update it.
Suggested implementation:
```
- name: Run skillsaw
uses: stbenjam/skillsaw@<PINNED_COMMIT_SHA>
with:
strict: true
```
1. Replace `<PINNED_COMMIT_SHA>` with the actual 40-character commit SHA of the `stbenjam/skillsaw` release you want to use (for example, the commit behind the current `v0` tag).
2. Optionally, you can add a YAML comment above the `uses:` line documenting which tag/release this SHA corresponds to (e.g., `# skillsaw v0.3.1`), to make future updates easier.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
0a3c1ee to
5af2640
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider scoping the new
Skillsawworkflow with apathsfilter (e.g., skills content and.skillsaw.yaml) so that linting doesn’t run on unrelated changes and slow down CI unnecessarily. - The
.skillsaw.yamlfile pinsversion: "0.15.0"; it may be worth adding a brief comment in that file or in CLAUDE.md about updating this in lockstep with the GitHub Action version to avoid drift between local and CI lint configurations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider scoping the new `Skillsaw` workflow with a `paths` filter (e.g., skills content and `.skillsaw.yaml`) so that linting doesn’t run on unrelated changes and slow down CI unnecessarily.
- The `.skillsaw.yaml` file pins `version: "0.15.0"`; it may be worth adding a brief comment in that file or in CLAUDE.md about updating this in lockstep with the GitHub Action version to avoid drift between local and CI lint configurations.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Eval Results
Eval Results: plan-feature
| Eval | Passed | Failed | Pass Rate |
|---|---|---|---|
| eval-1 | 19/19 | 0 | 100% |
| eval-2 | 16/16 | 0 | 100% |
| eval-3 | 15/15 | 0 | 100% |
| eval-4 | 11/11 | 0 | 100% |
| eval-5 | 15/15 | 0 | 100% |
| eval-6 | 14/14 | 0 | 100% |
Pass rate: 100% · Tokens: 75,965 · Duration: 317s
Baseline (7b46435b): 98% · 81,180 tokens · 334s
Eval Results: triage-bug
| Eval | Passed | Failed | Pass Rate |
|---|---|---|---|
| eval-1 | 9/9 | 0 | 100% |
| eval-2 | 4/4 | 0 | 100% |
| eval-3 | 5/5 | 0 | 100% |
Pass rate: 100% · Tokens: 31,805 · Duration: 61s
Baseline (7b46435b): 100% · 20,911 tokens · 53s
Eval Results: verify-pr
| Eval | Passed | Failed | Pass Rate |
|---|---|---|---|
| eval-1 | 12/12 | 0 | 100% |
| eval-2 | 11/11 | 0 | 100% |
| eval-3 | 12/14 | 2 | 86% |
| eval-4 | 10/10 | 0 | 100% |
| eval-5 | 10/10 | 0 | 100% |
| eval-6 | 10/10 | 0 | 100% |
Failed Assertions
eval-3: 2 failing assertions
-
Assertion: "Each sub-task description includes a Target PR section with the PR URL https://github.com/trustify/trustify-backend/pull/744 (constraint 1.12)"
Evidence: "subtask-1.md has '## Target PR' (line 31) with URL 'https://github.com/trustify/trustify-backend/pull/744' (line 32) -- PASS. However, subtask-2.md (root-cause task, 25 lines total) does NOT contain a '## Target PR' section anywhere in the file. The assertion requires 'each' sub-task description to include this section, and subtask-2.md fails this requirement." -
Assertion: "Each sub-task description includes a Review Context section with the original review comment text (constraint 1.12)"
Evidence: "subtask-1.md has '## Review Context' (lines 34-38) with comment ID 30001, author 'reviewer-a', file path, and full comment text -- PASS. However, subtask-2.md (root-cause task, 25 lines total) does NOT contain a '## Review Context' section anywhere in the file. The assertion requires 'each' sub-task description to include this section, and subtask-2.md fails this requirement."
Pass rate: 98% · Tokens: 61,292 · Duration: 246s
Baseline (7b46435b): 91% · 79,591 tokens · 392s
Generated by sdlc-workflow/run-evals v0.12.2
5af2640 to
ea4d441
Compare
mrizzi
left a comment
There was a problem hiding this comment.
@rh-jfuller thanks: skill lint was missing but great to have and I can already see the improvements in this PR 👏
Adding skillsaw check for skills ... setting 16k token maximum - anything over will throw an error:
There are a bunch of content errors which I leave to @mrizzi to contemplate (perhaps in another PR)
fixed obvious broken references and tweaked .skillsaw config to ignore some things.
Summary by Sourcery
Add automated Skillsaw linting for skills content in CI with configured token limits.
Enhancements:
Build:
CI:
Summary by Sourcery
Add automated Skillsaw linting for skills documentation and fix internal documentation links.
Enhancements:
Build:
CI:
Documentation: