Skip to content

Commit 8cd7d9d

Browse files
odinrCopilot
odinr
and
Copilot
authored
fix: actionable diagnostics, PR review comments, and SKILL.md size check (#88)
* fix: actionable diagnostics, regex-explanation separation, PR review comments, SKILL.md size check - Add per-code fix hints to validate-scripts diagnostics - Separate regex-explanation issues from intent-comment heading - Add --json-output flag for structured CI diagnostics - Post inline PR review comments via actions/github-script - Dismiss stale bot reviews on subsequent pushes - Add SKILL.md line-count validation (warn 300+, error 500+) - Document regex-explanation rule in README and contributor guide - Fix pre-existing intent-comment issues in release-prepare Resolves #84 * docs: add cross-platform SKILL.md size limits table and enforce in authoring workflow - Add runtime size comparison table to contribute/02-skill-structure-and-content.md - Add SKILL.md size limits section to copilot-instructions.md - Add size awareness to fusion-skill-authoring Step 4 and validation failure signals - Warning at 300 lines, error at 500 lines * fix: skill-sizes check uses diff-only mode in CI - Add --only-diff and --base-ref flags to validate:skill-sizes - CI only checks SKILL.md files changed in the PR, not the entire catalog - Full scan still available locally without flags * fix: export SKILL.md size thresholds to prevent drift in CLI output Export WARN_THRESHOLD and ERROR_THRESHOLD constants from check-skill-sizes.ts so the CLI uses these values rather than hard-coding them. This ensures the output stays in sync if thresholds are updated in the future. - Export thresholds from check-skill-sizes.ts - Update CLI to import and use thresholds in error messages - Tests verified and code formatting checked * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(validate-scripts): restrict PR diagnostics to changed diff lines Filter JSON diagnostics to changed hunks in --only-diff mode to avoid invalid inline review positions.\n\nHandle both b/path and --no-prefix diff header formats in diff-line parsing.\n\nAdd regression tests for diff-line map extraction and fallback behavior. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(validate-scripts): filter review diagnostics to diff hunks Limit JSON diagnostics in --only-diff mode to changed lines so PR inline review comments don't target non-diff lines. Improve diff parsing for --no-prefix headers and keep validate-scripts rule compliance in parser/test updates. * fix: normalize SKILL.md line counting and harden PR review comment posting - Fix off-by-one line count caused by trailing newlines via getNormalizedLineCount() - Support both Unix (LF) and Windows (CRLF) line endings in SKILL.md size validation - Add boundary-value tests for 300-line warn and 500-line error thresholds - Chunk PR review comments into 50-per-review batches to avoid GitHub API limits - Add error handling and fallback for createReview() failures - Clarify inline comments scope (intent/regex/disallowed diagnostics only) - Fix validate-skills-catalog.yml checkout fetch-depth for diff-only size checks - All validation checks pass: validate:scripts, biome:check, test suite * fix: clarify success message includes regex-explanation checks Update the success log message to explicitly list regex-explanation as one of the checks performed, since it's now a distinct validation category (separate from general intent-comment checks). * chore: add changeset for skill-authoring documentation updates Document the new SKILL.md size limits (300 warn, 500 error) in the skill-authoring guide, including expectations for moving overflow to references/. * fix: format success message for better readability * chore: update devDependencies in package.json - Added "@biomejs/biome" version "^2.4.5" to devDependencies. - Removed duplicate entry for "@biomejs/biome". - Added "biome" version "^0.3.3" to devDependencies. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 8a47b8b commit 8cd7d9d

20 files changed

+1099
-20
lines changed

‎.changeset/skill-authoring-size-limits.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"fusion-skill-authoring": patch
3+
---
4+
5+
Document SKILL.md size limits and CI guardrails
6+
7+
- Document 300-line recommended limit (triggers CI warning)
8+
- Document 500-line hard limit (fails CI)
9+
- Clarify expectation to move overflow to references/ early
10+
- Add failure signal for exceeding size thresholds
11+
12+
Relates to: equinor/fusion-core-tasks#84

‎.github/copilot-instructions.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ Repository-wide guidance for Copilot in this repository.
1919
- For local skill discovery in this repository, search `skills/` first.
2020
- Include `skills/` and `skills/.experimental/` in local `.agents/skills` lookup paths for this workspace.
2121

22+
## SKILL.md size limits
23+
24+
- SKILL.md files must stay under 500 lines (CI error threshold).
25+
- SKILL.md files above 300 lines trigger a CI warning — consider moving content to `references/`.
26+
- See `contribute/02-skill-structure-and-content.md` for the cross-platform size rationale and progressive disclosure guidance.
27+
2228
## Maintainer docs
2329

2430
- For maintainer workflow and policy context, use `contribute/README.md` and the topic files in `contribute/`.

‎.github/workflows/validate-scripts.yml‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ on:
2323

2424
permissions:
2525
contents: read
26+
pull-requests: write
2627

2728
jobs:
2829
validate:
@@ -58,10 +59,13 @@ jobs:
5859
run: bun install --frozen-lockfile
5960

6061
- name: Validate script TSDoc coverage (diff)
62+
id: validate-diff
6163
if: github.event_name == 'pull_request'
62-
run: bun run validate:scripts --only-diff --base-ref origin/${{ github.base_ref }}
64+
continue-on-error: true
65+
run: bun run validate:scripts --only-diff --base-ref origin/${{ github.base_ref }} --json-output .tmp/validate-scripts-diagnostics.json
6366

6467
- name: Validate script TSDoc coverage (full)
68+
id: validate-full
6569
if: github.event_name != 'pull_request'
6670
run: bun run validate:scripts
6771

@@ -73,3 +77,97 @@ jobs:
7377

7478
- name: TypeScript check
7579
run: bunx tsc --noEmit -p tsconfig.json
80+
81+
- name: Post or update PR review comments
82+
if: github.event_name == 'pull_request' && always()
83+
uses: actions/github-script@v7
84+
with:
85+
script: |
86+
const fs = require('fs');
87+
const path = '.tmp/validate-scripts-diagnostics.json';
88+
const marker = '<!-- validate-scripts-review -->';
89+
const { owner, repo } = context.repo;
90+
const pull_number = context.payload.pull_request.number;
91+
const commit_id = context.payload.pull_request.head.sha;
92+
93+
// Read diagnostics (empty array if file missing = no issues).
94+
let diagnostics = [];
95+
if (fs.existsSync(path)) {
96+
diagnostics = JSON.parse(fs.readFileSync(path, 'utf8'));
97+
}
98+
99+
// Find existing bot reviews tagged with our marker.
100+
const reviews = await github.rest.pulls.listReviews({ owner, repo, pull_number });
101+
const botReviews = reviews.data.filter(
102+
r => r.user?.type === 'Bot' && r.body?.includes(marker)
103+
);
104+
105+
// Dismiss stale bot reviews so resolved issues don't linger.
106+
for (const review of botReviews) {
107+
try {
108+
await github.rest.pulls.dismissReview({
109+
owner, repo, pull_number,
110+
review_id: review.id,
111+
message: 'Superseded by newer validation run.',
112+
});
113+
} catch (e) {
114+
core.info(`Could not dismiss review ${review.id}: ${e.message}`);
115+
}
116+
}
117+
118+
if (diagnostics.length === 0) {
119+
core.info('No inline diagnostics for intent/regex/disallowed checks — skipping review.');
120+
return;
121+
}
122+
123+
// Build inline review comments from diff-filtered diagnostics.
124+
const comments = diagnostics.map(d => ({
125+
path: d.path,
126+
line: d.line,
127+
body: `${marker}\n**\`${d.code}\`**: ${d.message}`,
128+
}));
129+
130+
// GitHub limits inline comments per review; chunk to avoid API failures.
131+
const MAX_COMMENTS_PER_REVIEW = 50;
132+
const commentChunks = [];
133+
for (let i = 0; i < comments.length; i += MAX_COMMENTS_PER_REVIEW) {
134+
commentChunks.push(comments.slice(i, i + MAX_COMMENTS_PER_REVIEW));
135+
}
136+
137+
// Post one or more reviews with inline comments.
138+
for (const [index, chunk] of commentChunks.entries()) {
139+
const bodyLines = [
140+
marker,
141+
`**validate-scripts** found ${diagnostics.length} intent/regex/disallowed issue(s).`,
142+
];
143+
144+
if (commentChunks.length > 1) {
145+
bodyLines.push(`Review chunk ${index + 1}/${commentChunks.length}.`);
146+
}
147+
148+
if (index === commentChunks.length - 1) {
149+
bodyLines.push('Fix them to pass CI.');
150+
}
151+
152+
try {
153+
await github.rest.pulls.createReview({
154+
owner,
155+
repo,
156+
pull_number,
157+
commit_id,
158+
event: 'COMMENT',
159+
body: bodyLines.join('\n'),
160+
comments: chunk,
161+
});
162+
} catch (error) {
163+
const message = error instanceof Error ? error.message : String(error);
164+
core.warning(
165+
`Failed to create validate-scripts review chunk ${index + 1}/${commentChunks.length}: ${message}`,
166+
);
167+
break;
168+
}
169+
}
170+
171+
- name: Fail if validation failed
172+
if: github.event_name == 'pull_request' && steps.validate-diff.outcome == 'failure'
173+
run: exit 1

‎.github/workflows/validate-skills-catalog.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414
- name: Checkout repository
1515
uses: actions/checkout@v6
1616
with:
17-
fetch-depth: 1
17+
fetch-depth: 0
1818

1919
- name: Setup Node.js
2020
uses: actions/setup-node@v6
@@ -47,6 +47,10 @@ jobs:
4747
id: validate_ownership
4848
run: bun run validate:ownership
4949

50+
- name: Validate SKILL.md file sizes
51+
id: validate_sizes
52+
run: bun run validate:skill-sizes --only-diff --base-ref origin/${{ github.base_ref || 'main' }}
53+
5054
- name: Job summary
5155
if: always()
5256
run: |
@@ -55,8 +59,10 @@ jobs:
5559
echo
5660
echo "- Skills validation result: ${{ steps.validate_skills.outcome }}"
5761
echo "- Ownership validation result: ${{ steps.validate_ownership.outcome }}"
62+
echo "- SKILL.md size check result: ${{ steps.validate_sizes.outcome }}"
5863
echo
5964
echo "### Commands run"
6065
echo "- \\`bun run validate:skills\\`"
6166
echo "- \\`bun run validate:ownership\\`"
67+
echo "- \\`bun run validate:skill-sizes\\`"
6268
} >> "$GITHUB_STEP_SUMMARY"

‎biome.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"$schema": "https://biomejs.dev/schemas/2.4.4/schema.json",
2+
"$schema": "https://biomejs.dev/schemas/2.4.6/schema.json",
33
"files": {
44
"includes": ["scripts/**/*.ts", "**/*.graphql", "**/*.gql", "package.json", "tsconfig.json"]
55
},

0 commit comments

Comments
 (0)