Skip to content

fix(scripts): address PR #79 dual-blind review findings#80

Merged
danielvm-git merged 3 commits into
mainfrom
fix/pr79-review-findings
Jul 19, 2026
Merged

fix(scripts): address PR #79 dual-blind review findings#80
danielvm-git merged 3 commits into
mainfrom
fix/pr79-review-findings

Conversation

@danielvm-git

Copy link
Copy Markdown
Owner

Addresses all must-fix and should-fix items from the dual-blind code review of #79.

Findings resolved

Must-fix

  • [A-1] Code-block false positives — LINK_RE was applied to raw Markdown including fenced blocks and inline code. Added strip_code_spans() in link_utils.py to blank those before matching; both rewrite_links_for_pi and check-skill-links.py now skip code examples.
  • [A-2] Links with title attributes — [text](target "Title") were silently skipped by [^)\s]+ regex. LINK_RE now captures the optional title group.
  • [B-1] DRY violation — LINK_RE, EXTERNAL_RE, MACHINE_PATH_RE were duplicated between srp-engine.py and check-skill-links.py. Extracted to scripts/lib/link_utils.py; both files import from it.

Should-fix

  • [A-3] Fragment before normpathdoc.md#section/sub was passed whole to os.path.normpath() which could mangle fragments on Windows. Fragment is now split off, path normalised, fragment re-appended.
  • [A-4] Windows regex — r"[A-Z]:\\\\Users" (two backslashes) corrected to r"[A-Z]:\\Users" (one backslash) matching real C:\Users paths.
  • [B-2] Unit tests — Added tests/test_srp_engine.py with 20 tests covering: code blocks, inline code, title attrs, fragments, Windows paths, external/anchor links, relative repointing.
  • [B-3] Undocumented exclusion — check-skill-links.py now 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

python3 tests/test_srp_engine.py   # 20/20 OK
python3 scripts/check-skill-links.py   # OK — no machine paths or dangling links

Closes companion to #79 (intended to be merged together or as a follow-up).

korallis and others added 2 commits July 17, 2026 13:26
- 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
@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

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.
@danielvm-git
danielvm-git merged commit a03ada9 into main Jul 19, 2026
2 checks passed
@danielvm-git
danielvm-git deleted the fix/pr79-review-findings branch July 19, 2026 00:04
github-actions Bot pushed a commit that referenced this pull request Jul 19, 2026
## [2.77.1](v2.77.0...v2.77.1) (2026-07-19)

### Bug Fixes

* **scripts:** address PR [#79](#79) dual-blind review findings ([#80](#80)) ([a03ada9](a03ada9)), closes [#78](#78) [#78](#78)
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 2.77.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants