fix(docs): comprehensive documentation site review fixes#1578
fix(docs): comprehensive documentation site review fixes#1578bmadcode merged 4 commits intobmad-code-org:mainfrom
Conversation
alexeyv
force-pushed
the
docs/review-fixes
branch
from
354d5d9 to
1cfe398
Compare
|
@coderabbitai review |
🤖 Augment PR SummarySummary: This PR applies a comprehensive documentation-site cleanup for BMad Method, improving navigation consistency, link portability, and docs build tooling. Changes:
Technical Notes: Docs build now exits early on Windows (unsupported) and link validation is run as part of the documentation build pipeline. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughRemoves several BMGD docs and a non-interactive install doc, adds/edits sidebar metadata and many doc pages (link/path fixes, content expansions), updates doc build and link-validation tooling (relative link resolution), removes web bundling npm scripts, and adds/updates tests and build script guards. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (31)
docs/how-to/shard-large-documents.md (11)
8-10:⚠️ Potential issue | 🟠 MajorClarify the deprecation stance and provide the supported alternative.
You open with “Use the shard-doc tool,” then immediately say it’s no longer recommended. That’s conflicting guidance without a supported replacement or transition path. State the preferred alternative and when/why to choose it.
10-10:⚠️ Potential issue | 🟡 Minor“Soon” and “most major llms/tools” are too vague to be actionable.
This reads like speculation. Provide concrete criteria (versions, capabilities, or a roadmap link) or remove the claim.
12-15:⚠️ Potential issue | 🟠 MajorUsage criteria are underspecified and risk misapplication.
“Only use this if your tool/model are failing to load all docs” doesn’t tell readers how to detect failure or what thresholds (tokens/size) warrant sharding. Add concrete symptoms and limits.
18-18:⚠️ Potential issue | 🟠 MajorMissing constraints on heading-derived shards and anchor compatibility.
If shards are based on##headings, document how duplicate headings, emojis, or punctuation map to filenames/anchors to prevent broken links.Based on learnings: “When documenting cross-reference patterns in Markdown, ensure header anchors follow GitHub's anchor generation rules: lowercase, spaces to hyphens, remove emojis/special characters, and strip leading/trailing hyphens.”
22-35:⚠️ Potential issue | 🟠 MajorExample conflicts with later discovery rules.
You showPRD.mdsharding intodocs/prd/, but later discovery says to look fordocument-name/index.md. If the source isPRD.md, discovery impliesdocs/PRD/index.md, notdocs/prd/index.md. Pick one convention and make it explicit.
39-43:⚠️ Potential issue | 🟠 MajorTool invocation lacks context and prerequisites.
The command doesn’t specify whether it’s a CLI, an agent command, or where it must be run (repo root, specific environment). Add prerequisites and execution context so users don’t fail silently.
47-53:⚠️ Potential issue | 🟡 MinorDefault output path behavior is opaque.
The transcript claims a default destination ofdocs/prd/without explaining how it’s derived or how to override it. Document the default path algorithm and override option.
65-67:⚠️ Potential issue | 🟠 MajorDiscovery rules omit path base and case sensitivity.
If the lookup is case-sensitive or relative to a specific root, users will get inconsistent results across systems. Define the exact lookup base and case rules.
67-67:⚠️ Potential issue | 🟠 MajorAdvising users to “remove the whole document” is risky.
Deleting the source file to switch behavior invites data loss and makes diff history painful. Provide a safer toggle (rename, config flag, or explicit precedence rule).
69-75:⚠️ Potential issue | 🟠 Major“All BMM workflows support both formats” is an unqualified guarantee.
This claims full coverage without version bounds or exceptions. If that’s not universally true, it misleads. Scope it to specific versions or list supported workflows.
73-75:⚠️ Potential issue | 🟡 Minor“Automatic detection” and “Transparent to user” need caveats.
Given the discovery precedence and path conventions above, detection can fail in edge cases. Add known limitations so users can troubleshoot.docs/explanation/advanced-elicitation.md (2)
18-22:⚠️ Potential issue | 🟠 MajorAdd a human-review caveat for high‑stakes use.
You recommend advanced elicitation for “high‑stakes content,” but there’s no reminder to verify outputs. That’s a risk for users in compliance, legal, or safety‑critical contexts.Proposed fix
-- For high-stakes content where rethinking helps +- For high-stakes content where rethinking helps (still requires human verification)
23-23:⚠️ Potential issue | 🟡 MinorUse the standardized action phrasing.
“You’ll be asked if you want to run it” contradicts the repo’s action‑instruction standard (“load and follow” / “read and follow”).Based on learnings: “only action instructions … are replaced with ‘load and follow’ or ‘read and follow’.”Proposed fix
-Workflows offer advanced elicitation at decision points - after the LLM has generated something, you'll be asked if you want to run it. +Workflows offer advanced elicitation at decision points - after the LLM has generated something, you'll be asked if you want to load and follow it.docs/explanation/brainstorming.md (1)
12-12:⚠️ Potential issue | 🟡 MinorUse “load and follow” for action instructions.
This is an action instruction and should follow the project’s verb standardization (“load and follow” / “read and follow”).Based on learnings: “only action instructions … are replaced with ‘load and follow’ or ‘read and follow’.”Suggested tweak
-Run `brainstorming` and you've got a creative facilitator pulling ideas out of you - not generating them for you. +Load and follow `brainstorming` and you've got a creative facilitator pulling ideas out of you - not generating them for you.docs/explanation/party-mode.md (3)
12-12:⚠️ Potential issue | 🟡 MinorAction instruction should use “load and follow.”
This is an action directive and should follow the standardized phrasing.Based on learnings: “only action instructions … are replaced with ‘load and follow’ or ‘read and follow’.”Suggested tweak
-Run `party-mode` and you've got your whole AI team in one room - PM, Architect, Dev, UX Designer, whoever you need. +Load and follow `party-mode` and you've got your whole AI team in one room - PM, Architect, Dev, UX Designer, whoever you need.
12-12:⚠️ Potential issue | 🟡 MinorClarify that only installed agents can be selected.
“Picking relevant agents per message” implies any agent, but in practice it should be limited to installed modules. This is a common source of user confusion.Suggested tweak
-BMad Master orchestrates, picking relevant agents per message. +BMad Master orchestrates, picking relevant installed agents per message.
27-36:⚠️ Potential issue | 🟠 MajorExample references TEA without stating the prerequisite.
The TEA agent requires the Test Architect module; readers without that module will see a mismatch. Add a prerequisite note or swap in a core agent.docs/reference/modules.md (1)
74-76:⚠️ Potential issue | 🟡 MinorRoadmap statement may be stale.
“Community modules and a module marketplace are coming” can quickly go out of date. Either link to a current roadmap or rephrase as a conditional statement with no implied timeline.docs/explanation/preventing-agent-conflicts.md (1)
19-22:⚠️ Potential issue | 🟡 MinorOverly absolute guidance can mislead.
“Use GraphQL for all client-server communication” is framed as universal; in practice many stacks are hybrid (REST + GraphQL). Consider wording this as an example decision (“Use GraphQL for X surfaces”) to avoid implying a one-size-fits-all mandate.docs/non-interactive-installation.md (3)
23-36:⚠️ Potential issue | 🟠 MajorMissing explicit list of required flags for fully non-interactive runs.
You say “provide all required flags,” but never enumerate them. In CI, ambiguity causes hangs or partial installs. Add a short required-flag checklist (directory, modules, tools/none, etc.) for the fully non-interactive path.
262-274:⚠️ Potential issue | 🟠 MajorInteractive fallback is dangerous in CI.
If invalid/missing values trigger prompts, non-interactive pipelines can hang. Either document that--yesforces hard failures, or explicitly warn that missing required values will pause the pipeline.
76-79:⚠️ Potential issue | 🟡 Minor
--yesis not a reliable “no tools” option on shared hosts.
Earlier you note--yesuses previously configured tools. In shared runners, that can re-enable tools unexpectedly. Recommend--tools noneexplicitly for true tool-less installs.tools/validate-doc-links.js (1)
233-235:⚠️ Potential issue | 🟡 Minor
findFileWithContextdoesn't strip./or../prefixes from relative paths.When a relative link is broken,
findFileWithContext(linkPath)is called. ButfindFileWithContext(line 156) only strips leading/— it doesn't handle./or../prefixes. This means for a broken relative link like./missing-page.md, it'll search for a file literally named./missing-page.mdand likely find nothing, falling through tomanual-checkeven when the file exists elsewhere and could be auto-fixed.docs/explanation/why-solutioning-matters.md (1)
55-60:⚠️ Potential issue | 🟠 MajorUse canonical track names instead of "Simple" and "Complex" variants.
The table uses "BMad Method Simple" and "BMad Method Complex," but these terms don't appear anywhere else in the documentation or workflow architecture. The canonical track names are:
- Quick Flow (for small changes)
- BMad Method (full process—variants are brownfield/greenfield, not Simple/Complex)
- Enterprise (separate path)
Update the table to match established terminology. If you need to distinguish by project scope or type, use the canonical variants or clarify the distinction explicitly.
docs/index.md (1)
30-37:⚠️ Potential issue | 🟡 MinorOverbroad compatibility claim (“any AI coding assistant”).
This is too absolute and likely false for tools that don’t support file access, multi‑file context, or slash commands. Qualify with minimum capability requirements and/or list known‑supported tools.docs/how-to/get-answers-about-bmad.md (1)
39-45:⚠️ Potential issue | 🟠 MajorChatGPT/Claude.ai “fetch” step is not generally possible.
Those UIs can’t reliably fetch URLs unless browsing/tools are enabled. Provide explicit steps: download the file and upload it, or enable browsing and paste the content.Proposed wording tweak
- Fetch `llms-full.txt` into your session: + Download `llms-full.txt` and upload it to your session (or enable browsing/tools and paste the contents):docs/how-to/established-projects.md (1)
41-42:⚠️ Potential issue | 🟡 MinorCommand is inconsistent and likely wrong without the slash.
Elsewhere you use/bmad-help. Here it’s shown asbmad-help, which reads like a shell command and will confuse users.Proposed fix
- Run `bmad-help` to get guidance when you are not sure what to do next. + Run `/bmad-help` to get guidance when you are not sure what to do next.docs/explanation/established-projects-faq.md (1)
32-38:⚠️ Potential issue | 🟠 MajorSoften "auto-detect" claim in Quick Flow capabilities.
The quick-flow.md documentation describes a conversational discovery process ("You describe what you want to build. Barry scans the codebase to ask informed questions"), not automatic stack detection. Revise line 32-34 from "Auto-detect your existing stack" and "Analyze existing code patterns" to more accurately reflect the workflow: "Scan and map your existing code patterns" or "Analyze code patterns when prompted." The claims about detecting conventions and respecting existing code are already documented and supported elsewhere in the FAQ.
docs/how-to/upgrade-to-v6.md (3)
34-34:⚠️ Potential issue | 🟡 MinorFix the instruction grammar to avoid ambiguity.
Current wording reads as an error and makes the action unclear.
Suggested edit
-If you named your bmad method folder something else - you will need to manual remove the folder yourself. +If you named your BMad Method folder something else, you will need to manually remove that folder yourself.
38-41:⚠️ Potential issue | 🟡 MinorClarify case sensitivity for folder matching.
You say “folders that start with bmad” but the examples use
BMad. On case-sensitive filesystems that’s a different folder. Clarify whether case should be matched exactly and update examples accordingly.
34-38:⚠️ Potential issue | 🟡 MinorAdd a concrete example path for custom legacy folder names.
This instruction leaves readers guessing what to delete. Add a concrete example of a custom folder name and where it would be located to reduce the chance of deleting the wrong directory.
🤖 Fix all issues with AI agents
In `@docs/explanation/advanced-elicitation.md`:
- Line 41: The guidance under the "**Constraint Removal**" bullet is unsafe as
written; modify the sentence to add a clear safety boundary by specifying that
only non-safety operational constraints (e.g., formatting or performance
heuristics) should be temporarily relaxed for diagnostic experiments and that
safety/ethical, legal, and privacy constraints must never be removed; update the
"**Constraint Removal**" line to include a cautionary clause and, if helpful,
add a short example or parenthetical (e.g., "drop non-safety constraints for
debugging only — never disable safety/ethical/privacy constraints").
In `@docs/how-to/quick-fixes.md`:
- Around line 24-45: The Quick Flow path currently stops at loading the Quick
Flow agent; add an explicit next step after the
/bmad-agent-bmm-quick-flow-solo-dev instruction that tells the user to create
and run the quick‑spec and then start the Quick Flow dev cycle (e.g., "Create a
quick‑spec for the change, run the quick‑spec workflow, then start the Quick
Flow dev cycle to implement and review"); reference the Quick Flow Solo Dev
persona and the term "quick‑spec" so readers know to proceed beyond just loading
the agent.
- Around line 69-76: The paragraph claiming the agent "runs your project's test
suite" and "iterates until tests pass" overpromises; update the copy to soften
these guarantees by replacing those absolute phrases (the sentences containing
"Run your project's test suite" and "iterates until tests pass") with wording
that says the agent will attempt to run tests only if the user's
IDE/tooling/permissions allow it and that it will request explicit confirmation
from the user before running or re-running tests; ensure the new text instructs
users to grant permission or run tests manually when tooling is unavailable and
keeps the rest of the checklist behavior unchanged.
In `@docs/reference/commands.md`:
- Line 8: The opening sentence "Slash commands are pre-built prompts that load
agents, run workflows, or execute tasks inside your IDE. The BMad installer
generates them from your installed modules so every command matches what is
actually available in your project." contradicts the later "stale files"
warning; update this paragraph (the sentence starting "The BMad installer
generates them...") to qualify that commands are generated at install/refresh
time and may become stale if modules are removed or files are not cleaned up,
and point readers to the "stale files" section for how to refresh or remove
orphaned commands so the two statements are consistent.
- Around line 62-68: Update the example filenames in the docs to use the current
dash-based naming convention instead of the deprecated underscore form: replace
instances like bmad_bmm_agent_dev.md and bmad_bmm_create-prd.md with
bmad-agent-bmm-dev.md and bmad-bmm-create-prd.md (keeping bmad-help.md). Ensure
the examples align with the installer utility that uses toDashPath (and mark the
underscore variant as `@deprecated` in path-utils.js) and the slash command format
(/bmad-bmm-create-prd).
- Around line 40-70: Update the docs/examples in docs/reference/commands.md to
match the installer output: change directory prefixes from "bmad/..." to
"_bmad/..." (e.g., `_bmad/core/tasks/help`), switch flat filenames from the old
underscore format to the current dash-based format (e.g., `bmad-agent-bmm-pm`
instead of `bmad_bmm_agent_dev`), and replace agent `.md` examples with the
actual `.agent.yaml` extension; confirm the exact naming rules by checking the
installer helper in tools/cli/installers/lib/ide/shared/path-utils.js and
reflect the `_bmad` hierarchy that maps to source `src/...` (e.g.,
`src/core/tasks/help` → `_bmad/core/tasks/help`).
In `@docs/reference/workflow-map.md`:
- Line 16: The iframe embedding the diagram (src="/workflow-map-diagram.html",
title="BMad Method Workflow Map Diagram") lacks a non-iframe fallback; add a
visible plain-text anchor link to the same URL and/or a <noscript> block
directly after the iframe that provides an accessible link with descriptive text
(e.g., "Open the BMad Method Workflow Map diagram") so users who block iframes
or use non-HTML renderers can still access the diagram.
In `@tools/validate-doc-links.js`:
- Around line 116-122: The relative-path branch (checking
checkPath.startsWith('./') || checkPath.startsWith('../')) incorrectly returns a
directory when path.resolve yields a folder and doesn't try index.md; update
that block in tools/validate-doc-links.js (the code manipulating checkPath,
sourceFile and resolved) to mirror the absolute-path handling: after resolving,
compute asFile = resolved and asIndex = path.join(resolved, 'index.md'); if
fs.existsSync(asFile) and fs.statSync(asFile).isFile() return asFile; if
fs.existsSync(asFile + '.md') return asFile + '.md'; if fs.existsSync(asIndex)
return asIndex; otherwise return null; also ensure you handle trailing-slash
links (checkPath.endsWith('/')) by preferring the asIndex check when present.
🟡 Minor comments (15)
docs/explanation/advanced-elicitation.md-47-49 (1)
47-49:⚠️ Potential issue | 🟡 MinorOverbroad recommendation for pre‑mortem.
“Any spec or plan” + “consistently finds gaps” overpromises. This should be scoped (e.g., complex or high‑risk plans).Proposed fix
-Pre-mortem Analysis is a good first pick for any spec or plan. It consistently finds gaps that a standard review misses. +Pre-mortem Analysis is a good first pick for complex or high‑risk specs/plans. It often finds gaps that a standard review misses.docs/explanation/advanced-elicitation.md-34-35 (1)
34-35:⚠️ Potential issue | 🟡 Minor“Dozens of methods” needs a source or link.
You claim “dozens” but only list eight with no link to the full catalog. Either link to the catalog or remove the quantity claim.Proposed fix
-Dozens of reasoning methods are available. A few examples: +A range of reasoning methods are available. A few examples:docs/explanation/advanced-elicitation.md-12-13 (1)
12-13:⚠️ Potential issue | 🟡 MinorSpecify the exact scope of the “second pass.”
“A structured second pass” doesn’t state whether it re-evaluates the last response, a specific section, or the entire workflow output. This is a common user question and leads to mismatched expectations.Proposed fix
-A structured second pass. Instead of asking the AI to "try again" or "make it better," you select a specific reasoning method and the AI re-examines its own output through that lens. +A structured second pass over the most recent output (or the section you select). Instead of asking the AI to "try again" or "make it better," you select a specific reasoning method and the AI re-examines that output through the chosen lens.docs/explanation/advanced-elicitation.md-14-14 (1)
14-14:⚠️ Potential issue | 🟡 MinorAvoid absolute effectiveness claims.
“Would miss” implies guaranteed improvement over a retry. That’s not always true and can be misleading.Proposed fix
-The difference matters. Vague requests produce vague revisions. A named method forces a particular angle of attack, surfacing insights that a generic retry would miss. +The difference matters. Vague requests often produce vague revisions. A named method forces a particular angle of attack, which can surface insights a generic retry might miss.docs/explanation/advanced-elicitation.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorResolve who selects the method (user vs AI).
Line 8 says the user picks the method, but later steps say the AI suggests options first. This ambiguity will confuse users about control flow.Proposed fix
-Make the LLM reconsider what it just generated. You pick a reasoning method, it applies that method to its own output, you decide whether to keep the improvements. +Make the LLM reconsider what it just generated. The AI suggests methods, you choose one, it applies the method to its own output, and you decide whether to keep the improvements.docs/explanation/advanced-elicitation.md-28-28 (1)
28-28:⚠️ Potential issue | 🟡 Minor“Reshuffle” needs an affordance or instruction.
There’s no reference to how the user reshuffles (button, command, prompt). That’s a missing step.Proposed fix
-You pick one (or reshuffle for different options) +You pick one (or use the “reshuffle” option in the UI for different suggestions)docs/explanation/brainstorming.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorQuantified claim needs verification or a source.
“60+ proven ideation techniques” is a concrete number without attribution. Either cite the canonical list or soften the claim to avoid drifting from the actual inventory.Possible revision
-description: Interactive creative sessions using 60+ proven ideation techniques +description: Interactive creative sessions using a large set of proven ideation techniquesdocs/explanation/adversarial-review.md-29-29 (1)
29-29:⚠️ Potential issue | 🟡 MinorBroad claim needs concrete references.
“Appears throughout BMad workflows” is vague and unverifiable. Add 1–2 explicit workflow names or links so readers can confirm where it’s used.docs/explanation/quick-flow.md-46-49 (1)
46-49:⚠️ Potential issue | 🟡 MinorClarify the syntax in lines 48–49 to show
quick-devas a chat trigger, not a shell command.The commands shown (
quick-dev tech-spec-auth.mdandquick-dev "refactor the auth middleware") use argument-passing syntax that resembles shell commands. However, based on the repository's command reference,quick-devis a chat trigger (with the full form/bmad-bmm-quick-dev) likeQAandCR. Users invoke it conversationally, and the file or instruction is provided as part of the chat context—not as a command-line argument.The current notation risks confusing users into thinking they should run this in a terminal. Reframe these examples to clarify they are conversational inputs to the agent, either by restructuring the examples or by adding a note explaining the chat interaction model.
docs/how-to/get-answers-about-bmad.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorUnsubstantiated “80% of all questions” metric.
That’s a precise claim with no cited basis. Remove or qualify as anecdotal, or provide a source.docs/how-to/quick-fixes.md-17-20 (1)
17-20:⚠️ Potential issue | 🟡 MinorPrerequisites are too narrow for the described use case.
You require an install, but Quick Fixes can be used by users who only have repo access and a cloned_bmad/directory. Align this prerequisite with the “get answers” guidance to avoid blocking legitimate use.Proposed wording tweak
- - BMad Method installed (`npx bmad-method install`) + - BMad Method installed (`npx bmad-method install`) or access to a repo with `_bmad/` presentdocs/how-to/quick-fixes.md-88-90 (1)
88-90:⚠️ Potential issue | 🟡 Minor
git revert HEADguidance is incomplete for common failure modes.
This only reverts the last commit and ignores uncommitted changes or multi‑commit fixes. Add a note to stash/discard working tree changes first and to revert the specific commit hash when multiple commits are involved.Proposed wording tweak
- If a committed change causes unexpected issues, use `git revert HEAD` to undo the last commit cleanly. + If a committed change causes unexpected issues, first stash/discard any uncommitted changes, then revert the specific commit hash (e.g., `git revert <hash>`). `git revert HEAD` only covers the latest commit.docs/reference/workflow-map.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorRemove contradictory border styles.
The style sets
border: none;and then immediately overrides it withborder: 1px solid .... This is confusing and makes the intent unclear. Keep a single declaration.docs/reference/commands.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorOverstates where tasks run.
“Execute tasks inside your IDE” is misleading; the commands trigger AI prompts, not IDE-executed tasks. Please rephrase to avoid implying the IDE itself performs the work.
docs/how-to/install-bmad.md-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorFix the incomplete instruction and ambiguity.
The sentence reads as a fragment and makes the action unclear. Also “non interactive” reads like a noun phrase, not the installer name, which can confuse readers. Consider making the action explicit.
Suggested edit
-If you want to use a non interactive installer and provide all install options on the command line, [this guide](../non-interactive-installation.md). +If you want to use the non-interactive installer and provide all install options on the command line, see [this guide](../non-interactive-installation.md).
🧹 Nitpick comments (1)
tools/build-docs.mjs (1)
239-247: Dead sort key for removedbmgd/docs.
getLlmSortKeystill assigns a sort priority tobmgd/paths (line 245), but the PR removes the BMGD docs. Combined withbmgd/being inLLM_EXCLUDE_PATTERNS(line 40), this branch can never be reached. It's harmless but misleading — a future maintainer might think BMGD content is still expected.Remove dead branch
if (filePath.startsWith(`reference${path.sep}`) || filePath.startsWith('reference/')) return 5; - if (filePath.startsWith(`bmgd${path.sep}`) || filePath.startsWith('bmgd/')) return 6; return 7;
alexeyv
force-pushed
the
docs/review-fixes
branch
from
87e5b07 to
114063d
Compare
alexeyv
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
docs/404.md (3)
1-9:⚠️ Potential issue | 🟠 MajorThe 404 page doesn't leverage your documentation's navigation structure.
By using the
splashtemplate, this page likely strips away the standard documentation sidebar, header, and navigation that users expect. This disorients users further. Unless there's a specific reason to use a splash layout, the default documentation template would provide:
- Persistent sidebar navigation
- Site-wide search
- Breadcrumbs
- Header with main navigation links
These elements would significantly improve the user experience when encountering an error.
1-4:⚠️ Potential issue | 🟡 MinorDescription metadata would improve consistency, but robots configuration requires HTML meta tags, not frontmatter.
The frontmatter lacks a
descriptionfield that other documentation pages include (e.g.,docs/index.md,docs/how-to/install-bmad.md). Adding one would be consistent with the documentation pattern.However,
robots: noindexcannot be set in markdown frontmatter in Astro/Starlight. The Astro sitemap plugin automatically excludes 404 pages, and robots directives must be configured via HTML meta tags inastro.config.mjsif needed—not in page frontmatter. Thetemplate: splashis already appropriate for error page handling.
1-4:⚠️ Potential issue | 🔴 CriticalChange the template from
splashtodocfor improved error recovery experience.The
splashtemplate is designed specifically for landing pages with a wide layout and no sidebars. On a 404 error page, this removes critical navigation context and search functionality that helps users recover from the error. Use the defaultdoctemplate instead to preserve sidebars and site navigation, allowing users to find their way through the documentation after encountering the error..github/workflows/quality.yaml (1)
3-10:⚠️ Potential issue | 🟡 MinorStale workflow description — "Bundle validation" step no longer exists.
Line 10 references "Bundle validation (web bundle integrity)" but no job or step performs bundle validation. If web bundling npm scripts were removed (per the AI summary), this comment is misleading and should be updated.
docs/how-to/get-answers-about-bmad.md (1)
80-103:⚠️ Potential issue | 🟠 MajorA poem in a how-to guide is inappropriate for reference documentation.
This 24-line poem attributed to "—Claude" adds no informational value to a how-to page and raises questions about AI-generated filler in official docs. Users scanning for actionable guidance will hit this instead. Remove it or move it to a blog/about page if it must exist.
tools/validate-doc-links.js (1)
302-304:⚠️ Potential issue | 🟡 Minor
replaceonly fixes the first occurrence of a duplicate broken link in a file.
String.prototype.replacewith a string (not a regex with/g) replaces only the first match. If the same[text](broken-path)appears multiple times in one file, subsequent occurrences are silently left broken.Proposed fix
- updated = updated.replace(oldLink, newLink); + updated = updated.replaceAll(oldLink, newLink);docs/how-to/shard-large-documents.md (5)
47-61:⚠️ Potential issue | 🟠 MajorInteractive example lacks explanations and error handling.
The example dialogue uses undefined terminology and omits critical scenarios:
Issues:
Line 50: "Agent" is never defined—is this referring to:
- An AI assistant like Claude/ChatGPT?
- The shard-doc tool itself prompting in CLI?
- A workflow automation?
No error scenarios: What if:
- The file doesn't exist?
- The destination already has files?
- The file has no level 2 headings?
- File permissions deny write access?
Missing customization: Lines 53-55 show accepting defaults, but:
- Can users customize section filenames?
- Can they exclude certain sections?
- Can they specify different heading levels?
Line 58: "Created 12 section files"—how were these determined? What if a user expected different number?
📝 Add error handling and clarification
Add subsections after line 61:
+### 3. Troubleshooting Common Issues + +#### File Not Found +```text +Agent: Error: docs/PRD.md not found + Please check the path and try again +``` + +#### Destination Already Exists +```text +Agent: Warning: docs/prd/ already exists + Overwrite? [y/n] +``` + +### 4. Customization Options + +You can customize the sharding process: +- `--exclude <section>`: Skip specific sections +- `--heading-level <n>`: Use different heading level (default: 2) +- `--dry-run`: Preview changes without writing files
63-70:⚠️ Potential issue | 🟠 MajorCritical gotcha buried in misplaced section.
Line 69 contains crucial information—"remove the whole document if you want the sharded to be used instead"—but it's:
- Buried at the end rather than prominently displayed where users will see it
- Awkwardly phrased: "remove the whole document if you want the sharded to be used instead" should be direct: "After sharding, delete the original file to use the sharded version"
- In the wrong section: Workflow discovery implementation details don't belong in a user-facing HOW-TO guide
This creates a trap: users shard their documents, but the system continues using the original file, and they have no idea why.
🔄 Restructure for clarity
Move this critical information to Step 3 (after line 61):
+### 3. Remove the Original File + +:::caution[Important] +After sharding, **delete the original file** (`docs/PRD.md`) to ensure workflows use the sharded version. If both files exist, the whole document takes precedence and your sharded version will be ignored. +::: + +```bash +rm docs/PRD.md +```Optionally, move lines 63-70 to a separate "Technical Details" section at the end or remove entirely if too implementation-focused.
72-78: 🛠️ Refactor suggestion | 🟠 MajorLow-value section that contradicts itself.
This section lists features but provides no actionable information:
Line 78: Claims "Transparent to user"—but if it's truly transparent, why does this guide exist? The very existence of this document proves it's NOT transparent.
Lines 75-77: Bullet points ("Whole documents", "Sharded documents", "Automatic detection") are self-evident from the previous section and add no new information.
Poor information architecture: This should either:
- Be merged into the "Workflow Discovery" section (they cover the same concept), OR
- Be removed entirely as redundant
♻️ Consolidate or remove
Option 1 - Merge into previous section:
## How Workflow Discovery Works -BMad workflows use a **dual discovery system**: +All BMad workflows automatically support both whole and sharded documents using a **dual discovery system**:Option 2 - Remove lines 72-78 entirely as they don't add value beyond what's already explained.
1-78:⚠️ Potential issue | 🟠 MajorGuide missing critical sections for complete user journey.
This guide has significant structural gaps:
Missing sections:
Prerequisites (should be after line 6):
- Required tools/versions
- File system requirements
- Link to installation guide
Reverting/Undoing sharding (should be after line 78):
- How to merge shards back into single file
- When you might need to do this
- Data loss considerations
Troubleshooting (should be after Steps):
- Common error messages and solutions
- Performance issues
- Invalid heading structures
Examples of actual output:
- Show what
index.mdlooks like- Show what a sharded section file contains
- Show how links are transformed
See Also section (should be at end):
- Link to context management docs
- Link to LLM token limit documentation
- Related workflow documentation
Naming inconsistency: Title says "Document Sharding Guide" but tool is "shard-doc"—use consistent terminology throughout.
22-37:⚠️ Potential issue | 🟠 MajorArchitecture section omits critical operational details and misses deprecation context.
Some claimed gaps are actually addressed, but significant documentation holes remain:
Addressed but unclear:
- Line 20 states filenames are "based on level 2 headings," but doesn't explain conversion rules (kebab-case? lowercase? special character handling?).
- Lines 68-69 cover the original file's fate ("remove the whole document if you want the sharded to be used"), though this deserves clearer prominence.
Genuinely missing:
- Line 27: "50k token file"—most users think in KB/MB, not tokens. Use accessible metrics.
- Custom naming: Can users control filenames or section names?
- Duplicate headings: What happens if multiple level 2 headings share the same text?
- Nested headings: How are level 3+ headings within a level 2 section handled?
- Inter-document links: How are internal references updated after sharding? Does the tool handle
[link](../other-section.md)patterns?- Index generation: The example shows
index.mdwith "Table of contents with descriptions"—are these auto-generated or manual?Critical context gap: Lines 10-12 mark this feature as deprecated with no timeline or migration guidance. This should influence readers' decision to use it at all.
🤖 Fix all issues with AI agents
In `@docs/404.md`:
- Around line 7-9: Update docs/404.md to provide helpful recovery options:
replace the lone sentence and the single "[Return to Home](./index.md)" link
with a short explanatory heading, a suggestion to check the URL for typos, an
embedded site search box or a reference to the documentation search, prominent
links to major sections (e.g., Getting Started, API Reference, Guides), a link
to the documentation index/sitemap, and a “Report a broken link” link or issue
template; ensure the content mentions the original missing page and keeps the
existing home link text "[Return to Home](./index.md)" while adding these
navigational aids.
In `@docs/how-to/install-bmad.md`:
- Around line 32-37: The documented bleeding-edge command is invalid because
BMAD-METHOD's package.json exposes binaries named "bmad" and "bmad-method" (not
"install"); update the docs to call the actual binary by changing the command to
"npx github:bmad-code-org/BMAD-METHOD bmad-method install" (or "npx
github:bmad-code-org/BMAD-METHOD bmad install" if that binary supports the same
subcommand), or alternatively document cloning and running "node
./bin/bmad-method install" — reference the package.json bin entries "bmad" and
"bmad-method" when making the correction.
In `@docs/how-to/non-interactive-installation.md`:
- Around line 49-56: Update the Module IDs list to include the official IDs
`cis`, `gds`, and `tea` alongside `bmm` and `bmb`, and replace the term "BMad
Method Master" with the canonical name used in docs/reference/modules.md (e.g.,
"BMM (Agile suite)"); ensure the bullet list reads: `bmm` — BMM (Agile suite),
`bmb` — BMad Builder, `cis` — <official short name>, `gds` — <official short
name>, `tea` — <official short name>, and verify the wording matches modules.md
so CI/CD consumers get the exact IDs and consistent terminology.
- Around line 96-101: The example command incorrectly passes a custom module via
the --modules flag; update the example to pass custom modules using the
--custom-content flag with a filesystem path instead of listing "custom-module"
in --modules. Edit the snippet so --modules only lists built-in modules like
bmm,bmb and add a --custom-content /path/to/custom-module (or multiple
comma-separated paths) to match the behavior expected by the install routine and
the documentation for --custom-content.
In `@docs/how-to/shard-large-documents.md`:
- Around line 10-16: The caution note under the "When to Use This" section
contradicts the usage guidance; update the document by either removing the guide
entirely or converting it into a migration/compatibility section: replace the
:::caution[Deprecated] block with a clear deprecation statement naming a removal
timeline (e.g., "Deprecated — will be removed in vX.Y or on YYYY-MM-DD"), add a
"Migration and Alternatives" subsection that explains what "subprocesses" means
for users, links to the updated workflow docs, lists concrete alternative
approaches/tools, and replace the vague symptom "failing to load and read all
the documents" with specific, observable failure indicators (e.g., token
truncation, model input errors, partial document ingestion) and step-by-step
fallback instructions for the current tool.
- Line 16: Replace the vague sentence "Only use this if you notice your chosen
tool / model combination is failing to load and read all the documents as input
when needed." with concrete, observable criteria and examples: list specific
symptoms (e.g., "context length exceeded" or "input too long" errors, truncated
or incomplete responses referencing missing sections, runtime failures when
ingesting large files), show example error messages and common
model/context-size thresholds (e.g., token limits), clarify "tool / model
combination" by naming examples (e.g., LLM + retriever/embeddings pipeline) and
give a short decision checklist (if you see X, Y, or Z then apply sharding) so
readers know exactly when to shard large documents.
- Around line 18-21: Update the "Document Sharding" page to either remove the
guide and point readers to the deprecation note, or add a concise "Limitations"
subsection that explicitly states how the sharder behaves: it splits only on
level-2 headings (##) and preserves other heading levels inside shards rather
than promoting or stripping them; YAML frontmatter is not reliably preserved by
the underlying implementation (shard-doc.xml → `@kayvan/markdown-tree-parser`) so
note whether frontmatter is copied to each shard, only to an index, or
discarded; internal anchor links and relative links are not rewritten (so links
like [text](`#anchor`) or relative image paths may break after sharding); and
recommend migration steps or workarounds and reiterate the deprecation and where
to find the supported replacement.
🟡 Minor comments (17)
docs/404.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorUse extensionless link format for Starlight routing.
The link should be
[Return to Home](./index)without the.mdextension. Starlight generates extensionless routes from markdown files and expects links to match the built URLs, not the source file paths. The Starlight documentation explicitly states: "generated routes are extensionless (e.g..../reference/faq, notfaq.md)." Note: This appears to be a systemic issue across the documentation—other files throughoutdocs/also use.mdextensions in links and should be updated to match Starlight best practices.docs/how-to/non-interactive-installation.md-56-56 (1)
56-56:⚠️ Potential issue | 🟡 Minor"BMad registry" links to the GitHub org page, not a registry.
The link text says "BMad registry" but points to
https://github.com/bmad-code-org, which is the GitHub organization page. If there's no formal registry, call it what it is (e.g., "BMad GitHub organization") to avoid implying infrastructure that doesn't exist.docs/how-to/get-answers-about-bmad.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorInconsistent command name:
/bmad-helpvsbmad-help.Line 8 uses
/bmad-help(with leading slash) but line 41 usesbmad-help(without). One of these is wrong and will confuse users trying to follow the guide. Reconcile to the correct invocation.docs/how-to/non-interactive-installation.md-8-16 (1)
8-16:⚠️ Potential issue | 🟡 MinorDangling introductory sentence — "This is useful for:" leads into a heading, not a list.
Line 8 ends with "This is useful for:" but the next element is the
## When to Use Thisheading on line 10. The colon promises an inline list that never comes. Either remove the trailing clause from line 8 or remove the## When to Use Thisheading and let the bullet list follow the sentence directly.Proposed fix
-Use command-line flags to install BMad non-interactively. This is useful for: - -## When to Use This +Use command-line flags to install BMad non-interactively. + +## When to Use Thisdocs/how-to/non-interactive-installation.md-131-140 (1)
131-140:⚠️ Potential issue | 🟡 MinorSilent failures on invalid module/tool IDs undermine CI/CD reliability — document the risk.
Lines 132-133 state invalid module and tool IDs produce warnings but don't fail. For CI/CD pipelines (the primary audience of this page), this means a typo in
--modulessilently produces an incomplete installation. At minimum, document this gotcha prominently — or better, mention the--debugflag in the CI/CD example so operators can catch these warnings in logs.docs/reference/testing.md-86-86 (1)
86-86:⚠️ Potential issue | 🟡 Minor"MCP tooling" is an unexplained acronym with no link.
Readers unfamiliar with MCP (Model Context Protocol? Something else?) won't know what this refers to. Either expand the acronym on first use or link to documentation.
docs/how-to/customize-bmad.md-79-79 (1)
79-79:⚠️ Potential issue | 🟡 MinorPartial persona replacement silently drops unset fields — this needs a stronger warning or a safe example.
"Include all four fields if you set it" is easy to miss. If someone sets only
role, they loseidentity,communication_style, andprincipleswith no error. Consider showing a minimal complete example or adding a caution admonition.Suggested improvement
-The `persona` section replaces the entire default persona, so include all four fields if you set it. +:::caution +The `persona` section **replaces** the entire default persona. If you set it, include all four fields (`role`, `identity`, `communication_style`, `principles`) — any omitted field will be lost, not inherited from the default. +:::test/test-rehype-plugins.mjs-628-633 (1)
628-633:⚠️ Potential issue | 🟡 MinorWeak assertion: only checks the output is not
.md, never asserts what it should be.This test verifies the plugin doesn't leave
.mdunchanged but doesn't assert the expected output. If the plugin produces garbage (e.g.,/guide/), this test still passes. Pin down the expected value.Proposed fix
{ // .md bare -> processes (not left as ".md") const tree = makeAnchorTree('.md'); transform(tree, STD_FILE, STD_OPTS); - assert(getHref(tree) !== '.md', '.md bare -> processes (not left as ".md")', `Got ${getHref(tree)}`); + // Verify the expected resolved path for a bare `.md` input + const expected = '/guide/'; // or whatever the correct resolution is + assert(getHref(tree) === expected, '.md bare -> resolves correctly', `Expected ${expected}, got ${getHref(tree)}`); }docs/reference/workflow-map.md-28-28 (1)
28-28:⚠️ Potential issue | 🟡 MinorUpdate the style guide documentation to match the renamed command.
The workflow is correctly named
brainstormingin both the actual workflow definition and indocs/reference/workflow-map.md. However,docs/_STYLE_GUIDE.md(line 67) still references the oldbrainstormcommand. This creates user confusion when they see different command names across documentation. The style guide needs to be updated to showbrainstorminginstead ofbrainstorm.docs/reference/commands.md-76-78 (1)
76-78:⚠️ Potential issue | 🟡 MinorAgent commands description says the agent "stays in character and responds to menu triggers" — but menu triggers are a separate mechanism.
This conflates two ideas introduced earlier in the page: slash commands load agents, and menu triggers are a different invocation mechanism (per the table on lines 14-17). Saying an agent "responds to menu triggers" in the agent commands section blurs the distinction you just drew. Rephrase to clarify that loading an agent via slash command enables the use of menu triggers in the subsequent session.
docs/reference/commands.md-114-124 (1)
114-124:⚠️ Potential issue | 🟡 MinorModule code
gdsis listed but no commands with that prefix appear anywhere in the examples.The naming convention table and module code list include
gds(Game Dev Studio), but no example command in this page uses it. If that module doesn't ship default commands yet, note it explicitly so users don't wonder why/bmad-gds-*autocomplete turns up empty.docs/how-to/quick-fixes.md-88-92 (1)
88-92:⚠️ Potential issue | 🟡 Minor
git revert HEADadvice doesn't mention uncommitted changes.The caution says to use
git revert HEADif something breaks, but if the user hasn't committed yet (they're told to commit on line 88, but might not have),git revert HEADwill revert the wrong commit. The flow should distinguish between "undo uncommitted changes" (git checkout .) and "revert a committed change" (git revert HEAD).docs/how-to/upgrade-to-v6.md-34-34 (1)
34-34:⚠️ Potential issue | 🟡 MinorCasing inconsistency: "bmad method" should be "BMad Method".
This PR explicitly standardizes casing to "BMad Method" throughout the docs, but line 34 uses lowercase "bmad method." Also, "manually remove the folder yourself" is redundant — "manually" and "yourself" convey the same idea.
Fix
-If you named your bmad method folder something else - you will need to manually remove the folder yourself. +If you named your BMad Method folder something else, you will need to remove it manually.docs/reference/agents.md-8-16 (1)
8-16:⚠️ Potential issue | 🟡 Minor"## Notes" heading creates a misleading document structure.
"## Default Agents" (line 8) contains only one sentence before "## Notes" (line 12) appears as a sibling heading. The notes are about the default agents, so they should be nested under that section (e.g., as body text or a
:::noteadmonition), not promoted to a peer heading. As-is, the TOC shows "Default Agents" and "Notes" as siblings, which suggests "Notes" is a separate topic.Suggested restructuring
## Default Agents This page lists the default BMM (Agile suite) agents that install with BMad Method, along with their menu triggers and primary workflows. -## Notes - -- Triggers are the short menu codes (e.g., `CP`) and fuzzy matches shown in each agent menu. -- Slash commands are generated separately. See [Commands](./commands.md) for the slash command list and where they are defined. -- QA (Quinn) is the lightweight test automation agent in BMM. The full Test Architect (TEA) lives in its own module. +:::note +- Triggers are the short menu codes (e.g., `CP`) and fuzzy matches shown in each agent menu. +- Slash commands are generated separately. See [Commands](./commands.md) for the slash command list and where they are defined. +- QA (Quinn) is the lightweight test automation agent in BMM. The full Test Architect (TEA) lives in its own module. +:::docs/reference/commands.md-29-32 (1)
29-32:⚠️ Potential issue | 🟡 MinorClarify workflow file format support in the command description.
Line 30 currently states: "Loads the workflow engine (
workflow.xml) and passes the workflow config". Whileworkflow.xmlis accurate, the documentation is incomplete because workflows can also use YAML format (as shown in customize-bmad.md). Additionally, the phrasing "workflow engine" is imprecise—the file contains workflow definitions/configuration, not the engine itself. Revise to: "Loads the workflow definition (workflow.xmlor.yaml) and passes the workflow config" or similar to reflect both supported formats.docs/how-to/shard-large-documents.md-4-5 (1)
4-5:⚠️ Potential issue | 🟡 MinorDeprecated feature has prominent sidebar placement.
Lines 4-5 set
sidebar.order: 8, giving this guide a mid-range position in the how-to guides. However, line 10-12 mark this feature as deprecated.Issue: Deprecated features should not have prominent navigation placement, as it encourages use of discouraged patterns.
Options:
- If truly deprecated: Move to a "Legacy" or "Deprecated" section with higher order number (e.g., 900+) or mark in sidebar title
- If still supported: Remove the deprecation notice and provide clear timeline for when it will actually be deprecated
- If removal is imminent: Consider removing the guide entirely and replacing with a redirect to the new approach
The current state sends mixed signals about whether users should reference this guide.
docs/how-to/shard-large-documents.md-41-45 (1)
41-45:⚠️ Potential issue | 🟡 MinorAdd reference to installation prerequisites and clarify execution context.
Line 44 shows
/bmad-shard-docwithout linking to setup documentation. While the Agent/User dialogue (lines 47-61) hints at IDE-based execution, readers would benefit from:
- Explicit context: State that this runs as a slash command in your AI IDE (Claude Code, Cursor, etc.)
- Prerequisite link: Reference the installation guide or note that
npx bmad-method installmust be run first- Consider the deprecation: This guide is marked deprecated (lines 10-12) — consider whether continued documentation is worthwhile
Note:
docs/reference/commands.mdcomprehensively documents command execution and installation prerequisites, but linking to it from this guide would improve standalone usability.
🧹 Nitpick comments (19)
docs/404.md (6)
9-9: The link text "Home" is ambiguous in a documentation context."Return to Home" could mean the project homepage, the documentation homepage, or the repository. Since this file is in
/docs, consider being explicit: "Return to Documentation Home" or "Back to Documentation Index".
7-7: The error message provides no actionable context.The message "The page you're looking for doesn't exist or has been moved" states the obvious but offers no help. Consider adding guidance such as:
- "This may happen if you followed an outdated link or typed the URL incorrectly."
- "Try searching for the content you're looking for using the search box above."
- "If you believe this is an error, please report it."
Users need direction, not just confirmation that they're lost.
7-9: Missing search functionality on the error page.Documentation sites typically surface search prominently on 404 pages since users are already lost. If your static site generator supports it, consider adding a search component or at minimum a link to a search page. This is one of the most effective tools for converting a 404 into a successful navigation.
1-9: No mechanism to report broken links or documentation errors.When users hit a 404, they've often followed a broken link from elsewhere in your docs or from external sources. Providing a way to report the broken link (GitHub issue link with pre-filled template, feedback form, etc.) would help you identify and fix documentation problems proactively.
Would you like me to generate a sample enhanced 404 page with search integration and helpful navigation links?
1-9: Consider adding popular/common documentation pages.Effective 404 pages often include quick links to the most frequently accessed documentation sections. Based on your sidebar structure, consider adding links to:
- Getting Started / Installation
- Core Concepts
- API Reference
- Troubleshooting
This gives users immediate value rather than forcing them to navigate from the home page.
7-9: No accessibility considerations apparent.While the content is simple, there are no ARIA attributes, role declarations, or semantic enhancements that would help screen reader users understand they've encountered an error state. Consider:
- Adding
role="main"to the content area- Using semantic HTML if the template allows customization
- Ensuring focus management after the error page loads
docs/explanation/quick-flow.md (2)
66-73: Heading "Escalating to Full BMad Method" covers light escalation too, which isn't the full method.Line 70's "Light escalation" just recommends running
quick-specfirst — that's still within Quick Flow, not the full BMad Method. The heading implies all escalation paths lead to the full method. Consider renaming to something like "Escalation Paths" or "When Quick Flow Isn't Enough" to accurately cover both cases.
36-41: Add cross-reference links for "adversarial review" and "advanced elicitation" in Step 4.Line 40 mentions these features by name without linking to their documentation pages. Since both
docs/explanation/adversarial-review.mdanddocs/explanation/advanced-elicitation.mdexist, link them to improve discoverability:[adversarial review](./adversarial-review.md)and[advanced elicitation](./advanced-elicitation.md).docs/reference/workflow-map.md (1)
12-12: Inconsistent formatting of/bmad-help— should use backticks like other commands.This paragraph references
/bmad-helpthree times as a command but never wraps it in backticks, unlike every other command/workflow name in this doc (e.g.,brainstorming,create-product-brief,QA). Wrap all three occurrences in backticks for consistency.test/test-rehype-plugins.mjs (2)
32-49: Custom test harness with global mutable counters — consider Node's built-in test runner.Node 18+ ships with
node:testandnode:assertwhich provide structured test output, proper isolation, and TAP/spec reporters out of the box. The custom harness here works but adds maintenance burden and lacks features like--filter,--watch, or CI-friendly output formats.Not blocking, but worth considering for future test files.
880-891: Consider documenting the intentional srcset exclusion in the plugin code.The plugin explicitly excludes
srcsetattributes from base-path prefixing (onlysrcandhrefare supported, as documented in the header comment and verified by the test). While this is an intentional design decision, there's no explanation in the plugin code itself explaining whysrcsetis excluded or acknowledging it as a known limitation.If a site uses responsive images with absolute paths in
srcsetattributes, they won't receive base-path prefixing — this behavior is correct but could be surprising. Consider adding a comment in therehypeBasePathsfunction documenting this constraint, or open an issue to track it as a potential future enhancement.docs/explanation/advanced-elicitation.md (2)
23-23: "Workflows offer advanced elicitation at decision points" — which ones?This is the only sentence that connects this concept to actual usage, but it doesn't name any specific workflow or command. A user reading this page still doesn't know where they'll encounter the prompt. Consider naming at least one workflow (e.g.,
quick-specstep 4) or linking to the workflow map.
27-30: The "How It Works" steps are context-free — who is "LLM" and where does this happen?Step 1 says "LLM suggests 5 relevant methods" but doesn't clarify this happens within a workflow during an interactive session. A reader unfamiliar with BMad could think this is a standalone feature they invoke directly. One sentence of context framing the interaction would help.
docs/reference/testing.md (1)
98-98: "Phase 4 (Implementation) of the BMad Method workflow map" references the map without linking.Line 106 links to it, but this earlier reference should also link since users scanning may not reach the bottom. Or restructure so the link appears at first mention.
tools/validate-doc-links.js (1)
25-25: Regex won't match bare relative.mdlinks that start with a non-word character.The second alternative
[\w][^)\s]*\.mdrequires the link to start with[a-zA-Z0-9_]. A link like[text](-page.md)or[text](+notes.md)would be silently skipped. This is likely a rare edge case, but worth documenting in a comment if intentional.docs/how-to/customize-bmad.md (1)
97-104: Add documentation explaining the{project-root}variable.
{project-root}is a supported variable that the system automatically expands to the project's root directory. Readers copying this example will succeed, but the documentation should clarify what the variable represents so they understand how to use it in their own custom paths. Add a brief note explaining the variable substitution (e.g., "where{project-root}resolves to your project's root directory").tools/build-docs.mjs (1)
55-59: Windows guard exits the entire process — consider whether CI could run on Windows.The hard
process.exit(1)is appropriate for a build script that genuinely can't work on Windows (likely due to shell commands inexecSync). However, the error message doesn't explain why Windows isn't supported, which makes debugging harder if someone encounters it unexpectedly in CI.docs/reference/commands.md (1)
70-72: "Canonical list" claim may confuse users who customized their install.The tip says the generated command folders are "the canonical list," but users who manually added or removed command files (or who have stale files per the troubleshooting section) would see a list that doesn't match the actual module selection. Consider softening to "the current list of installed commands" or similar.
docs/how-to/upgrade-to-v6.md (1)
68-77: Directory tree doesn't show_config/agents/purpose or that modules are optional.The tree shows
_config/agents/with the comment "Agent customization files" but doesn't clarify that this is where users override default agents (not where agents are installed). New users could confuse this with the module agent directories. A one-liner clarification would help.
|
ignore the last (6th? 10th? I'm not even sure) round of reviews - enough is enough |
Rehype plugins: - Rewrite rehype-markdown-links for correct relative .md resolution - Handle raw HTML base paths and bare .md links in rehype-base-paths - Guard protocol-relative URLs (//...) in all link processors - Use file.path instead of file.history[0] for vfile compatibility - Fail build when content directory cannot be detected - Export helpers for testability; add 107 unit tests Build & CI: - Revert cancel-in-progress to false to avoid mid-deploy cancellation - Remove redundant link-validation CI step (build validates internally) - Remove unnecessary fetch-depth:0 from docs deploy workflow - Refuse docs build on Windows (platform guard) - Remove dead build scripts and stale references Tooling: - Add DOCS_ROOT boundary check in validate-doc-links.js - Handle directory paths and prefix stripping in link validator - Remove dead regex and add // guard in fix-doc-links.js Accessibility & CSS: - Darken caution/danger aside title colors for WCAG AA 4.5:1 contrast - Fix 100vw scrollbar overflow (banner width:100%, html overflow-x:clip) - Add :focus-visible ring to banner link for keyboard navigation - Remove dead CSS declaration and add missing code block lang Documentation content: - Convert /docs/ absolute links to relative paths and fix llms.txt URLs - Correct command file paths and naming in commands reference - Update stale shard-doc command to current /bmad-shard-doc format - Fix incomplete sentence in install-bmad.md - Add Quick Flow next steps and fix 404 link path - Expand thin content pages with substantive detail - Add sidebar ordering frontmatter to all content pages - Remove BMGD docs (moved to dedicated repo) - Remove unused assets and misleading diagram caption - Add non-iframe fallback link to workflow map diagram - Remove dead noscript block from workflow-map - Standardize BMAD to BMad, fix spelling/grammar, normalize headings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eorder how-to guides - Move non-interactive-installation.md into how-to/ directory so it appears in the sidebar navigation (was orphaned at docs root) - Rewrite the page based on editorial review: consolidate redundant sections, add missing how-to structure (prerequisites, "What You Get"), condense installation modes from 5 subsections to a table, cut speculative examples - Reorder how-to sidebar: Install (1), Non-Interactive (2), Upgrade to v6 (3), then the rest following user journey order - Fix README link to point to docs site instead of repo-internal markdown path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix broken directory tree, grammar errors, inconsistent naming, missing admonition/headings, enable lastUpdated timestamps in CI, and remove footer CSS that misapplied to the content footer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexeyv
force-pushed
the
docs/review-fixes
branch
from
0f7ba6d to
f7bee04
Compare
The alternative npx command for installing from main was misplaced inside the "Verify Installation" section. Move it to a tip admonition under Step 1 where users look for install options. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexeyv
force-pushed
the
docs/review-fixes
branch
from
f7bee04 to
49eb4e8
Compare
2 participants
Summary
└──vs├──for siblings):::cautionadmonition (shard-large-documents)## Default Agentsand## Notesheadings to agents reference for TOC navigationbrainstorm→brainstormingto match/bmad-brainstormingcommandlastUpdatedtimestamps in CI by addingfetch-depth: 0to checkoutTest plan
npm test)docs:build)🤖 Generated with Claude Code