fix(scripts): address PR #79 dual-blind review findings#80
Merged
Conversation
- replace file:///Users/danielvm countable-story-format links with the GitHub blob URL (docs/ is excluded from the npm package, so the file cannot be linked relatively in published installs) - evolve-skill: benchmark repo path is now machine-agnostic - craft-skill/investigate-bug/plan-refactor: docs/ links -> blob URLs - using-bigpowers: profiles link -> repo-relative; write-document: CHANGELOG/LICENSE -> repo-relative; guard-git: hooks/ -> REFERENCE.md (the bundle lives there); audit-code: drop dead REFERENCE.md link - grill-me/model-domain: de-link illustrative example paths - srp-engine: rewrite relative links for the pi tree so they resolve from .pi/skills/<name>/ against the shipped source tree - pi adapter: render skill files from body_pi_skill (prompts unchanged) - add scripts/check-skill-links.py regression gate, wired into the compliance npm script Fixes #78
Must-fix: - [A-1] skip code fences and inline code before LINK_RE matching in both rewrite_links_for_pi and check-skill-links; uses strip_code_spans() helper - [A-2] LINK_RE now captures links with title attributes [text](url "title") - [B-1] DRY: extract LINK_RE, EXTERNAL_RE, MACHINE_PATH_RE, strip_code_spans to scripts/lib/link_utils.py; both srp-engine and check-skill-links import from the single source of truth Should-fix: - [A-3] fragment split before os.path.normpath in rewrite_links_for_pi to prevent mangling of #section/sub on Windows - [A-4] Windows MACHINE_PATH_RE corrected to r'[A-Z]:\Users' (single backslash) - [B-2] add tests/test_srp_engine.py with 20 unit tests covering all edge cases: code blocks, title attrs, fragments, Windows paths, external/anchor links, image links, inline-code links - [B-3] document exclusion of .cursor/.gemini dirs in check-skill-links.py with a TODO comment referencing issue #78
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Changes: - scripts/generate-reference-tables.sh: resolve skill paths relative to REPO_ROOT fordocs/references/model-profiles.md. This prevents environment-absolute paths (like local checkout paths or CI /tmp paths) from causing dirty checkouts. - Regenerate .pi/skills and docs/references/model-profiles.md using sync-skills.sh.
|
🎉 This PR is included in version 2.77.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses all must-fix and should-fix items from the dual-blind code review of #79.
Findings resolved
Must-fix
LINK_REwas applied to raw Markdown including fencedblocksand inlinecode. Addedstrip_code_spans()inlink_utils.pyto blank those before matching; bothrewrite_links_for_piandcheck-skill-links.pynow skip code examples.[text](target "Title")were silently skipped by[^)\s]+regex.LINK_REnow captures the optional title group.LINK_RE,EXTERNAL_RE,MACHINE_PATH_REwere duplicated betweensrp-engine.pyandcheck-skill-links.py. Extracted toscripts/lib/link_utils.py; both files import from it.Should-fix
normpath—doc.md#section/subwas passed whole toos.path.normpath()which could mangle fragments on Windows. Fragment is now split off, path normalised, fragment re-appended.r"[A-Z]:\\\\Users"(two backslashes) corrected tor"[A-Z]:\\Users"(one backslash) matching realC:\Userspaths.tests/test_srp_engine.pywith 20 tests covering: code blocks, inline code, title attrs, fragments, Windows paths, external/anchor links, relative repointing.check-skill-links.pynow has a TODO comment explaining why.cursor/and.gemini/are excluded, referencing fix: hardcoded author absolute paths + stripped reference files break 45+ links in .pi/skills tree #78.Verify
Closes companion to #79 (intended to be merged together or as a follow-up).