Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
117 changes: 117 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
#
# CodeRabbit aligns with AGENTS.md: bundle contracts, adapter boundaries to specfact_cli, Hatch gates.
# `reviews.auto_review.base_branches` includes `dev` so PRs into `dev` are auto-reviewed (not only the
# repo default branch). See https://docs.coderabbit.ai/reference/configuration (auto_review).
# Pre-push / finalize: run `cr --base dev` (or `coderabbit review`) from repo root; see
# https://docs.coderabbit.ai/cli/overview
# PR description: include `@coderabbitai summary` (default placeholder) for the high-level summary.
# Linked analysis: pair with nold-ai/specfact-cli (install CodeRabbit app on both repos).
#
language: "en-US"
early_access: false
tone_instructions: >-
Prioritize adapter boundaries between bundled modules and specfact_cli core: registry,
module-package.yaml, signing, and docs parity with modules.specfact.io. Flag cross-repo impact when
core APIs or contracts change.

reviews:
profile: assertive
request_changes_workflow: false
high_level_summary: true
high_level_summary_in_walkthrough: true
review_details: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: true
related_prs: true
poem: false
collapse_walkthrough: true
changed_files_summary: true
review_status: true
commit_status: true
high_level_summary_instructions: |
Structure the summary for specfact-cli-modules maintainers:
- Bundle and module surface: commands, adapters, bridge/runtime behavior vs. specfact_cli APIs.
- Manifest and integrity: module-package.yaml, semver, signature verification, registry impacts.
- Cross-repo: required specfact-cli changes, import/contract alignment, dev-deps path assumptions.
- Docs: modules.specfact.io / GitHub Pages accuracy, documentation-url-contract, CHANGELOG.
- If applicable: OpenSpec change ID and scenario coverage for module-specific behavior.
auto_review:
enabled: true
drafts: false
auto_incremental_review: true
base_branches:
- "^dev$"
path_instructions:
- path: "packages/**/src/**/*.py"
instructions: |
Focus on adapter and bridge patterns: imports from specfact_cli (models, runtime, validators),
Typer/Rich command surfaces, and clear boundaries so core upgrades do not silently break bundles.
Flag breaking assumptions about registry loading, lazy imports, and environment/mode behavior.
- path: "packages/**/module-package.yaml"
instructions: |
Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
- path: "registry/**"
instructions: |
Registry and index consistency: bundle listings, version pins, and compatibility with
published module artifacts.
- path: "src/**/*.py"
instructions: |
Repo infrastructure (not bundle code): keep parity with specfact-cli quality patterns;
contract-first public helpers where applicable; avoid print() in library paths.
- path: "openspec/**/*.md"
instructions: |
Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
- path: "tests/**/*.py"
instructions: |
Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
- path: ".github/workflows/**"
instructions: |
CI: secrets, hatch/verify-modules-signature gates, contract-test alignment, action versions.
- path: "scripts/**/*.py"
instructions: |
Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
- path: "tools/**/*.py"
instructions: |
Developer tooling aligned with pyproject Hatch scripts and CI expectations.
- path: "docs/**/*.md"
instructions: |
User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.

tools:
ruff:
enabled: true
semgrep:
enabled: true
yamllint:
enabled: true
actionlint:
enabled: true
shellcheck:
enabled: true

pre_merge_checks:
title:
mode: warning
requirements: "Prefer Conventional Commits-style prefixes (feat:, fix:, docs:, test:, refactor:, chore:)."
issue_assessment:
mode: warning

knowledge_base:
learnings:
scope: local
linked_repositories:
- repository: "nold-ai/specfact-cli"
instructions: >-
Core CLI and shared runtime: Typer app, module registry/bootstrap, specfact_cli public APIs,
contract-test and bundled-module signing flows. When modules change adapters or contracts,
flag required core changes, import paths, and coordinated version or signature updates.

chat:
auto_reply: true
6 changes: 6 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,9 @@ repos:
entry: ./scripts/pre-commit-quality-checks.sh
language: system
pass_filenames: false
- id: specfact-code-review-gate
name: Run code review gate on staged Python files
entry: hatch run python scripts/pre_commit_code_review.py
language: system
files: \.pyi?$
verbose: true
35 changes: 35 additions & 0 deletions .vibe/skills/specfact-code-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
name: specfact-code-review
description: House rules for AI coding sessions derived from review findings
allowed-tools: []
---

# House Rules - AI Coding Context (v4)

Updated: 2026-03-30 | Module: nold-ai/specfact-code-review

## DO

