diff --git a/docs/bundles/code-review/run.md b/docs/bundles/code-review/run.md index 15f0d71..e1b3cad 100644 --- a/docs/bundles/code-review/run.md +++ b/docs/bundles/code-review/run.md @@ -43,7 +43,7 @@ The command prints **progress** to the terminal (spinner/status while the pipeli The command validates several incompatible flag mixes before the review pipeline runs. -The Typer entrypoint calls **`_resolve_review_run_flags()`** first; that helper raises **`typer.BadParameter`** for **`--include-tests`** together with **`--exclude-tests`**, and for **`--focus`** together with **`--include-tests`** or **`--exclude-tests`**. Other pairings are rejected later by command-layer validators such as **`_validate_review_request()`** and **`_raise_if_targeting_styles_conflict()`** (wired from **`run_command()`**), including **`--json`** with **`--score-only`**, **`--out`** without **`--json`**, and positional **`FILES...`** with **`--scope`** or **`--path`**. Those command-layer checks are still surfaced as **`typer.BadParameter`** with the messages below. +The Typer entrypoint validates **review flags** first: it raises **`typer.BadParameter`** when **`--include-tests`** is combined with **`--exclude-tests`**, or when **`--focus`** is combined with **`--include-tests`** or **`--exclude-tests`**. **Request validation** then rejects incompatible output modes (**`--json`** with **`--score-only`**, or **`--out`** without **`--json`**), and rules for **conflicting targeting styles** reject mixing positional **`FILES...`** with **`--scope`** or **`--path`**. Those deeper checks still surface as **`typer.BadParameter`** with the messages below. - **Positional `FILES...` with `--scope` or `--path`**: when you pass explicit paths, do not also pass **`--scope`** or **`--path`** (those options apply only to auto-discovery). Runtime error: **`Choose positional files or auto-scope controls, not both.`** - **`--focus` with `--include-tests` or `--exclude-tests`**: use **`--focus`** *or* the include/exclude test flags, not both. Runtime error: **`Cannot combine --focus with --include-tests or --exclude-tests`** diff --git a/docs/modules/code-review.md b/docs/modules/code-review.md index dc76225..ce498ab 100644 --- a/docs/modules/code-review.md +++ b/docs/modules/code-review.md @@ -62,7 +62,7 @@ Options (aligned with `specfact code review run --help`): ### Invalid combinations -The command rejects incompatible mixes (same rules as the bundle run guide): Typer raises **`BadParameter`** from **`_resolve_review_run_flags()`** for **`--include-tests`** with **`--exclude-tests`**, and for **`--focus`** with **`--include-tests`** or **`--exclude-tests`**; other pairings are enforced in **`run_command()`** via **`_validate_review_request()`**, **`_raise_if_targeting_styles_conflict()`**, and related helpers. +The command rejects incompatible mixes (same rules as the bundle run guide): Typer raises **`BadParameter`** while validating **review flags** (**`--include-tests`** with **`--exclude-tests`**, or **`--focus`** with **`--include-tests`** / **`--exclude-tests`**). **Request validation** and **conflicting targeting styles** rules in the run pipeline enforce the remaining pairings (output modes and positional files vs **`--scope`** / **`--path`**), still surfaced as **`typer.BadParameter`**. - **Positional `FILES...` with `--scope` or `--path`**: choose explicit files **or** auto-scope controls, not both. diff --git a/openspec/changes/docs-15-code-review-validation-guardrails/tasks.md b/openspec/changes/docs-15-code-review-validation-guardrails/tasks.md index 838a21a..0042f58 100644 --- a/openspec/changes/docs-15-code-review-validation-guardrails/tasks.md +++ b/openspec/changes/docs-15-code-review-validation-guardrails/tasks.md @@ -41,6 +41,8 @@ ## 6. Local And CI Gate Integration +**Implementation repository:** Deliverables in this section (for example `.github/workflows/docs-review.yml`, `.github/workflows/docs-pages.yml`, `scripts/docs_site_validation.py`, `requirements-docs-ci.txt`, `scripts/pre-commit-quality-checks.sh`, `.pre-commit-config.yaml`, and `tests/unit/docs/test_code_review_docs_parity.py`) are implemented and maintained in **nold-ai/specfact-cli-modules**. They are **not** expected to appear on the paired **nold-ai/specfact-cli** `dev` branch unless a separate, explicitly tracked cross-repo coordination change says otherwise. + - [x] 6.1 Update `scripts/pre-commit-quality-checks.sh` so docs-only staged changes run deterministic docs validation before skipping code-specific review and contract-test stages. - [x] 6.2 Update `.pre-commit-config.yaml` if needed to expose docs validation as a separate visible hook or stage. - [x] 6.3 Update `.github/workflows/docs-review.yml` so it runs the shared docs validation command and docs unit tests with clear logs for each category. diff --git a/scripts/docs_site_validation.py b/scripts/docs_site_validation.py index 50a0b69..a11c385 100644 --- a/scripts/docs_site_validation.py +++ b/scripts/docs_site_validation.py @@ -167,34 +167,42 @@ def list_redirect_from_routes(front_matter: dict[str, Any]) -> list[str]: @beartype @require(lambda docs_root: isinstance(docs_root, Path)) -@ensure(lambda result: isinstance(result, dict)) -def build_route_index(docs_root: Path) -> dict[str, Path]: - """Map every published and redirect alias route to its declaring markdown file.""" +@ensure( + lambda result: ( + isinstance(result, tuple) and len(result) == 2 and isinstance(result[0], dict) and isinstance(result[1], dict) + ) +) +def build_route_mappings(docs_root: Path) -> tuple[dict[str, Path], dict[Path, str]]: + """Map routes to declaring files and resolved files to canonical routes in one tree pass.""" route_to_path: dict[str, Path] = {} + path_to_route: dict[Path, str] = {} for path in iter_docs_markdown_paths(docs_root): - text = path.read_text(encoding="utf-8") - fm, _, _ = split_yaml_front_matter(text) + fm, _, _ = split_yaml_front_matter(path.read_text(encoding="utf-8")) if not fm: continue canonical = published_route_for_doc(path, docs_root, fm) route_to_path[canonical] = path for alias in list_redirect_from_routes(fm): route_to_path[alias] = path - return route_to_path + path_to_route[path.resolve()] = canonical + return route_to_path, path_to_route + + +@beartype +@require(lambda docs_root: isinstance(docs_root, Path)) +@ensure(lambda result: isinstance(result, dict)) +def build_route_index(docs_root: Path) -> dict[str, Path]: + """Map every published and redirect alias route to its declaring markdown file.""" + return build_route_mappings(docs_root)[0] @beartype @require(lambda docs_root: isinstance(docs_root, Path)) @ensure(lambda result: isinstance(result, dict)) def build_path_to_canonical_route(docs_root: Path) -> dict[Path, str]: - mapping: dict[Path, str] = {} - for path in iter_docs_markdown_paths(docs_root): - fm, _, _ = split_yaml_front_matter(path.read_text(encoding="utf-8")) - if not fm: - continue - mapping[path.resolve()] = published_route_for_doc(path, docs_root, fm) - return mapping + """Map each markdown source path to its canonical published route.""" + return build_route_mappings(docs_root)[1] def _normalize_jekyll_relative_url(link: str) -> str: @@ -410,8 +418,7 @@ def extract_markdown_links_with_lines(body: str) -> list[tuple[int, str]]: @ensure(lambda result: isinstance(result, list)) def scan_published_route_body_links(docs_root: Path, repo_root: Path) -> list[ValidationFinding]: findings: list[ValidationFinding] = [] - route_to_path = build_route_index(docs_root) - path_to_route = build_path_to_canonical_route(docs_root) + route_to_path, path_to_route = build_route_mappings(docs_root) ctx = DocsLinkResolutionContext(docs_root, route_to_path, path_to_route, MODULES_DOCS_HOST) for path in iter_docs_markdown_paths(docs_root): @@ -487,7 +494,8 @@ def scan_frontmatter_findings(docs_root: Path, repo_root: Path) -> list[Validati @ensure(lambda result: isinstance(result, set)) def build_valid_internal_routes(docs_root: Path) -> set[str]: """All routes that should be treated as in-site targets (canonical + redirect aliases).""" - return set(build_route_index(docs_root).keys()) + route_to_path, _ = build_route_mappings(docs_root) + return set(route_to_path.keys()) @beartype diff --git a/tests/unit/docs/test_code_review_docs_parity.py b/tests/unit/docs/test_code_review_docs_parity.py index 44c3738..bdabab5 100644 --- a/tests/unit/docs/test_code_review_docs_parity.py +++ b/tests/unit/docs/test_code_review_docs_parity.py @@ -91,9 +91,9 @@ def test_code_review_run_doc_describes_invalid_flag_combinations() -> None: text = RUN_DOC.read_text(encoding="utf-8") assert "## Invalid combinations" in text - assert "_resolve_review_run_flags()" in text - assert "_validate_review_request()" in text - assert "_raise_if_targeting_styles_conflict()" in text + assert "review flags" in text.lower() + assert "request validation" in text.lower() + assert "conflicting targeting styles" in text.lower() assert "progress" in text assert "default **`enforce`**" in text assert "Optional reporting level override" in text diff --git a/tests/unit/docs/test_docs_review.py b/tests/unit/docs/test_docs_review.py index 42989e3..d2a964c 100644 --- a/tests/unit/docs/test_docs_review.py +++ b/tests/unit/docs/test_docs_review.py @@ -166,8 +166,7 @@ def _navigation_sources() -> list[Path]: def _scan_navigation_targets() -> tuple[list[str], set[Path]]: docs_root = _docs_root() repo_root = _repo_root() - route_to_path = dsv.build_route_index(docs_root) - path_to_route = dsv.build_path_to_canonical_route(docs_root) + route_to_path, path_to_route = dsv.build_route_mappings(docs_root) ctx = dsv.DocsLinkResolutionContext(docs_root, route_to_path, path_to_route, dsv.MODULES_DOCS_HOST) failures: list[str] = [] targets: set[Path] = set()