Conversation
…rity Add shared docs site validation (published routes, front matter, optional bundle check), wire pre-commit and docs CI, fix overview/guide links and Gemfile.lock, align code-review module docs with the run guide. Harden the git-branch signature flag test: use git commit --no-verify in the fixture repo and scrub inherited GIT_DIR-style env vars so nested git never touches the outer worktree. Tracks openspec change docs-15-code-review-validation-guardrails. Made-with: Cursor
…parity Add requirements-docs-ci.txt and use it in docs workflows; pin specfact-cli in pr-orchestrator. Harden docs_site_validation (README publishability, HTML hrefs, bundle timeout, redirect title). Run check-docs-commands via hatch in pre-commit; fix OpenSpec spec H1s; align run.md and tests with CLI rules. Made-with: Cursor
Document invalid --json/--score-only pairing in the code-review run bundle. Normalize OpenSpec delta spec headings for markdownlint increment rules and align review-run-command spec with --path targeting and published-link cases. Harden docs_site_validation: treat README like other pages for route indexing, catch bundle check timeouts as categorized findings, and fix README links so check-docs-commands passes. Update parity and workflow tests to follow resolver/CLI flags and public check-docs-commands ordering without coupling to helper script paths. Made-with: Cursor
…dation-guardrails docs(docs-15): modules docs validation guardrails and code-review parity
djm81
added
documentation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary for specfact-cli-modules MaintainersBundle and module surface
Manifest and integrity
Cross-repo / specfact-cli dependencies
Docs, publishing, and site validation
OpenSpec change tracking and scenario coverage
Risk and maintainer actions
Notable file-level changes (high-impact)
Overall, this PR enforces stricter, permalink-aware docs validation, aligns Code Review documentation to the actual CLI surface with automated parity checks, and hardens CI/pre-commit gating and dependency pinning to make docs publication and docs-only PR validation more deterministic and reproducible. WalkthroughAdds a docs validation subsystem and integrates it into local pre-commit and CI: new Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Pre as Pre-commit Hook
participant Script as check-docs-commands.py
participant Validator as docs_site_validation.py
participant CI as GitHub Actions
participant Bundler as Bundler/Jekyll
participant Pages as Pages Upload
Dev->>Pre: commit/push (docs files)
Pre->>Script: run python scripts/check-docs-commands.py
Script->>Validator: build routes, scan frontmatter & body links
Validator-->>Script: findings (ok | failures)
alt failures
Script-->>Pre: exit non-zero (fail commit)
else ok
Pre-->>Dev: allow commit
Dev->>CI: push triggers docs workflows
CI->>CI: pip install -r requirements-docs-ci.txt
CI->>Script: python scripts/check-docs-commands.py --jekyll-bundle-check
Script->>Validator: scan_gemfile_lock_installability
Validator->>Bundler: run bundle check
alt bundle & validator ok
CI->>Pages: build & upload artifact
else any failure
CI-->>CI: fail before Pages upload
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/bundles/code-review/run.md`:
- Around line 44-52: Update the docs text to attribute validation to the command
layer and relax file-type wording: change the heading/intro that currently reads
"Typer validates" to "the command validates" (or similar) and clarify that only
the conflicts between --include-tests/--exclude-tests and --focus are enforced
by Typer in _resolve_review_run_flags(), while the other checks (--json vs
--score-only, --out without --json, positional files vs --scope/--path) are
performed by the command validators like _validate_review_request() and its
helpers; also change the user-facing phrase "positional Python file paths" to
"positional file paths" (or "file paths") to avoid implying non-Python targets
are rejected despite internal filtering to .py/.pyi.
In `@docs/modules/code-review.md`:
- Around line 63-73: Add the missing invalid combination to the list: document
that using --json together with --score-only is rejected (i.e., "--json" cannot
be used with "--score-only"); update the "Invalid combinations" section so it
lists this pairing alongside the others and mirror the existing phrasing style
(e.g., "- **`--json` with `--score-only`**: pick one, not both.").
In `@openspec/changes/docs-15-code-review-validation-guardrails/design.md`:
- Line 1: Add a top-level H1 heading before the existing "## Context" heading so
the document has a title (e.g., add a line starting with "# " above "##
Context"); ensure the new H1 is descriptive of the design doc so markdownlint
MD041 is satisfied and the artifact renders with a title.
In `@openspec/changes/docs-15-code-review-validation-guardrails/proposal.md`:
- Line 1: Insert a top-level H1 above the existing "## Why" header (the file
currently starts at "## Why"), e.g., "# Proposal: Code review validation
guardrails" or "# Changes: docs-15 — Code review validation guardrails", so the
document complies with markdownlint MD041 and matches the other OpenSpec docs;
keep the H1 concise and descriptive.
In
`@openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md`:
- Line 19: Update the heading text in the specs document so the compound
modifier is hyphenated: change the phrase "Bundle overview related links" to
"Bundle overview-related links" in the requirement line (the heading starting
"## Requirement: Bundle overview related links") so "overview-related" correctly
modifies "links".
In
`@openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.md`:
- Around line 15-17: The spec text incorrectly narrows `FILES...` to "positional
Python file paths"; update the wording to avoid language that restricts the CLI
contract—reference the public `run` signature (`files: list[Path]`) and the
flags (`--scope`, `--path`, `--focus`, `--include-tests`) and replace
"positional Python file paths" with a neutral phrase such as "positional file
paths" or "positional files", and ensure the alternative guidance still
explains: pass explicit positional files for a fixed review set, or use
`--scope` / `--path` (without positional files) to auto-discover targets.
In `@openspec/changes/docs-15-code-review-validation-guardrails/tasks.md`:
- Line 1: The document starts with a second-level heading "## 1. Governance And
Readiness" which triggers markdownlint MD041; change it to a top-level heading
by replacing it with a single-hash title (e.g., "# 1. Governance And Readiness"
or a clearer main title like "# Governance And Readiness") so the doc has a
proper title and satisfies the linter; update the existing "## 1. Governance And
Readiness" heading text accordingly.
In `@scripts/docs_site_validation.py`:
- Around line 247-259: The directory-target resolution in
_resolve_candidate_markdown_target incorrectly only checks for README.md; update
the candidate.is_dir() branch to also probe candidate / "index.md" (after
README.md) and, if index.md exists and is_docs_markdown(...) returns true,
return _path_lookup_result(index.md.resolve(), target_value, route_to_path,
path_to_route); keep the same is_file()/is_docs_markdown checks and resolution
behavior as for README.md so directory links backed by index.md are treated as
valid.
- Around line 505-515: The current try/except around subprocess.run(["bundle",
"check"], cwd=str(docs_dir), ...) swallows FileNotFoundError and returns an
empty result, making a missing `bundle` look like a passing
--jekyll-bundle-check; instead, when the jekyll bundle check flag is requested,
do not swallow FileNotFoundError: let it propagate (or re-raise a clear
exception) so the caller/CI fails visibly; locate the block that creates `proc`
(uses subprocess.run, `proc`, `docs_dir`) and remove or change the `except
FileNotFoundError: return []` to re-raise or raise a descriptive RuntimeError so
missing Bundler is reported.
In `@tests/unit/docs/test_code_review_docs_parity.py`:
- Around line 46-52: Update the
test_code_review_run_doc_mentions_public_ty_options test to assert presence of
the documented behavioral text (not just flag tokens): after reading RUN_DOC and
the existing flag checks, add explicit assertions that RUN_DOC contains the
progress output string(s) shown during a run, the default mode description
"mode: enforce" (or equivalent phrasing), the semantics of "--level" (e.g.,
descriptions of levels or examples), the documented bug-hunt behavior text, and
the default JSON report path snippet; use the same helpers (_public_option_flags
and _review_run_click_command) to locate options but add these concrete string
assertions (also update the companion assertions in the block referenced by
lines 82-106) so the test fails if the docs drift from the standardized behavior
descriptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 165e47ce-5d05-4b56-82c2-a9c7d570f9b3
⛔ Files ignored due to path filters (1)
docs/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
.github/workflows/docs-pages.yml.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.ymldocs/bundles/backlog/overview.mddocs/bundles/code-review/ledger.mddocs/bundles/code-review/overview.mddocs/bundles/code-review/rules.mddocs/bundles/code-review/run.mddocs/bundles/codebase/analyze.mddocs/bundles/codebase/overview.mddocs/bundles/codebase/repro.mddocs/bundles/govern/overview.mddocs/bundles/project/overview.mddocs/bundles/spec/generate-tests.mddocs/bundles/spec/mock.mddocs/getting-started/README.mddocs/guides/README.mddocs/guides/brownfield-engineer.mddocs/guides/brownfield-faq.mddocs/guides/brownfield-journey.mddocs/guides/brownfield-roi.mddocs/guides/ide-integration.mddocs/guides/openspec-journey.mddocs/modules/code-review.mddocs/reference/README.mdopenspec/CHANGE_ORDER.mdopenspec/changes/docs-15-code-review-validation-guardrails/.openspec.yamlopenspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mdrequirements-docs-ci.txtscripts/check-docs-commands.pyscripts/docs_site_validation.pyscripts/pre-commit-quality-checks.shtests/unit/docs/test_code_review_docs_parity.pytests/unit/docs/test_docs_review.pytests/unit/test_check_docs_commands_script.pytests/unit/test_git_branch_module_signature_flag_script.pytests/unit/test_pre_commit_quality_parity.pytests/unit/workflows/test_pr_orchestrator_signing.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: quality (3.13)
- GitHub Check: quality (3.12)
- GitHub Check: quality (3.11)
🧰 Additional context used
📓 Path-based instructions (6)
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/bundles/spec/mock.mddocs/guides/brownfield-engineer.mddocs/bundles/govern/overview.mddocs/guides/brownfield-faq.mddocs/bundles/code-review/ledger.mddocs/bundles/code-review/overview.mddocs/bundles/project/overview.mddocs/bundles/backlog/overview.mddocs/bundles/codebase/overview.mddocs/bundles/spec/generate-tests.mddocs/guides/ide-integration.mddocs/guides/openspec-journey.mddocs/bundles/codebase/analyze.mddocs/bundles/code-review/rules.mddocs/guides/brownfield-roi.mddocs/getting-started/README.mddocs/bundles/codebase/repro.mddocs/guides/brownfield-journey.mddocs/reference/README.mddocs/guides/README.mddocs/bundles/code-review/run.mddocs/modules/code-review.md
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: CI: secrets, hatch/verify-modules-signature gates, contract-test alignment, action versions.
Files:
.github/workflows/docs-pages.yml.github/workflows/pr-orchestrator.yml.github/workflows/docs-review.yml
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.md
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
tests/unit/test_pre_commit_quality_parity.pytests/unit/test_git_branch_module_signature_flag_script.pytests/unit/workflows/test_pr_orchestrator_signing.pytests/unit/docs/test_code_review_docs_parity.pytests/unit/test_check_docs_commands_script.pytests/unit/docs/test_docs_review.pyscripts/check-docs-commands.pyscripts/docs_site_validation.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/unit/test_pre_commit_quality_parity.pytests/unit/test_git_branch_module_signature_flag_script.pytests/unit/workflows/test_pr_orchestrator_signing.pytests/unit/docs/test_code_review_docs_parity.pytests/unit/test_check_docs_commands_script.pytests/unit/docs/test_docs_review.py
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/check-docs-commands.pyscripts/docs_site_validation.py
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Treat canonical rule docs (docs/agent-rules/INDEX.md) as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Perform `spec -> tests -> failing evidence -> code -> passing evidence` in that order for behavior changes
Applied to files:
docs/bundles/spec/mock.mdopenspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:22.837Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: Use docs/agent-rules/INDEX.md as the canonical governance dispatcher for governance rules
Applied to files:
docs/bundles/govern/overview.md
📚 Learning: 2026-04-13T10:38:22.837Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: This repository enforces the clean-code review gate through hatch run specfact code review run --json --out .specfact/code-review.json
Applied to files:
docs/bundles/code-review/ledger.mddocs/bundles/code-review/overview.mddocs/bundles/code-review/rules.md.github/workflows/pr-orchestrator.ymlopenspec/CHANGE_ORDER.mdopenspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdscripts/pre-commit-quality-checks.shdocs/bundles/code-review/run.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mddocs/modules/code-review.md
📚 Learning: 2026-04-13T10:38:15.842Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-13T10:38:15.842Z
Learning: Adhere to worktree policy, OpenSpec gating, GitHub hierarchy-cache refresh, TDD order, quality gates, versioning, and documentation rules as defined in `docs/agent-rules/`
Applied to files:
docs/bundles/code-review/ledger.mddocs/bundles/backlog/overview.mdopenspec/CHANGE_ORDER.mdopenspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdscripts/pre-commit-quality-checks.shopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Treat canonical rule docs (docs/agent-rules/INDEX.md) as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
docs/bundles/code-review/ledger.mddocs/bundles/backlog/overview.mdopenspec/CHANGE_ORDER.mddocs/guides/README.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.md
📚 Learning: 2026-04-13T10:38:22.837Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: Full governance rules live in docs/agent-rules/; do not treat this file (GitHub Copilot Instructions) as a complete standalone handbook
Applied to files:
docs/bundles/backlog/overview.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Finalize completed OpenSpec changes with `openspec archive <change-id>` and do not manually move change folders under `openspec/changes/archive/`
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/.openspec.yaml
📚 Learning: 2026-04-02T21:49:11.371Z
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Applied to files:
.github/workflows/pr-orchestrator.ymlopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.mdtests/unit/workflows/test_pr_orchestrator_signing.pyopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.md
📚 Learning: 2026-04-13T10:38:22.837Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: Signed module or manifest changes require version-bump review and verify-modules-signature
Applied to files:
.github/workflows/pr-orchestrator.ymlopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Enforce module signatures and version bumps when signed module assets or manifests are affected
Applied to files:
.github/workflows/pr-orchestrator.yml
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Applied to files:
.github/workflows/pr-orchestrator.ymlopenspec/CHANGE_ORDER.md.github/workflows/docs-review.ymlopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-publishing/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdtests/unit/workflows/test_pr_orchestrator_signing.pydocs/bundles/code-review/run.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-docs-command-validation/spec.mddocs/modules/code-review.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Treat the clean-code compliance gate as mandatory: the review surface enforces `naming`, `kiss`, `yagni`, `dry`, and `solid` categories and blocks regressions
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mddocs/bundles/code-review/run.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mddocs/modules/code-review.md
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Applies to **/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h} : Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Run the required verification and quality gates for the touched scope before finalization
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Treat clean-code regressions (naming, kiss, yagni, dry, solid violations) as blocking until they are fixed or explicitly justified
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/tasks.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.md
🪛 LanguageTool
docs/bundles/codebase/overview.md
[grammar] ~77-~77: Ensure spelling is correct
Context: ...se --repo . ``` ## Deep dives - Code analyze contracts - [Code drift d...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/bundles/codebase/repro.md
[grammar] ~66-~66: Ensure spelling is correct
Context: ...tall-crosshair ``` ## Related - Code analyze contracts - [Code drift d...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md
[grammar] ~19-~19: Use a hyphen to join words.
Context: ...ew page ## Requirement: Bundle overview related links SHALL be covered by docs v...
(QB_NEW_EN_HYPHEN)
openspec/changes/docs-15-code-review-validation-guardrails/proposal.md
[style] ~33-~33: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...on helper if consolidation is needed. - Affected CI and local gates include `.github/wor...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~33-~33: The official name of this software platform is spelled with a capital “H”.
Context: .... - Affected CI and local gates include .github/workflows/docs-review.yml, `.github/wo...
(GITHUB)
[uncategorized] ~33-~33: The official name of this software platform is spelled with a capital “H”.
Context: ...de .github/workflows/docs-review.yml, .github/workflows/docs-pages.yml, `.github/wor...
(GITHUB)
[uncategorized] ~33-~33: The official name of this software platform is spelled with a capital “H”.
Context: ...l, .github/workflows/docs-pages.yml, .github/workflows/pr-orchestrator.yml, .pre-c...
(GITHUB)
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
[uncategorized] ~4-~4: The official name of this software platform is spelled with a capital “H”.
Context: ...e work is required, refresh and consult .specfact/backlog/github_hierarchy_cache.md and verify parent, ...
(GITHUB)
[uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”.
Context: ...visible hook or stage. - [x] 6.3 Update .github/workflows/docs-review.yml so it runs t...
(GITHUB)
[uncategorized] ~45-~45: The official name of this software platform is spelled with a capital “H”.
Context: ...ogs for each category. - [x] 6.4 Update .github/workflows/docs-pages.yml so dependency...
(GITHUB)
openspec/changes/docs-15-code-review-validation-guardrails/design.md
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...inistic and usable before publishing. - Do not change bundle runtime behavior or r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...directly from signed module payloads. - Do not replace the existing Jekyll stack o...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~55-~55: Consider an alternative for the overused word “exactly”.
Context: ...ly on review. Rejected because this was exactly the failure mode. ## Risks / Trade-off...
(EXACTLY_PRECISELY)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tly incomplete redirect/guide pages. 3. Add failing tests for Code Review run optio...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~82-~82: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ndency maintenance change updates it? - Should generated _site link validation be ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/docs-15-code-review-validation-guardrails/proposal.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
openspec/changes/docs-15-code-review-validation-guardrails/design.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔀 Multi-repo context nold-ai/specfact-cli
[::nold-ai/specfact-cli::] .github/workflows/docs-review.yml — workflow now runs hatch env and executes hatch run check-docs-commands; file present under .github/workflows/docs-review.yml (see top-level job steps). Relevant for CI change verification.
[::nold-ai/specfact-cli::] .github/workflows/pr-orchestrator.yml — verify-module-signatures step sets BEFORE from github.event.before (fallback HEAD~1) and passes --version-check-base "$BEFORE"; also contains Python 3.12 setup and verifier dependency install steps. Relevant to pinned specfact-cli / signature verification behavior.
[::nold-ai/specfact-cli::] scripts/check-docs-commands.py — present and updated: new CLI entrypoints, uses specfact_cli.cli.app, and contains functions for extracting and validating specfact command examples (collect_specfact_commands_from_text, _tokens_from_specfact_line, etc.). Changes in PR rely on an additional helper module (docs_site_validation) but that file is MISSING in the checkout.
[::nold-ai/specfact-cli::] scripts/pre-commit-quality-checks.sh — present and updated: defines staged_docs detection and code-review gating functions (staged_review_gate_files, check_safe_change) and references running hatch run python scripts/check-docs-commands.py; contains new run_docs_site_validation_gate logic strings (search hits show the script contains gate functions).
[::nold-ai/specfact-cli::] tests/unit/docs/test_code_review_docs_parity.py — referenced by workflows and specs; rg found related test files and CI expects this parity test to exist. tests/unit/docs/test_docs_validation_scripts.py references scripts/check-docs-commands.py (line shown). Several test files reference pr-orchestrator.yml.
[::nold-ai/specfact-cli::] requirements-docs-ci.txt — referenced by workflows and specs but MISSING in this checkout.
[::nold-ai/specfact-cli::] scripts/docs_site_validation.py — referenced by check-docs-commands changes and new tests/specs (docs validation helpers), but MISSING in this checkout.
Notes on impact:
- CI/workflow changes expect requirements-docs-ci.txt and scripts/docs_site_validation.py to exist; their absence would break the new CI steps or local pre-commit docs gate.
- scripts/check-docs-commands.py and pre-commit script call into docs-site validation facilities; verify that docs_site_validation.py and requirements-docs-ci.txt are present in the PR branch before merging to avoid CI/pre-commit failures.
- pr-orchestrator.yml change to pass --version-check-base and pinned specfact-cli may affect downstream workflows that depend on previous implicit behavior (ensure consumers of pr-orchestrator behavior are compatible).
🔇 Additional comments (44)
docs/guides/brownfield-engineer.md (1)
3-3: Front matter title update looks correct.Line 3 cleanly labels this as a redirect page and keeps metadata consistent with the existing permalink/redirect fields.
As per coding guidelines, this satisfies “User-facing and cross-site accuracy: Jekyll front matter...”.
docs/guides/ide-integration.md (1)
3-3: Front matter redirect title is correctly aligned.
title: IDE integration (redirect)is a good addition for redirect-stub metadata consistency and docs parity; no issues here.As per coding guidelines, this satisfies the
docs/**/*.mdrequirement for accurate Jekyll front matter.docs/guides/brownfield-journey.md (1)
3-3: Front-matter parity fix looks good.Adding
titleto this redirect page aligns with the docs-site validation contract and keeps published metadata consistent.requirements-docs-ci.txt (1)
1-11: Pinned docs-CI dependency set is coherent.Good move centralizing and pinning this manifest for workflow parity and deterministic installs.
tests/unit/test_git_branch_module_signature_flag_script.py (1)
31-75: Isolation hardening is correct and high-value.The scrubbed git environment and nested-repo safety assertions make this test far less flaky and prevent outer-worktree side effects.
openspec/changes/docs-15-code-review-validation-guardrails/.openspec.yaml (1)
1-2: OpenSpec metadata bootstrap looks correct.The manifest is minimal and consistent with a new spec-driven change entry.
docs/bundles/spec/mock.md (1)
53-54: Related-link path correction is good.Switching to
../...targets improves route resolution consistency for published docs.docs/bundles/code-review/ledger.md (1)
48-49: Route-relative Related links are correctly fixed.This keeps ledger docs navigation consistent with the bundle URL contract.
docs/guides/brownfield-faq.md (1)
3-3: Front-matter completion is correct.Adding the redirect page title aligns this guide with the tightened docs metadata checks.
docs/bundles/spec/generate-tests.md (1)
50-51: Link normalization is correct and publish-route safe.
These parent-relative targets align with permalink-based bundle navigation and docs parity expectations.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/bundles/code-review/rules.md (1)
45-46: Good fix for Code Review bundle route parity.
../run/and../ledger/correctly preserve published-route navigation for the code-review docs surface.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/bundles/govern/overview.md (1)
48-49: Navigation targets are correctly normalized.
These sibling-route links match published permalink semantics for the Govern bundle.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/guides/openspec-journey.md (1)
479-479: Root-relative getting-started link is the right contract target.
This removes file-path coupling and keeps docs navigation consistent with published routes.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/reference/README.md (1)
72-73: These top-level doc links now match permalink routing.
Good parity update for cross-site navigation from the reference index.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/bundles/codebase/repro.md (1)
66-68: Related links are correctly aligned to sibling bundle routes.
This is consistent with permalink-based docs validation and improves route reliability.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/bundles/code-review/overview.md (1)
63-65: See-also links now correctly mirror the Code Review command surface routes.
This is a solid docs-parity fix for the bundle page graph.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/bundles/codebase/overview.md (1)
77-80: Deep-dive link normalization looks correct.
Sibling-route targeting is now consistent and should satisfy docs route validation.As per coding guidelines "User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract, CLI examples matching bundled commands."
docs/bundles/codebase/analyze.md (1)
43-45: LGTM!Parent-relative paths (
../drift/,../repro/,../overview/) correctly resolve from the/bundles/codebase/analyze/permalink to sibling pages within the codebase bundle. This aligns with the published-route-safe URL normalization pattern across the docs.docs/guides/brownfield-roi.md (1)
1-8: LGTM!Adding
title: Brownfield ROI (redirect)ensures this redirect page passes front-matter validation. The "(redirect)" suffix clearly communicates the page's purpose in navigation contexts.docs/bundles/backlog/overview.md (2)
69-69: LGTM!Parent-relative path correctly resolves to the policy-engine sibling page.
114-117: LGTM!Deep dives links consistently use parent-relative paths (
../refinement/,../dependency-analysis/,../delta/,../policy-engine/), correctly resolving from/bundles/backlog/overview/to sibling pages within the backlog bundle.docs/bundles/project/overview.md (2)
79-79: LGTM!Cross-bundle reference to
../codebase/overview/and sibling link to../import-migration/correctly follow the parent-relative path pattern for published-route resolution.
96-97: LGTM!See also links consistently use parent-relative paths for sibling pages within the project bundle.
.github/workflows/docs-pages.yml (2)
8-8: LGTM!Adding
requirements-docs-ci.txtto the paths trigger ensures the workflow runs when docs CI dependencies are updated.
56-65: Well-structured validation gate before Pages deployment.The validation step correctly:
- Uses pinned Python 3.12 matching other workflows
- Installs dependencies from
requirements-docs-ci.txtwith pinned versions (including specfact-cli==0.46.2 for contract alignment)- Runs validation with
--jekyll-bundle-checkafter Jekyll build but before artifact uploadDependencies and validation scripts are properly present. This ensures
bundle checkfailures and docs command validation errors block deployment, maintaining docs parity with the modules registry.docs/getting-started/README.md (2)
1-7: LGTM!Adding Jekyll front matter to README.md enables proper route indexing and published-page treatment. The permalink
/getting-started/and nav_order establish this as a primary navigation entry point.
90-90: Integration journey links properly configured.The separation into distinct "OpenSpec integration journey" and "Spec-Kit integration journey" links maintains documentation consistency. Both referenced guides include proper Jekyll front matter (layout, title, permalink) per the documentation URL contract.
.github/workflows/pr-orchestrator.yml (2)
97-104: Correct fix for version-bump false positives on merge commits.Using
github.event.beforeas the version-check base for push/workflow_dispatch events correctly handles three-way merge commits where HEAD~1 may not be the appropriate comparison point. The zero-SHA fallback (0000000000000000000000000000000000000000) handles the branch-creation edge case gracefully.This aligns with the
_resolve_version_check_base()logic inscripts/verify-modules-signature.py(context snippet 1) which accepts an explicit base ref.
143-143: Pinned specfact-cli version ensures reproducible CI.Version
0.46.2in pr-orchestrator.yml matches the pinned version inrequirements-docs-ci.txt, maintaining consistency across CI workflows for docs validation. The version is current in specfact-cli.openspec/CHANGE_ORDER.md (1)
81-81: Verified: CHANGE_ORDER entry correctly documents the specification change.The
docs-15-code-review-validation-guardrailschange folder exists with complete OpenSpec structure (proposal.md, design.md, TDD_EVIDENCE.md, tasks.md, specs/, and .openspec.yaml metadata). Entry follows established table format and references appropriate parent dependencies. Adapter boundary between change documentation and module packaging is properly maintained..github/workflows/docs-review.yml (3)
12-17: LGTM — path triggers are symmetrical and complete.The workflow now triggers on changes to the new docs validation infrastructure (
requirements-docs-ci.txt,scripts/docs_site_validation.py,test_code_review_docs_parity.py). Path lists are consistent betweenpull_requestandpushtriggers.Also applies to: 25-30
54-57: Good hardening: pinned deps via manifest.Switching from inline
pip installto-r requirements-docs-ci.txtensures reproducible CI runs and aligns with the pinnedspecfact-cli==0.46.2documented in the manifest.
59-64: No action required — all required files are present in the PR branch.Verification confirms that
requirements-docs-ci.txt,scripts/docs_site_validation.py, andtests/unit/docs/test_code_review_docs_parity.pyall exist in the specfact-cli-modules repository. The workflow will proceed without CI breakage from missing file dependencies.tests/unit/test_pre_commit_quality_parity.py (1)
38-40: LGTM — parity assertions match the new docs validation gate.The new required fragments align precisely with the
run_docs_site_validation_gate,needs_docs_site_validation, and the hatch invocation added toscripts/pre-commit-quality-checks.sh. This ensures the pre-commit script maintains docs validation gating.docs/guides/README.md (2)
1-6: LGTM — Jekyll front matter is complete and correct.The front matter includes all required fields (
layout,title,permalink,nav_order). The permalink/guides/aligns with the directory structure and follows the documentation-url-contract.
39-47: Docs validation infrastructure confirms link resolution across module boundaries.The parent-relative links in docs/guides/README.md (lines 39–47) are properly set up for published-permalink resolution. All target files exist (devops-adapter-overview.md, refinement.md, custom-registries.md, publishing-modules.md, module-signing.md, modes.md), and the jekyll-relative-links plugin + docs_site_validation.py are configured to resolve Markdown links relative to each page's published permalink—not source paths. The validation suite (check-docs-commands.py with docs_site_validation.py) enforces this contract; nav data links in _data/nav.yml are validated against actual page permalinks, ensuring no stale cross-module routes slip past CI review.
openspec/changes/docs-15-code-review-validation-guardrails/TDD_EVIDENCE.md (1)
1-19: LGTM — TDD evidence log is complete and follows the required order.The evidence documents the full validation flow: docs validation scripts, pytest suites, OpenSpec validation, format/lint, and the mandatory SpecFact code review gate. This aligns with the TDD order requirement (
spec -> tests -> failing evidence -> code -> passing evidence). Based on learnings: "Performspec -> tests -> failing evidence -> code -> passing evidencein that order for behavior changes."tests/unit/workflows/test_pr_orchestrator_signing.py (1)
30-39: LGTM — workflow parity assertions cover version-check-base and pinned CLI.The tests ensure
pr-orchestrator.ymlcorrectly wiresBEFOREfromgithub.event.beforewith the all-zero commit hash fallback, and thatspecfact-cli==0.46.2is pinned. The pinned version matchesrequirements-docs-ci.txt, maintaining cross-file consistency.Note: These exact-string assertions are brittle to whitespace/formatting changes in the workflow. Consider whether regex or YAML parsing would be more resilient for future maintenance.
scripts/pre-commit-quality-checks.sh (2)
76-109: LGTM — docs validation gate is well-structured.The gate logic correctly:
- Filters staged paths matching docs validation triggers (aligned with CI workflow paths).
- Returns early if no docs paths are staged (
needs_docs_site_validation).- Runs
hatch run python scripts/check-docs-commands.pyand fails pre-commit on validation errors.- Provides actionable remediation hint on failure.
The
exit 1on Line 107 ensures pre-commit fails before any "safe bypass" logic executes.
258-260: Gate placement correctly satisfies spec requirements.Running
run_docs_site_validation_gatebeforecheck_safe_changein bothrun_block2andrun_allensures:
- Docs-only commits with broken links fail before being marked "safe".
- Docs validation passes before the safe-change bypass is applied.
This matches the OpenSpec requirement: "pre-commit fails before reporting the change as safe."
Also applies to: 277-279
openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md (1)
1-27: LGTM — spec scenarios clearly define published-route link validation requirements.The WHEN/THEN/AND scenarios properly capture:
- Links must resolve from published permalink routes, not source paths.
- Cross-bundle links must resolve to canonical published routes.
- Source-valid but published-broken links must be rejected by validation tests.
This aligns with the docs validation implementation in
scripts/docs_site_validation.pyandscripts/check-docs-commands.py.openspec/changes/docs-15-code-review-validation-guardrails/specs/modules-pre-commit-quality-parity/spec.md (2)
3-17: LGTM — spec accurately reflects pre-commit gate behavior.The scenarios correctly specify:
- Docs-only commits with broken links fail pre-commit before the safe-change bypass.
- Valid docs commits skip code review and contract-test stages after validation passes.
This matches the implementation where
run_docs_site_validation_gateis called beforecheck_safe_changein bothrun_block2andrun_all.
19-27:published-linkfinding category is correctly implemented and consistently reported by both gates.The category exists at line 432 in
scripts/docs_site_validation.pyand is invoked byscan_published_route_body_links()when broken internal links are detected. Both the local pre-commit gate (scripts/pre-commit-quality-checks.sh) and the CI docs-review workflow executescripts/check-docs-commands.py, which imports and uses the samedocs_site_validationmodule. Findings are formatted consistently with the category name via_format_findings(), ensuring matching defect categories are reported locally and in CI. The spec accurately reflects this implementation.tests/unit/test_check_docs_commands_script.py (1)
143-163: No action required;requirements-docs-ci.txtis present and properly wired.The file exists in the repository and is correctly referenced by both
docs-review.ymlanddocs-pages.ymlworkflows, as well as the test assertions at lines 145 and 159. The linked repository finding about a missingrequirements-docs-ci.txtrefers to thenold-ai/specfact-clirepository, not this one. The test contract is valid.> Likely an incorrect or invalid review comment.
docs-15 follow-up: review fixes after merge to dev
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/docs-15-code-review-validation-guardrails/tasks.md`:
- Around line 44-48: The paired repo nold-ai/specfact-cli dev branch is missing
required artifacts—create and commit the following files with functional
implementations referenced by this spec: add .github/workflows/docs-review.yml
(runs the shared docs validation command and docs unit tests with clear logs),
.github/workflows/docs-pages.yml (runs dependency install, Jekyll build, and
blocks artifact upload on validation failures), scripts/docs_site_validation.py
(the deterministic docs validation script invoked by
scripts/pre-commit-quality-checks.sh), requirements-docs-ci.txt (CI dependency
lock for docs validation), and tests/unit/docs/test_code_review_docs_parity.py
(unit test asserting docs review and pre-commit parity). Ensure
scripts/pre-commit-quality-checks.sh and .pre-commit-config.yaml reference the
new docs validation hook/command so docs-only staged changes run deterministic
docs validation before skipping code-specific review and contract-test stages.
In `@scripts/docs_site_validation.py`:
- Around line 168-197: Consolidate the two passes into a single function (e.g.,
build_route_mappings) that iterates once over iter_docs_markdown_paths, uses
split_yaml_front_matter and published_route_for_doc to compute the canonical
route and list_redirect_from_routes for each file, and populates and returns
both the route_to_path dict (used by build_route_index) and the
path_to_canonical dict (used by build_path_to_canonical_route); then replace
callers such as scan_published_route_body_links to call the new single-pass
function and remove the duplicated
build_route_index/build_path_to_canonical_route double-read logic.
In `@tests/unit/docs/test_code_review_docs_parity.py`:
- Around line 94-97: The test is asserting on private helper names
(_resolve_review_run_flags, _validate_review_request,
_raise_if_targeting_styles_conflict) which couples docs parity tests to internal
implementation; replace those literal name checks with behavior-facing
assertions: remove the symbol-name asserts and instead verify user-visible
content such as headings, explanatory phrases or example usage that reflect the
behavior (e.g., "review flags" or "flags for running a review", "validate review
request" or "request validation", "targeting styles conflict" or "conflict
between targeting styles", and keep the existing check for "progress"); update
the test in tests/unit/docs/test_code_review_docs_parity.py to assert on those
user-facing strings or regexes so docs tests remain stable across internal
refactors while still confirming the same documented behaviors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 952372e7-5105-4f21-81f4-88127a415072
📒 Files selected for processing (9)
docs/bundles/code-review/run.mddocs/modules/code-review.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mdscripts/docs_site_validation.pytests/unit/docs/test_code_review_docs_parity.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: quality (3.13)
- GitHub Check: quality (3.12)
- GitHub Check: quality (3.11)
🧰 Additional context used
📓 Path-based instructions (5)
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
tests/unit/docs/test_code_review_docs_parity.pyscripts/docs_site_validation.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/unit/docs/test_code_review_docs_parity.py
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/bundles/code-review/run.mddocs/modules/code-review.md
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/docs_site_validation.py
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
📚 Learning: 2026-04-13T10:38:22.837Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-13T10:38:22.837Z
Learning: This repository enforces the clean-code review gate through hatch run specfact code review run --json --out .specfact/code-review.json
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mddocs/bundles/code-review/run.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mddocs/modules/code-review.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Fix SpecFact code review findings, including warnings, unless a rare explicit exception is documented
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mddocs/bundles/code-review/run.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mddocs/modules/code-review.mdscripts/docs_site_validation.py
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: When a change is paired with work in specfact-cli, review the paired public change artifacts there before widening scope or redefining shared workflow semantics
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mddocs/bundles/code-review/run.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.mddocs/modules/code-review.md
📚 Learning: 2026-04-13T10:38:15.842Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: .cursorrules:0-0
Timestamp: 2026-04-13T10:38:15.842Z
Learning: Adhere to worktree policy, OpenSpec gating, GitHub hierarchy-cache refresh, TDD order, quality gates, versioning, and documentation rules as defined in `docs/agent-rules/`
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdtests/unit/docs/test_code_review_docs_parity.pyopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Treat the clean-code compliance gate as mandatory: the review surface enforces `naming`, `kiss`, `yagni`, `dry`, and `solid` categories and blocks regressions
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Treat canonical rule docs (docs/agent-rules/INDEX.md) as the source of truth for worktree policy, OpenSpec gating, GitHub completeness checks, TDD order, quality gates, versioning, and documentation rules
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.mdopenspec/changes/docs-15-code-review-validation-guardrails/design.mdopenspec/changes/docs-15-code-review-validation-guardrails/proposal.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Treat clean-code regressions (naming, kiss, yagni, dry, solid violations) as blocking until they are fixed or explicitly justified
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/design.mdopenspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Run the required verification and quality gates for the touched scope before finalization
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Perform `spec -> tests -> failing evidence -> code -> passing evidence` in that order for behavior changes
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:29.379Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-13T10:38:29.379Z
Learning: Applies to **/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h} : Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
📚 Learning: 2026-04-13T10:38:43.524Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-13T10:38:43.524Z
Learning: Require public GitHub metadata completeness before implementation when linked issue workflow applies: parent, labels, project assignment, blockers, and blocked-by relationships
Applied to files:
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
🪛 LanguageTool
openspec/changes/docs-15-code-review-validation-guardrails/design.md
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...inistic and usable before publishing. - Do not change bundle runtime behavior or r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...directly from signed module payloads. - Do not replace the existing Jekyll stack o...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~57-~57: Consider an alternative for the overused word “exactly”.
Context: ...ly on review. Rejected because this was exactly the failure mode. ## Risks / Trade-off...
(EXACTLY_PRECISELY)
[style] ~71-~71: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tly incomplete redirect/guide pages. 3. Add failing tests for Code Review run optio...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~84-~84: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ndency maintenance change updates it? - Should generated _site link validation be ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/docs-15-code-review-validation-guardrails/proposal.md
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...on helper if consolidation is needed. - Affected CI and local gates include `.github/wor...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: .... - Affected CI and local gates include .github/workflows/docs-review.yml, `.github/wo...
(GITHUB)
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ...de .github/workflows/docs-review.yml, .github/workflows/docs-pages.yml, `.github/wor...
(GITHUB)
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ...l, .github/workflows/docs-pages.yml, .github/workflows/pr-orchestrator.yml, .pre-c...
(GITHUB)
openspec/changes/docs-15-code-review-validation-guardrails/tasks.md
[uncategorized] ~6-~6: The official name of this software platform is spelled with a capital “H”.
Context: ...e work is required, refresh and consult .specfact/backlog/github_hierarchy_cache.md and verify parent, ...
(GITHUB)
[uncategorized] ~46-~46: The official name of this software platform is spelled with a capital “H”.
Context: ...visible hook or stage. - [x] 6.3 Update .github/workflows/docs-review.yml so it runs t...
(GITHUB)
[uncategorized] ~47-~47: The official name of this software platform is spelled with a capital “H”.
Context: ...ogs for each category. - [x] 6.4 Update .github/workflows/docs-pages.yml so dependency...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli
Linked repositories findings
nold-ai/specfact-cli
-
scripts/docs_site_validation.py is referenced by multiple changed modules and tests but is missing from the checkout: searches and file listing show no scripts/docs_site_validation.py file. This is a direct risk because scripts/check-docs-commands.py and tests expect to import/use it. [::nold-ai/specfact-cli::]
-
requirements-docs-ci.txt is referenced by workflows and the new CI gating but is missing from the repo root in this checkout. Workflows (.github/workflows/docs-pages.yml and docs-review.yml) and CI steps expect this pinned requirements file to exist. [::nold-ai/specfact-cli::]
-
scripts/check-docs-commands.py exists and in this tree has been updated to import from scripts/docs_site_validation (it sets scripts/ on sys.path and later relies on docs_site_validation types/exports). Given docs_site_validation.py is missing, running this script (CI, pre-commit, or tests) will fail with ImportError. File preview: scripts/check-docs-commands.py uses _REPO_ROOT / "scripts" and imports specfact_cli.cli.app. [::nold-ai/specfact-cli::]
-
.github/workflows/docs-review.yml exists and the docs-review job runs "hatch run check-docs-commands" (and other docs validators). The workflow does not show installation from requirements-docs-ci.txt in this checkout (docs-pages.yml preview was missing), but the PR description expects workflows to install requirements-docs-ci.txt — ensure the workflow and requirements file are present and consistent. If workflows reference requirements-docs-ci.txt but the file is absent, CI will fail. [::nold-ai/specfact-cli::]
-
scripts/pre-commit-quality-checks.sh exists and references running the docs validation gate (it contains staged_docs logic and mentions running hatch run python scripts/check-docs-commands.py). Because check-docs-commands.py imports docs_site_validation, the pre-commit gate will fail locally if docs_site_validation.py is missing. Tests and pre-commit expectations updated in PR require these scripts to be present. [::nold-ai/specfact-cli::]
-
tests/unit/docs/test_code_review_docs_parity.py is expected by workflows/tests (and by PR changes) but is missing in this checkout. The docs-review workflow and new tests added in the PR rely on this test being present. [::nold-ai/specfact-cli::]
Summary / immediate risks:
- Missing scripts/docs_site_validation.py and requirements-docs-ci.txt (and missing new test file) in this checkout will cause CI/workflow steps, pre-commit gates, and tests that were added/updated by the PR to fail (ImportError or missing-file failures). Before merging, confirm those files are present in the PR branch or update workflows/scripts to match the actual committed files. [::nold-ai/specfact-cli::]
🔇 Additional comments (15)
openspec/changes/docs-15-code-review-validation-guardrails/specs/bundle-overview-pages/spec.md (1)
3-27: Published-route requirements are clearly specified and testable.This adds concrete, verifiable scenarios for permalink-based link resolution and pre-publish failure behavior.
docs/bundles/code-review/run.md (1)
42-55: Invalid-combination and targeting guidance looks aligned with command behavior.Good clarity on pre-run validation boundaries and user-facing alternatives for positional vs auto-scope targeting.
docs/modules/code-review.md (1)
63-74: Parity section now covers the key invalid flag combinations.The added combinations and phrasing close prior drift between module notes and run guide behavior.
openspec/changes/docs-15-code-review-validation-guardrails/specs/review-run-command/spec.md (1)
15-18: Targeting contract wording is now appropriately neutral.Using “positional files (file paths)” avoids over-constraining the public CLI contract.
openspec/changes/docs-15-code-review-validation-guardrails/proposal.md (1)
31-38: Impact and risk statements are explicit and actionable.The proposal clearly scopes affected docs, validation surfaces, and release-risk boundaries.
openspec/changes/docs-15-code-review-validation-guardrails/design.md (1)
29-66: Validation architecture is well-scoped and operationally pragmatic.The category model plus staged local/CI gates is a solid design for diagnosable docs quality enforcement.
scripts/docs_site_validation.py (9)
508-525: HandleFileNotFoundErrorwhenbundlecommand is unavailable.The docstring states "no silent skip," but currently if
bundleisn't on PATH,subprocess.runraisesFileNotFoundErrorwhich propagates as an unhandled crash. For deterministic CI tooling, returning aValidationFindingis preferable to both silent success and uncaught exceptions—it provides a clear, actionable message.🛡️ Proposed fix
try: proc = subprocess.run( ["bundle", "check"], cwd=str(docs_dir), capture_output=True, text=True, timeout=120, check=False, ) + except FileNotFoundError: + return [ + ValidationFinding( + "docs-build-dependency", + gemfile_lock, + 1, + "`bundle` is not available; install Ruby/Bundler before running --jekyll-bundle-check", + ) + ] except subprocess.TimeoutExpired:As per coding guidelines,
scripts/**/*.py: Deterministic tooling: subprocess and path safety.
1-33: Well-structured module header and constants.Imports are cleanly organized, and the regex patterns are appropriately bounded to avoid ReDoS concerns. The
_REQUIRED_PAGE_KEYStuple establishes a clear contract for publishable pages that aligns with the Jekyll front matter expectations onmodules.specfact.io.
35-68: Clear data structures and path utilities.
ValidationFindingprovides a consistent contract for all validation outputs across the module. TheDocsLinkResolutionContextNamedTuple keeps helper arity low while maintaining immutability—good for deterministic tooling. Theis_docs_markdownfunction correctly handles edge cases where paths might be outsidedocs_root.
71-88: Proper README front matter detection for route indexing.The
is_publishable_pagelogic correctly treats READMEs as publishable only when they use Jekyll front matter—this maintains parity with themodules.specfact.iopublishing behavior. Usingsorted()initer_docs_markdown_pathsensures deterministic iteration order across platforms.
91-126: Robust route normalization and front matter parsing.
normalize_routeproperly handles URL encoding viaunquote()and normalizes path separators. Thesplit_yaml_front_matterfunction usesyaml.safe_load(notyaml.load) and gracefully handles malformed YAML by returning an empty dict rather than raising—appropriate for validation tooling that needs to continue processing other files.
240-262: Directory link resolution now handles both README.md and index.md.The addition of the
index.mdfallback at lines 251-253 correctly addresses the prior concern about directory links. Links like../guide/will now resolve properly whether the directory publishes fromREADME.mdorindex.md. This maintains parity with Jekyll's actual resolution behavior onmodules.specfact.io.
340-405: Hybrid link resolution correctly handles both authoring modes.The
_uses_repo_relative_authoringheuristic appropriately identifies filesystem-style links (containing/,.., or.md) and tries filesystem resolution first. This accommodates both repo-authored links and published-URL-style links in the same document. The<!-- TODO:skip is a sensible allowance for work-in-progress documentation.
408-482: Comprehensive validation entry points for links and front matter.
scan_published_route_body_linkscorrectly builds both route indexes before iterating, ensuring link resolution has full context. The line number calculation at line 432 properly accounts for front matter offset. The front matter validation clearly reports which required keys are missing, which aids debugging parity issues withmodules.specfact.io.
536-546: Standard linter-style output format.The
format_findingsoutput format (path:line: [category] message) aligns with common tooling conventions, making it easy to integrate with editors and CI output parsing. The_namehelper's fallback to absolute paths when files are outsiderepo_rootprevents crashes on edge cases.
Add build_route_mappings() so route index and path-to-route maps share one docs tree pass; wire scan_published_route_body_links, build_valid_internal_routes, and nav tests to use it. Clarify OpenSpec section 6: CI/pre-commit/docs validation artifacts live in specfact-cli-modules, not the paired specfact-cli repo by default. Reword invalid-combination docs without private helper names; align parity tests with user-facing phrases (review flags, request validation, targeting conflicts). Made-with: Cursor
…wup-2 docs-15 follow-up: single-pass route mapping, tasks scope, parity wording
1 participant
This PR promotes the current
devline tomainafter merging #203 (docs-15 code review validation guardrails).Highlights
check-docs-commands, site validation) aligned with publishable routes and Jekyll bundle checks.--path, targeting).docs_site_validationrobustness (README front matter parity,bundle checktimeout handling).Verification
devincludes the merge commite5d86ff.Risk / rollout
Follow-up promotion PR:
dev→main.Made with Cursor