Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/bundles/code-review/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,17 @@ The command prints **progress** to the terminal (spinner/status while the pipeli

## Invalid combinations

Typer validates several incompatible flag mixes before execution:
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.

- **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`**
- **`--include-tests` with `--exclude-tests`**: pick at most one test inclusion mode. Runtime error: **`Cannot use both --include-tests and --exclude-tests`**
- **`--out` without `--json`**: **`--out`** is accepted only when **`--json`** is also set. Runtime error: **`Use --out together with --json.`**
- **`--json` with `--score-only`**: do not combine JSON report output with score-only mode (**`Use either --json or --score-only, not both.`**).

**Supported targeting:** either pass **positional Python file paths** for a fixed review set, or omit files and use **`--scope`** / **`--path`** (and related test flags) for auto-discovery — do not mix positional paths with **`--scope`** or **`--path`**.
**Supported targeting:** either pass **positional file paths** for a fixed review set (the pipeline still only reviews Python sources it accepts, such as **`.py`** / **`.pyi`**), or omit files and use **`--scope`** / **`--path`** (and related test flags) for auto-discovery — do not mix positional paths with **`--scope`** or **`--path`**.

## Examples

Expand Down
3 changes: 2 additions & 1 deletion docs/modules/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,15 @@ Options (aligned with `specfact code review run --help`):

### Invalid combinations

Typer rejects incompatible mixes (same rules as the bundle run guide):
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.

- **Positional `FILES...` with `--scope` or `--path`**: choose explicit files **or**
auto-scope controls, not both.
- **`--focus` with `--include-tests` or `--exclude-tests`**: use **`--focus`** *or*
the include/exclude flags, not both.
- **`--include-tests` with `--exclude-tests`**: pick at most one test inclusion mode.
- **`--out` without `--json`**: **`--out`** is accepted only when **`--json`** is also set.
- **`--json` with `--score-only`**: pick one, not both (**`--json`** cannot be used with **`--score-only`**).

When `FILES` is omitted, the command falls back to:

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Design: docs-15 — Code review validation guardrails

## Context

The modules docs site is a Jekyll site published at `modules.specfact.io`. Many authored pages set explicit `permalink` values such as `/bundles/code-review/overview/`, so browser URL resolution happens relative to the published permalink route rather than the source file path. Current validation has two separate surfaces: `tests/unit/docs/test_docs_review.py` performs several source-oriented link and front matter checks, while `scripts/check-docs-commands.py` validates command examples, selected cross-site routes, resource path drift, and navigation data. Both currently pass even when a published page has relative links that resolve to broken public URLs.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Proposal: docs-15 — Code review validation guardrails

## Why

Recent Code Review bundle changes added `specfact code review run` behavior and options that are not consistently documented on the bundle deep-dive pages, and published module overview pages contain links that pass source-file checks while breaking after Jekyll publishes them under permalink routes. This slipped through because the current docs and pre-commit gates validate selected source-path and command categories, but do not systematically validate generated-page URL semantics, required front matter completeness, docs build dependency health, or docs drift caused by command option changes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Bundle overview pages SHALL use links that resolve correctly from the published
- **THEN** the link resolves to the target bundle's canonical published overview route
- **AND** docs validation fails if the link resolves to a route nested under the source overview page

## Requirement: Bundle overview related links SHALL be covered by docs validation tests
## Requirement: Bundle overview-related links SHALL be covered by docs validation tests

The bundle overview docs test suite SHALL include coverage that fails when any overview page contains a body link that is valid by source-file path but broken under published permalink semantics.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ The Code Review run documentation SHALL describe every supported public `specfac

- **WHEN** the command rejects combinations such as positional files with `--scope` or `--path`, or `--focus` with `--include-tests`
- **THEN** the Code Review docs describe the invalid combination behavior
- **AND** the docs include a user-facing alternative for the supported targeting style: pass explicit **positional Python file paths** for a fixed review set, or use **`--scope`** / **`--path`** (without positional files) to auto-discover targets from the repo
- **AND** the docs include a user-facing alternative for the supported targeting style aligned with the public **`run`** signature (**`files: list[Path]`**): pass explicit **positional files** (file paths) for a fixed review set, or use **`--scope`** / **`--path`** (without positional files) to auto-discover targets from the repo

## Requirement: Code Review docs SHALL stay aligned with review behavior

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# docs-15: Tasks — code review validation guardrails

## 1. Governance And Readiness

- [x] 1.1 Confirm this change remains scoped to docs, docs validation, pre-commit docs routing, and Code Review docs parity; do not change bundle runtime behavior unless a failing test proves it is required.
Expand Down
7 changes: 4 additions & 3 deletions scripts/docs_site_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ def _resolve_candidate_markdown_target(
readme_candidate = (candidate / "README.md").resolve()
if readme_candidate.is_file() and is_docs_markdown(readme_candidate, docs_root):
return _path_lookup_result(readme_candidate, target_value, route_to_path, path_to_route)
index_candidate = (candidate / "index.md").resolve()
if index_candidate.is_file() and is_docs_markdown(index_candidate, docs_root):
return _path_lookup_result(index_candidate, target_value, route_to_path, path_to_route)
return None, None
if candidate.is_file() and is_docs_markdown(candidate, docs_root):
return _path_lookup_result(candidate, target_value, route_to_path, path_to_route)
Expand Down Expand Up @@ -491,7 +494,7 @@ def build_valid_internal_routes(docs_root: Path) -> set[str]:
@require(lambda docs_dir: isinstance(docs_dir, Path))
@ensure(lambda result: isinstance(result, list))
def scan_gemfile_lock_installability(docs_dir: Path) -> list[ValidationFinding]:
"""Run ``bundle check`` when Ruby/Bundler is available; otherwise skip."""
"""Run ``bundle check`` in ``docs_dir``; requires ``bundle`` on PATH (no silent skip)."""
gemfile_lock = docs_dir / "Gemfile.lock"
if not gemfile_lock.is_file():
return [
Expand All @@ -511,8 +514,6 @@ def scan_gemfile_lock_installability(docs_dir: Path) -> list[ValidationFinding]:
timeout=120,
check=False,
)
except FileNotFoundError:
return []
except subprocess.TimeoutExpired:
return [
ValidationFinding(
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/docs/test_code_review_docs_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ def test_code_review_run_doc_mentions_public_ty_options() -> None:
for flag in sorted(flags):
assert flag in text, f"docs/bundles/code-review/run.md must document {flag}"

assert "progress" in text
assert "spinner" in text or "status" in text
assert "default **`enforce`**" in text
assert "Optional reporting level override" in text
assert "--bug-hunt" in text
assert "exploratory" in text.lower()
assert "review-report.json" in text


def _resolver_messages_for_docs_parity() -> list[str]:
messages: list[str] = []
Expand Down Expand Up @@ -83,6 +91,14 @@ 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 "progress" in text
assert "default **`enforce`**" in text
assert "Optional reporting level override" in text
assert "review-report.json" in text

for msg in _resolver_messages_for_docs_parity():
assert msg in text, f"run.md must document Typer resolver text: {msg!r}"

Expand Down
Loading