- Ask whether tests should be included before repo-wide review; default to excluding tests unless test changes are the target
- Use intention-revealing names; avoid placeholder public names like data/process/handle
- Keep functions under 120 LOC, shallow nesting, and <= 5 parameters (KISS)
- Delete unused private helpers and speculative abstractions quickly (YAGNI)
- Extract repeated function shapes once the second copy appears (DRY)
- Split persistence and transport concerns instead of mixing `repository.*` with `http_client.*` (SOLID)
- Add @require/@ensure (icontract) + @beartype to all new public APIs
- Run hatch run contract-test-contracts before any commit
- Write the test file BEFORE the feature file (TDD-first)
- Return typed values from all public methods and guard chained attribute access

## DON'T

- Don't enable known noisy findings unless you explicitly want strict/full review output
- Don't use bare except: or except Exception: pass
- Don't add # noqa / # type: ignore without inline justification
- Don't mix read + write in the same method or call `repository.*` and `http_client.*` together
- Don't import at module level if it triggers network calls
- Don't hardcode secrets; use env vars via pydantic.BaseSettings
- Don't create functions that exceed the KISS thresholds without a documented reason

## TOP VIOLATIONS (auto-updated by specfact code review rules update)
<!-- auto-managed: do not edit manually -->
33 changes: 32 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ hatch run verify-modules-signature --require-signature --payload-from-filesystem
hatch run contract-test
hatch run smart-test
hatch run test

# SpecFact code review JSON (dogfood; see "SpecFact Code Review JSON" below and openspec/config.yaml)
hatch run specfact code review run --json --out .specfact/code-review.json
```

CI orchestration runs in `.github/workflows/pr-orchestrator.yml` and enforces:
Expand All @@ -42,7 +45,35 @@ pre-commit install
pre-commit run --all-files
```

Staged `*.py` files trigger `hatch run lint` (includes pylint) via `scripts/pre-commit-quality-checks.sh`, matching `.github/workflows/pr-orchestrator.yml`.
Hooks run in order: **module signature verification** → **`scripts/pre-commit-quality-checks.sh`** (includes `hatch run lint` / pylint for staged Python) → **`scripts/pre_commit_code_review.py`** (SpecFact code review gate writing `.specfact/code-review.json`). That last hook is fast feedback on staged `*.py` / `*.pyi` files; it does not replace the **PR / change-completion** review rules in the next section when OpenSpec tasks require a full-scope run.

## SpecFact Code Review JSON (Dogfood, Quality Gate)

This matches **`openspec/config.yaml`** (project `context` and **`rules.tasks`** for code review): treat **`.specfact/code-review.json`** as mandatory evidence before an OpenSpec change is considered complete and before you rely on “all gates green” for a PR. Requires a working **specfact-cli** install (`hatch run dev-deps`).

**When to (re)run the review**

- The file is **missing**, or
- It is **stale**: the report’s last-modified time is older than any file you changed for this work under `packages/`, `registry/`, `scripts/`, `tools/`, `tests/`, or under `openspec/changes/<change-id>/` **except** `openspec/changes/<change-id>/TDD_EVIDENCE.md` — evidence-only edits there do **not** by themselves invalidate the review; re-run when proposal, specs, tasks, design, or code change.

**Command**

```bash
hatch run specfact code review run --json --out .specfact/code-review.json
```

- While iterating on a branch, prefer a **changed-files scope** when available (e.g. `--scope changed`) so feedback stays fast.
- Before the **final PR** for a change, run a **full** (or equivalent) scope so the report covers the whole quality surface your tasks expect (e.g. `--scope full`).

**Remediation**

- Read the JSON report and fix **every** finding at any severity (warning, advisory, error, or equivalent in the schema) unless the change proposal documents a **rare, explicit, justified** exception.
- After substantive edits, re-run until the report shows a **passing** outcome from the review module (e.g. overall verdict PASS / CI exit 0 per schema).
- Record the review command(s) and timestamp in `openspec/changes/<change-id>/TDD_EVIDENCE.md` or in the PR description when the change touches behavior or quality gates.

**Consistency**

- OpenSpec change **`tasks.md`** should include explicit tasks for generating/updating this file and clearing findings (see `openspec/config.yaml` → `rules.tasks` → “SpecFact code review JSON”). Agent runs should treat those tasks and this section as the same bar.

## Development workflow

Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ and this project follows SemVer for bundle versions.
### Added

- Documentation: authoritative `docs/reference/documentation-url-contract.md` for core vs modules URL ownership; `redirect_from` aliases for legacy `/guides/<basename>/` on pages whose canonical path is outside `/guides/`; sidebar link to the contract page.
- Add expanded clean-code review coverage to `specfact-code-review`, including
naming, KISS, YAGNI, DRY, SOLID, and PR-checklist findings plus the bundled
`specfact/clean-code-principles` policy-pack payload.

### Changed

- Refresh the canonical `specfact-code-review` house-rules skill to a compact
clean-code charter and bump the bundle metadata for the signed 0.45.1 release.

## [0.44.0] - 2026-03-17

Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ pre-commit install
pre-commit run --all-files
```

**Code review gate (matches specfact-cli core):** runs **after** module signature verification and `pre-commit-quality-checks.sh`. Staged `*.py` / `*.pyi` files run `specfact code review run --json --out .specfact/code-review.json` via `scripts/pre_commit_code_review.py`. The helper prints only a short findings summary and copy-paste prompts on stderr (not the nested CLI’s full tool output); enable `verbose: true` on the hook in `.pre-commit-config.yaml`. Requires a local **specfact-cli** install (`hatch run dev-deps` resolves sibling `../specfact-cli` or `SPECFACT_CLI_REPO`).

Scope notes:
- Pre-commit runs `hatch run lint` when any staged file is `*.py`, matching the CI quality job (Ruff alone does not run pylint).
- `ruff` runs on the full repo.
Expand Down
5 changes: 4 additions & 1 deletion docs/bundles/code-review/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ Use it together with the [Codebase](../codebase/overview/) bundle (`import`, `an

## Bundle-owned skills and policy packs

House rules and review payloads ship **inside the bundle** (for example Semgrep packs and skill metadata). They are **not** core CLI-owned resources. Install or refresh IDE-side assets with `specfact init ide` after upgrading the bundle.
House rules and review payloads ship **inside the bundle** (for example Semgrep
packs, the `specfact/clean-code-principles` policy-pack manifest, and skill
metadata). They are **not** core CLI-owned resources. Install or refresh
IDE-side assets with `specfact init ide` after upgrading the bundle.

## Quick examples

Expand Down
42 changes: 36 additions & 6 deletions docs/modules/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ Custom rule mapping:

| Semgrep rule | ReviewFinding category |
| --- | --- |
| `banned-generic-public-names` | `naming` |
| `swallowed-exception-pattern` | `clean_code` |
| `get-modify-same-method` | `clean_code` |
| `unguarded-nested-access` | `clean_code` |
| `cross-layer-call` | `architecture` |
Expand All @@ -234,6 +236,8 @@ Custom rule mapping:
Representative anti-patterns covered by the ruleset:

- methods that both read state and mutate it
- public symbols that use banned generic names like `data` or `process`
- swallowed exceptions that hide an underlying failure path
- direct nested attribute access like `obj.config.value`
- repository and HTTP client calls in the same function
- module-level network client instantiation
Expand All @@ -243,7 +247,7 @@ Additional behavior:

- only the provided file list is considered
- semgrep rule IDs emitted with path prefixes are normalized back to the governed rule IDs above
- malformed output or a missing Semgrep executable yields a single `tool_error` finding
- malformed output, a missing `results` list, or a missing Semgrep executable yields a single `tool_error` finding

### Contract runner

Expand Down Expand Up @@ -323,7 +327,15 @@ Then rerun the ledger command from the same repository checkout.
## House rules skill

The `specfact-code-review` bundle can derive a compact house-rules skill from the
reward ledger and keep it small enough for AI session context injection.
reward ledger and keep it small enough for AI session context injection. The
default charter now encodes the clean-code principles directly:

- Naming: use intention-revealing names instead of placeholders.
- KISS: keep functions small, shallow, and narrow in parameters.
- YAGNI: remove unused private helpers and speculative layers.
- DRY: extract repeated function shapes once duplication appears.
- SOLID: keep transport and persistence responsibilities separate.
- TDD + contracts: keep test-first and icontract discipline in the baseline skill.

### Command flow

Expand Down Expand Up @@ -362,13 +374,31 @@ bundle runners in this order:

1. Ruff
2. Radon
3. basedpyright
4. pylint
5. contract runner
6. TDD gate, unless `no_tests=True`
3. Semgrep
4. AST clean-code checks
5. basedpyright
6. pylint
7. contract runner
8. TDD gate, unless `no_tests=True`

When `SPECFACT_CODE_REVIEW_PR_MODE=1` is present, the runner also evaluates a
bundle-local advisory PR checklist from `SPECFACT_CODE_REVIEW_PR_TITLE`,
`SPECFACT_CODE_REVIEW_PR_BODY`, and `SPECFACT_CODE_REVIEW_PR_PROPOSAL` without
adding a new CLI flag.

The merged findings are then scored into a governed `ReviewReport`.

## Bundled policy pack

The bundle now ships `specfact/clean-code-principles` as a resource payload at:

- `packages/specfact-code-review/resources/policy-packs/specfact/clean-code-principles.yaml`
- `packages/specfact-code-review/src/specfact_code_review/resources/policy-packs/specfact/clean-code-principles.yaml`

The manifest exposes the clean-code rule IDs directly so downstream policy code
can apply advisory, mixed, or hard modes without a second review-specific
severity schema.

### TDD gate

`specfact_code_review.run.runner.run_tdd_gate(files)` enforces a bundle-local
Expand Down
1 change: 1 addition & 0 deletions openspec/CHANGE_ORDER.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Cross-repo dependency: `docs-06-modules-site-ia-restructure` is a prerequisite f
| docs | 11 | docs-11-team-enterprise-tier | [#99](https://github.com/nold-ai/specfact-cli-modules/issues/99) | docs-06-modules-site-ia-restructure |
| docs | 12 | docs-12-docs-validation-ci | [#100](https://github.com/nold-ai/specfact-cli-modules/issues/100) | docs-06 through docs-10; specfact-cli/docs-12-docs-validation-ci |
| docs | 13 | docs-13-nav-search-theme-roles | [#123](https://github.com/nold-ai/specfact-cli-modules/issues/123) | docs-06 through docs-12 (fixes navigation gaps left by prior changes; adds search, theme toggle, and role-based navigation) |
| docs | 14 | docs-14-module-release-history | [#124](https://github.com/nold-ai/specfact-cli-modules/issues/124) | docs-13-nav-search-theme-roles; publish-modules workflow (adds publish-driven module release history, AI-assisted backfill for already-published versions, and docs rendering of shipped features/improvements) |

### Spec-Kit v0.4.x change proposal bridge (spec-kit integration review, 2026-03-27)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# TDD Evidence

## 2026-03-31

- `2026-03-31T00:56:00+02:00` Bootstrap:
`hatch run dev-deps`
linked the local `specfact-cli` checkout into this worktree so the bundle tests and review runner could execute against the live code.
- `2026-03-31T01:02:00+02:00` Red phase:
`SPECFACT_ALLOW_UNSIGNED=1 hatch run pytest tests/unit/specfact_code_review/run/test_runner.py tests/unit/specfact_code_review/tools/test_ast_clean_code_runner.py tests/unit/specfact_code_review/tools/test_radon_runner.py tests/unit/specfact_code_review/tools/test_semgrep_runner.py -q`
failed with 5 targeted regressions that matched the new spec change:
- `test_run_review_requires_explicit_pr_mode_token_for_clean_code_reasoning`
expected `clean-code.pr-checklist-missing-rationale` but `_checklist_findings()` returned `[]`.
- `test_run_ast_clean_code_reports_mixed_dependency_roles_for_injected_dependencies`
expected `solid.mixed-dependency-role` for `self.repo.save()` / `self.client.get()` but the leftmost dependency was still treated as `self`.
- `test_run_ast_clean_code_continues_after_parse_error`
expected a per-file `tool_error` plus later-file findings, but the parser branch returned early.
- `test_run_radon_uses_dedicated_tool_identifier_for_kiss_findings`
expected `tool="radon-kiss"` but the emitted finding still used `tool="radon"`.
- `test_run_semgrep_returns_tool_error_when_results_key_is_missing`
expected a `tool_error` for malformed Semgrep JSON, but the runner treated a missing `results` key as a clean run.
- `2026-03-31T01:04:30+02:00` Implementation:
updated `packages/specfact-code-review/src/specfact_code_review/run/runner.py`,
`packages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.py`,
`packages/specfact-code-review/src/specfact_code_review/tools/radon_runner.py`,
`packages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.py`,
`packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md`,
`packages/specfact-code-review/src/specfact_code_review/resources/policy-packs/specfact/clean-code-principles.yaml`,
`docs/modules/code-review.md`,
and the targeted unit tests so the new clean-code checks, strict PR-mode gating, dependency-root detection, KISS tool labeling, and Semgrep parsing behavior matched the review comments.
- `2026-03-31T01:06:30+02:00` Green phase:
`SPECFACT_ALLOW_UNSIGNED=1 hatch run pytest tests/unit/specfact_code_review/run/test_runner.py tests/unit/specfact_code_review/tools/test_ast_clean_code_runner.py tests/unit/specfact_code_review/tools/test_radon_runner.py tests/unit/specfact_code_review/tools/test_semgrep_runner.py -q`
passed with `50 passed in 20.26s`.
- `2026-03-31T01:08:11+02:00` Release validation:
`hatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump`
passed after the module was signed again.
- `2026-03-31T01:10:42+02:00` Review validation:
`SPECFACT_ALLOW_UNSIGNED=1 hatch run specfact code review run --json --out .specfact/code-review.json`
passed with `findings: []`.
Loading
Loading