Repository navigation
fix(skills): keep only the PowerShell vally safety lint - #3142
Jamie Kim (jkim323) merged 6 commits into
Conversation
Bill Berry (WilliamBerryiii)
left a comment
There was a problem hiding this comment.
Thank you for this PR, Mohammed Alkindi (@MohammedAlkindi), and for tracking down both root causes in #3135. The new test cases for case-insensitivity, escaped brackets, [\s\-], and wrapped lines are a real improvement.
We also owe you an apology. This skill should never have had a bash safety lint. It already requires PowerShell 7+, and Lint-VallyTestSafety.ps1 is the lint that import_corpus.py actually runs, so the bash mirror only added a second parser that could drift. That's on us, and we're sorry it cost you time to debug.
Rather than bring in perl, we'd like to move to PowerShell only:
- Delete
scripts/lint-vally-test-safety.shand the bash tests, and keep your newCASESon the PowerShell lint. - Add the two inputs from #3135 and a UTF-16LE-with-BOM stimulus to
CASES. - Remove the bash lint row from the SKILL.md Helper Script Index, bump
metadata.last_updated, and update the engine sentence inreferences/refusal-taxonomy.mdand the test module docstring. - Run
npm run validate:localand the vally-tests pytest and Pester suites, and add the results to the Testing section. Updating the title and Type of Change to reflect the removed script would help too.
The inline comments have the details. If you'd rather not take this on, just let us know and we're happy to pick it up, with credit to you for the diagnosis. Please comment if you have questions about any of it.
|
Done in 924cdeb. The bash lint and its tests are gone, and your new cases, the two #3135 inputs and a UTF-16LE-with-BOM stimulus now run against |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3142 +/- ##
==========================================
- Coverage 86.79% 83.73% -3.07%
==========================================
Files 132 87 -45
Lines 15197 11383 -3814
Branches 46 0 -46
==========================================
- Hits 13190 9531 -3659
+ Misses 2001 1852 -149
+ Partials 6 0 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…-bash # Conflicts: # .github/skills/hve-core/vally-tests/SKILL.md
Pull Request
Description
Removes the bash safety lint, as asked in review:
Lint-VallyTestSafety.ps1is the lintimport_corpus.pyruns, and the bash copy had drifted.Related Issue(s)
Fixes #3135
Type of Change
.github/skills/*/SKILL.md).ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
N/A
Testing
The PowerShell cases now include both #3135 inputs and a UTF-16LE file with BOM. pytest 48 passed, Pester 50 passed;
validate:localpasses exceptlint:md-links.Checklist
Required Checks
AI Artifact Contributions
N/A
Required Local Checks
npm run validate:localnpm run validate:docsnpm run spell-checknpm run lint:md-linksSecurity Considerations
Additional Notes
None.