Skip to content

Plan 200: move docs/ embed out of internal/lsp/hover.go#374

Open
jeduden wants to merge 4 commits into
mainfrom
claude/nifty-hawking-YsL4f
Open

Plan 200: move docs/ embed out of internal/lsp/hover.go#374
jeduden wants to merge 4 commits into
mainfrom
claude/nifty-hawking-YsL4f

Conversation

@jeduden
Copy link
Copy Markdown
Owner

@jeduden jeduden commented May 23, 2026

Summary

  • internal/lsp/hover.go imported docs/guides/directives as a Go
    package, which put compiled source inside the docs tree and added a
    docs/ layer the architecture map doesn't have.
  • New internal/directives/ holds short, hover-sized stubs (the
    internal/concepts/placeholder-grammar.md pattern) embedded via
    embed.FS. Hover now imports internal/directives instead.
  • The full user guides stay in docs/guides/directives/ and remain
    indexed by docs/guides/index.md. Only embed.go was deleted there.
  • .mdsmith.yml gains internal/directives/** under
    directory-structure.allowed (consent confirmed in chat) so the new
    package's markdown lints under default rules.
  • Plan 200 marked ✅; PLAN.md catalog refreshed.

Test plan

  • go build ./...
  • go test ./...
  • go tool golangci-lint run
  • go run ./cmd/mdsmith check . (352 files, 0 failures)
  • grep -rn 'docs/guides/directives' internal/ --include="*.go"
    returns only a comment in internal/directives/directives.go

Resolves the hover entry from
docs/development/architecture-audit.md
("internal/lsp/hover.go imports from docs/").


Generated by Claude Code

internal/lsp/hover.go imported docs/guides/directives as
a Go package, putting compiled source inside the docs tree
and adding a docs/ layer to the import graph that the
architecture map does not have.

Move the embed to internal/directives, mirroring the
internal/concepts pattern: short hover-sized stubs in
internal/directives/*.md hold the per-directive copy that
the LSP serves; the full user guides stay in
docs/guides/directives/ and remain indexed by the catalog.

Adds internal/directives/** to directory-structure.allowed
so the new package's markdown lints under default rules.
Copilot AI review requested due to automatic review settings May 23, 2026 09:41
The mdsmith-fixed-version CI job runs the pinned v0.15.0
binary against .mdsmith.pinned.yml. The previous commit
added internal/directives/** to .mdsmith.yml but not to
the pinned mirror, so v0.15.0 fired MDS033 on every new
markdown file in the package.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes an architecture boundary violation by stopping internal/lsp/hover.go from importing a Go package under docs/, and instead embedding hover-sized directive docs from a new internal/directives/ package.

Changes:

  • Replaced the LSP hover provider’s dependency on docs/guides/directives with internal/directives (embedded stubs via embed.FS).
  • Added hover-targeted directive stub Markdown files plus a unit test ensuring the embedded files exist and have expected structure.
  • Updated repo structure config and plan tracking to reflect the new internal/directives/** location and completion of Plan 200.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plan/200_arch-fix-hover-embed.md Marks Plan 200 complete and updates tasks/acceptance criteria text to match the implemented approach.
PLAN.md Updates the plan catalog row for Plan 200 to ✅.
internal/lsp/hover.go Switches hover’s directive documentation source to internal/directives and continues using embedded FS loading/caching.
internal/directives/directives.go Introduces the internal/directives package exposing an embedded embed.FS for hover stubs.
internal/directives/directives_test.go Adds a guard test to ensure every directive doc file referenced by hover is embedded and front-matter formatted.
internal/directives/build.md Adds a concise hover stub for the <?build?> directive with a link to the full guide.
internal/directives/enforcing-structure.md Adds a concise hover stub for <?require?> / <?allow-empty-section?> with links to deeper docs.
internal/directives/generating-content.md Adds a concise hover stub for <?catalog?> / <?include?> with examples and a link to the full guide.
docs/guides/directives/embed.go Removes the docs-tree Go package that previously existed solely to host the embedded FS.
.mdsmith.yml Allows internal/directives/** under directory-structure.allowed so the new embedded Markdown stubs lint cleanly.

Comment thread internal/directives/generating-content.md Outdated
@jeduden jeduden marked this pull request as ready for review May 23, 2026 10:05
Copilot review on PR #374: a copy/pasteable plan catalog
example with `sort: id` collates lexicographically (100
before 52). Switch the stub to `sort: numeric:id` so it
matches the recommendation in the full generating-content
guide.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread internal/directives/directives_test.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

jeduden added a commit that referenced this pull request May 24, 2026
* Plan 200: move docs/ embed out of internal/lsp/hover.go

internal/lsp/hover.go imported docs/guides/directives as
a Go package, putting compiled source inside the docs tree
and adding a docs/ layer to the import graph that the
architecture map does not have.

Move the embed to internal/directives, mirroring the
internal/concepts pattern: short hover-sized stubs in
internal/directives/*.md hold the per-directive copy that
the LSP serves; the full user guides stay in
docs/guides/directives/ and remain indexed by the catalog.

Adds internal/directives/** to directory-structure.allowed
so the new package's markdown lints under default rules.

* ci: allow internal/directives/** in pinned mdsmith config

The mdsmith-fixed-version CI job runs the pinned v0.15.0
binary against .mdsmith.pinned.yml. The previous commit
added internal/directives/** to .mdsmith.yml but not to
the pinned mirror, so v0.15.0 fired MDS033 on every new
markdown file in the package.

* fix: use numeric:id sort in plan-catalog stub example

Copilot review on PR #374: a copy/pasteable plan catalog
example with `sort: id` collates lexicographically (100
before 52). Switch the stub to `sort: numeric:id` so it
matches the recommendation in the full generating-content
guide.

* fix: address PR #375 review comments

Two unresolved Copilot threads on PR #375:

generating-content.md: the heading-level paragraph was
backwards. The include rule only accepts
heading-level: "absolute", which SHIFTS included
headings to nest under the include site's parent
heading. Omitting heading-level keeps source levels.
There is no "relative" option — internal/rules/include
rejects any value other than "absolute". Rewrite the
sentence to match.

directives_test.go + new internal/lsp/hover_test.go:
the directives-side test only checks a hardcoded list,
so the comment claiming it guards directiveToDocFile
was misleading. Soften that comment and add the real
cross-package invariant test next to the map: walk
directiveToDocFile in internal/lsp and assert each
mapped file resolves in directives.FS.

* fix: clarify build never executes, include needs fix

Two further Copilot threads on #375:

build.md said "No external tool runs at lint time",
which implies fix might execute the recipe. mdsmith
never runs recipe commands at all — fix only renders
the body-template, and MDS040 is a static check.

generating-content.md said the include body "is
regenerated whenever the source changes", which
implies automatic updates. Make it explicit: mdsmith
fix regenerates; mdsmith check reports drift.

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants