Skip to content

fix: validate suggests absolute path for relative link errors#9

Merged
akeemjenkins merged 3 commits into
mainfrom
fix/validate-relative-link-suggestions
Jul 8, 2026
Merged

fix: validate suggests absolute path for relative link errors#9
akeemjenkins merged 3 commits into
mainfrom
fix/validate-relative-link-suggestions

Conversation

@akeemjenkins

Copy link
Copy Markdown
Contributor

Problem

Relative markdown links like [Cloaked](organizations/cloaked) from a concept at pages/about.md resolve to pages/organizations/cloaked (broken). The correct form is [Cloaked](/organizations/cloaked) (absolute). The error message didn't explain this gotcha.

Fix

When validate finds a broken link, it checks if the target would resolve as an absolute path. If so, the error message suggests the absolute path: use /organizations/cloaked for an absolute path.

@akeemjenkins

Copy link
Copy Markdown
Contributor Author

Review — reviewed against current main (d641562), test-merged locally

Verified: merges cleanly; go build / go vet / go test ./... pass, and golangci-lint is clean (0 issues) on the merged result.

The fix is correct and safe. When a relative link fails to resolve, the absolute-path suggestion is only emitted after b.HasConcept(absTarget) confirms it actually resolves, so it never proposes a path that isn't real. Already-absolute-but-broken links, external URLs, and root-level concepts (where relative and absolute coincide) all correctly avoid a spurious suggestion. Good doc comments, and both the positive (suggestion present) and negative (genuinely-missing concept must not mention "absolute path") cases are tested.

Nits (optional)

  • absoluteTarget re-implements what resolveLink("", link) already produces (an empty fromConceptID gives the bundle-root interpretation). Reusing it drops ~18 lines.
  • The suggestion strips any #fragment (e.g. organizations/cloaked#s → suggests /organizations/cloaked). Cosmetic — fragments are ignored for resolution anyway — but preserving it would be friendlier.
  • Add a test for the already-absolute-and-broken case to lock in that it does not suggest.

For the record: an earlier note worried the test file wasn't gofmt-clean. CI does not run a gofmt/gofumpt linter (.golangci.yml enables govet, staticcheck, errcheck, ineffassign, unused, revive, gosec, misspell), and the merged branch lints clean — so that is not a blocker.

Recommendation: approve — the nits are optional follow-ups.

🤖 Automated review via Claude Code, verified locally against current main.

akeemjenkins and others added 2 commits July 8, 2026 10:48
Address review nits on the relative-link absolute-path suggestion:

- Replace the duplicated absoluteTarget helper with resolveLink("", link),
  whose empty fromConceptID yields the bundle-root interpretation. Guard the
  suggestion on absTarget != target so an already-absolute broken link (and
  root-level concepts where relative and absolute coincide) falls through to
  the plain broken-link message rather than suggesting itself.
- Preserve the #fragment on the suggested absolute path (e.g.
  organizations/cloaked#section -> /organizations/cloaked#section).
  Fragments remain ignored for concept resolution.
- Add tests for the already-absolute-and-broken case (no "absolute path"
  suggestion) and for fragment preservation in the suggestion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@akeemjenkins akeemjenkins merged commit 9a30c75 into main Jul 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant