Conversation
…eview-module [codex] expand code review clean-code checks
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds an AST-backed clean-code analysis suite (YAGNI/DRY/SOLID/KISS/naming), new Semgrep rules and a bundled policy-pack, integrates a pre-commit code-review gate that emits .specfact/code-review.json, extends runner orchestration and categories, updates package/registry metadata, and lands OpenSpec docs/specs for a publish-driven per-module release-history. Note: keep adapter boundaries between bundled modules and specfact_cli core (registry, module-package metadata, signing, and docs parity). Changes
Sequence Diagram(s)sequenceDiagram
actor Dev
participant PreCommit as Git Pre-Commit Hook
participant Script as scripts/pre_commit_code_review.py
participant SpecFactCLI as specfact_cli (code review)
participant Runner as review.runner
participant Tools as Tools (ruff, semgrep, radon, ast)
participant Report as .specfact/code-review.json
Dev->>PreCommit: commit staged *.py
PreCommit->>Script: run specfact-code-review-gate
Script->>Script: filter staged .py/.pyi
Script->>SpecFactCLI: python -m specfact_cli.cli code review run --json --out ...
SpecFactCLI->>Runner: run_review(files)
Runner->>Tools: run_semgrep(files)
Runner->>Tools: run_ruff(files)
Runner->>Tools: run_radon(files)
Runner->>Tools: run_ast_clean_code(files)
Tools-->>Runner: list[ReviewFinding]
Runner->>Report: write JSON
Script->>Report: read & summarize findings
Script->>Dev: print aggregated summary / exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
chore(registry): publish changed modules
djm81
added
enhancement
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
packages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.py
Fixed
Show fixed
Hide fixed
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md (1)
11-74:⚠️ Potential issue | 🟠 MajorRestore explicit red/green evidence in this TDD record.
This section currently documents passing validation only; it should include at least one failing (red) result and the subsequent passing (green) result tied to the same scope so the TDD trail is auditable.
As per coding guidelines
openspec/changes/*/TDD_EVIDENCE.md: “Record failing/passing test evidence in openspec/changes//TDD_EVIDENCE.md”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md` around lines 11 - 74, The TDD_EVIDENCE.md currently only shows passing (green) validation; add an explicit failing (red) result plus the subsequent passing (green) result for the same scope so the TDD trail is auditable. Edit openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md to insert a failing run (e.g., a prior run of python3 scripts/check-docs-commands.py or the Jekyll build that produced an error) with its error output clearly marked as red/failing, then append the corrected run showing the passing/green output and note what change fixed it (reference the same commands and the nav/_data.yml or Jekyll build step you validated). Ensure the two entries are adjacent and labeled (Failing -> Fix -> Passing) so reviewers can trace the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vibe/skills/specfact-code-review/SKILL.md:
- Around line 11-19: The markdown heading "## DO" (text: "## DO") is missing
required blank lines per markdownlint MD022; update the SKILL.md content to
insert a blank line immediately above and one immediately below the "## DO"
heading so there is separation from surrounding paragraphs/listing, and apply
the same blank-line rule to any other headings in the file to satisfy MD022.
- Around line 13-29: The ruleset contains a duplicate 120-LOC constraint: "Keep
functions under 120 LOC" and "Don't create functions > 120 lines"; remove one to
avoid redundancy—either delete the DON'T line with "Don't create functions > 120
lines" or merge both into a single DO entry "Keep functions under 120 LOC" so
only one canonical rule remains; update the SKILL.md section where those two
lines appear so the constraint is stated once and wording/placement is
consistent with other DO/DON'T items.
- Around line 7-9: The IDE skill file .vibe/skills/specfact-code-review/SKILL.md
is stale and diverges from the canonical bundled version; remove the committed
IDE copy or regenerate it from the canonical bundle by running the updater (use
the sync function in rules/updater.py named sync_skill_to_ide or run the CLI
command specfact review rules update) so the IDE file matches the canonical
packaged SKILL.md and no longer diverges during installs.
In `@CHANGELOG.md`:
- Around line 17-20: Update the CHANGELOG.md entry that currently reads "signed
0.45.0 release" to match the actual shipped version in module-package.yaml (the
`version: 0.45.1` value); locate the text "signed 0.45.0 release" in the
"Changed" section and replace 0.45.0 with 0.45.1 so the changelog and the
module-package.yaml `version` field are consistent.
In `@openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md`:
- Line 7: In TDD_EVIDENCE.md the Claude session entry contains machine-specific
absolute filesystem paths; replace those absolute paths (the lines containing
the Claude session entry `fff31fcf-cd55-4952-896b-638cb0e8958f`) with
repo-relative placeholders (e.g. ./, <REPO_ROOT>/, or a documented ${REPO_ROOT}
variable) so the evidence is portable and does not expose workstation details;
update all other occurrences in the same file (the additional evidence lines) to
use the same placeholder convention for consistency.
In
`@openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.md`:
- Around line 34-39: In the "Scenario: Release-note style guidance is available
to AI copilot" block replace the informal phrase "technical bla bla" with
precise, normative language such as "unnecessary technical jargon or generic
filler text" and make the requirement testable (e.g., "THEN they MUST avoid
unnecessary technical jargon or generic filler text"); update the sentence so it
reads clearly and unambiguously using that phrasing to allow consistent review
and automated checks.
In `@openspec/changes/docs-14-module-release-history/tasks.md`:
- Around line 1-5: The file's first line uses a second-level heading ("## 1.
Change Setup") which triggers markdownlint MD041; change that to a top-level
heading by making the first heading a single '#' (e.g., "# 1. Change Setup") so
the document begins with a level-1 heading to match other OpenSpec task files
and satisfy the linter.
In `@packages/specfact-code-review/.semgrep/clean_code.yaml`:
- Around line 57-61: The semgrep rule id "banned-generic-public-names" currently
flags any def/class named process/handle/manager/data including private names;
update the rule's pattern-regex so it excludes identifiers that start with one
or more underscores (i.e., only match public names). Concretely, modify the
pattern-regex used in the rule to add a negative lookahead (or equivalent) after
the def|class token so names like _process or __handle are not matched while
public names still are. Ensure the updated regex is applied to the same rule id
and retains the multiline (?m) flag and the same target keywords.
In `@packages/specfact-code-review/module-package.yaml`:
- Around line 25-26: Update packages/specfact-code-review/module-package.yaml to
replace the stale integrity.checksum and integrity.signature with values
generated by the project signing workflow: run the signing commands (hatch run
sign-modules --module specfact-code-review) to produce a new tarball signature
and checksum, then run hatch run verify-modules-signature --require-signature
--payload-from-filesystem --enforce-version-bump to verify and obtain the
correct checksum (should match
72372145e9633d55c559f1efe4c0eb284d5814398b5bad837810dd69654f1dbb) and signature,
and update the integrity.checksum and integrity.signature fields in
module-package.yaml accordingly while leaving core_compatibility and the version
bump as-is.
In
`@packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md`:
- Line 11: Add a blank line after the markdown headings "## DO" and "## DON'T"
in SKILL.md to improve rendering consistency; locate the occurrences of the
headings (the lines containing "## DO" and "## DON'T") and insert an empty line
immediately below each heading so there is a blank line before the following
content.
In `@packages/specfact-code-review/src/specfact_code_review/rules/updater.py`:
- Around line 132-134: The current regex looks through the whole content and can
pick up body lines; instead first extract the YAML front matter block from
content (search for the first /^---$/ ... /^---$/ block with DOTALL/MULTILINE)
and then run the description regex against that front_matter string only; update
the code around the existing re.search call (the match variable and description
assignment) to target the front matter segment so only metadata `description:`
is captured and body lines are ignored.
In `@packages/specfact-code-review/src/specfact_code_review/run/findings.py`:
- Around line 22-27: VALID_CATEGORIES in this module and the
ReviewFinding.category Literal were extended with five clean-code categories
(naming, kiss, yagni, dry, solid) but the review-finding-model spec in the other
repo is still out-of-date; update the review-finding-model specification
(spec.md) in nold-ai/specfact-cli to list the full, current set of 13 categories
to match VALID_CATEGORIES and ReviewFinding.category, ensuring the documented
enum values exactly mirror the tuple and Literal, and coordinate/pin this change
as a cross-repo dependency so publishing and tests remain spec-compliant.
In `@registry/index.json`:
- Around line 76-78: The registry entry for specfact-code-review is missing the
core_compatibility field; update the JSON object that contains "latest_version":
"0.45.1", "download_url": "modules/specfact-code-review-0.45.1.tar.gz", and
"checksum_sha256":
"72372145e9633d55c559f1efe4c0eb284d5814398b5bad837810dd69654f1dbb" to add
"core_compatibility": ">=0.40.0,<1.0.0" (matching
packages/specfact-code-review/module-package.yaml) so downstream consumers like
marketplace_client.py and specfact-cli see consistent compatibility metadata.
In `@scripts/check-docs-commands.py`:
- Around line 199-200: _validate_command_examples is re-reading files instead of
using the in-memory snapshot in text_by_path, breaking deterministic snapshots;
update it to use the provided text_by_path contents when extracting examples.
Specifically, stop calling _extract_command_examples with a file path that
triggers disk reads and either (a) add a new helper like
_extract_command_examples_from_text(text, path) and call that with
text_by_path[path], or (b) change _extract_command_examples to accept an
optional text argument and pass text_by_path[path]; ensure all references to
file I/O inside _extract_command_examples/_extract_command_examples_from_text
are bypassed so the validator uses the in-memory text and remains deterministic.
In `@skills/specfact-code-review/SKILL.md`:
- Around line 13-30: The markdown triggers MD022 and MD037: add a blank line
immediately after the "## DO" and "## DON'T" headings to satisfy MD022, and wrap
wildcard patterns like repository.* and http_client.* in backticks (e.g.,
`repository.*`, `http_client.*`) to prevent MD037 asterisk parsing; update the
rules block in SKILL.md (look for the "## DO" / "## DON'T" headings and the
lines containing repository.* and http_client.*) accordingly.
In `@tests/unit/scripts/test_pre_commit_code_review.py`:
- Around line 161-182: The test test_main_timeout_fails_hook uses the real
_repo_root which makes it path-sensitive; update the test to monkeypatch
module._repo_root (or the module-level variable used to compute repo root) to a
controlled Path (e.g., the same repo_root value computed earlier in the test)
before calling module.main so subprocess.run receives a stable cwd; keep the
existing assertions and ensure monkeypatch.setattr(module, "_repo_root",
repo_root) (or equivalent) is applied alongside the existing monkeypatches for
ensure_runtime_available and subprocess.run.
In `@tests/unit/specfact_code_review/rules/test_updater.py`:
- Around line 42-50: The helper _skill_text() used by
test_update_house_rules_preserves_do_and_dont_sections diverges from the bundled
SKILL.md by missing inline code backticks; update the string(s) within
_skill_text() to wrap repository.* and http_client.* in backticks (i.e.,
"`repository.*`" and "`http_client.*`") so the test helper matches the deployed
SKILL.md formatting.
In `@tests/unit/specfact_code_review/run/test_findings.py`:
- Around line 22-26: Add the five new categories ("naming", "kiss", "yagni",
"dry", "solid") consistently in the test by updating the TypedDict definition,
the pytest parametrization list used in test_findings (the list of category
strings), and the cast Literal/typing used to mirror production VALID_CATEGORIES
so all three locations match the VALID_CATEGORIES tuple; optionally note the
cross-repo docs change but do not rely on it for the tests since pre-commit
validation uses ConfigDict(extra="ignore").
In `@tests/unit/specfact_code_review/run/test_runner.py`:
- Around line 268-307: The tests are flaky because ambient
SPECFACT_CODE_REVIEW_PR_* env vars leak into _checklist_findings(); add a
hermetic setup that clears all checklist env keys before each test (or create an
autouse fixture such as clear_pr_env) and then set only the required vars in
each test. Specifically, clear/delete SPECFACT_CODE_REVIEW_PR_MODE,
SPECFACT_CODE_REVIEW_PR_TITLE, SPECFACT_CODE_REVIEW_PR_BODY,
SPECFACT_CODE_REVIEW_PR_PROPOSAL (use monkeypatch.delenv(key, raising=False) or
monkeypatch.setenv(key, "") as appropriate) at the start of
test_run_review_emits_advisory_checklist_finding_in_pr_mode and
test_run_review_requires_explicit_pr_mode_token_for_clean_code_reasoning (or in
the autouse fixture) so _checklist_findings() reads only the intended values.
In `@tests/unit/specfact_code_review/tools/test_radon_runner.py`:
- Around line 99-124: Extract the duplicated synthetic-file setup used in
test_run_radon_emits_kiss_metrics_from_source_shape and
test_run_radon_uses_dedicated_tool_identifier_for_kiss_findings into a shared
helper or pytest fixture (e.g., a function create_noisy_file(tmp_path,
line_count=81) or a fixture noisy_file(tmp_path) that returns the Path); update
both tests to call the helper/fixture instead of duplicating the file creation
logic and keep existing uses of monkeypatch.setattr(subprocess, "run", ...) and
run_radon([file_path]) intact so behavior and assertions (including filtering by
finding.rule and expecting finding.tool == "radon-kiss") remain unchanged.
In `@tests/unit/test_pre_commit_quality_parity.py`:
- Around line 29-31: The test currently only asserts presence of hook IDs
(hook_ids) which allows regressions in ordering; update the test in
test_pre_commit_quality_parity.py to assert relative ordering by computing
indexes for each expected hook and asserting they are strictly increasing. Use
the canonical sequence (e.g., ["format", "type-check", "lint", "yaml-lint",
"verify-module-signatures", "contract-test", "smart-test", "test"]) and for each
adjacent pair assert hook_ids.index(first) < hook_ids.index(second) (or loop to
compare indexes) so the order is enforced rather than just existence.
---
Outside diff comments:
In `@openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md`:
- Around line 11-74: The TDD_EVIDENCE.md currently only shows passing (green)
validation; add an explicit failing (red) result plus the subsequent passing
(green) result for the same scope so the TDD trail is auditable. Edit
openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md to insert a
failing run (e.g., a prior run of python3 scripts/check-docs-commands.py or the
Jekyll build that produced an error) with its error output clearly marked as
red/failing, then append the corrected run showing the passing/green output and
note what change fixed it (reference the same commands and the nav/_data.yml or
Jekyll build step you validated). Ensure the two entries are adjacent and
labeled (Failing -> Fix -> Passing) so reviewers can trace the fix.
🪄 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: aa437ed8-5763-488d-86b5-83e650a1f423
⛔ Files ignored due to path filters (1)
registry/modules/specfact-code-review-0.45.1.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (55)
.coderabbit.yaml.pre-commit-config.yaml.vibe/skills/specfact-code-review/SKILL.mdAGENTS.mdCHANGELOG.mdREADME.mddocs/bundles/code-review/overview.mddocs/modules/code-review.mdopenspec/CHANGE_ORDER.mdopenspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdopenspec/changes/clean-code-02-expanded-review-module/tasks.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-13-nav-search-theme-roles/proposal.mdopenspec/changes/docs-13-nav-search-theme-roles/specs/cross-module-workflow-docs/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/specs/docs-client-search/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/specs/docs-nav-data-driven/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/specs/modules-docs-command-validation/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/tasks.mdopenspec/changes/docs-14-module-release-history/CHANGE_VALIDATION.mdopenspec/changes/docs-14-module-release-history/design.mdopenspec/changes/docs-14-module-release-history/proposal.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-registry/spec.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/config.yamlpackages/specfact-code-review/.semgrep/clean_code.yamlpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/resources/policy-packs/specfact/clean-code-principles.yamlpackages/specfact-code-review/src/specfact_code_review/resources/policy-packs/specfact/clean-code-principles.yamlpackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.pypyproject.tomlregistry/index.jsonregistry/modules/specfact-code-review-0.45.1.tar.gz.sha256registry/signatures/specfact-code-review-0.45.1.tar.sigscripts/__init__.pyscripts/check-docs-commands.pyscripts/pre_commit_code_review.pyskills/specfact-code-review/SKILL.mdtests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/tools/test___init__.pytests/unit/specfact_code_review/tools/test_ast_clean_code_runner.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/unit/test_bundle_resource_payloads.pytests/unit/test_pre_commit_quality_parity.py
💤 Files with no reviewable changes (4)
- openspec/changes/docs-13-nav-search-theme-roles/specs/docs-client-search/spec.md
- openspec/changes/docs-13-nav-search-theme-roles/specs/cross-module-workflow-docs/spec.md
- openspec/changes/docs-13-nav-search-theme-roles/specs/modules-docs-command-validation/spec.md
- openspec/changes/docs-13-nav-search-theme-roles/specs/docs-nav-data-driven/spec.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (19)
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/overview.mddocs/modules/code-review.md
registry/**
⚙️ CodeRabbit configuration file
registry/**: Registry and index consistency: bundle listings, version pins, and compatibility with
published module artifacts.
Files:
registry/signatures/specfact-code-review-0.45.1.tar.sigregistry/modules/specfact-code-review-0.45.1.tar.gz.sha256registry/index.json
openspec/changes/**
📄 CodeRabbit inference engine (CLAUDE.md)
Never manually move folders under
openspec/changes/intoarchive/. Archiving MUST useopenspec archive <change-id>command
Files:
openspec/changes/clean-code-02-expanded-review-module/tasks.mdopenspec/changes/docs-14-module-release-history/CHANGE_VALIDATION.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-registry/spec.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdopenspec/changes/docs-13-nav-search-theme-roles/tasks.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/proposal.mdopenspec/changes/docs-14-module-release-history/design.mdopenspec/changes/docs-13-nav-search-theme-roles/proposal.md
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/clean-code-02-expanded-review-module/tasks.mdopenspec/CHANGE_ORDER.mdopenspec/changes/docs-14-module-release-history/CHANGE_VALIDATION.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-registry/spec.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdopenspec/changes/docs-13-nav-search-theme-roles/tasks.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/proposal.mdopenspec/changes/docs-14-module-release-history/design.mdopenspec/changes/docs-13-nav-search-theme-roles/proposal.md
openspec/CHANGE_ORDER.md
📄 CodeRabbit inference engine (AGENTS.md)
Update openspec/CHANGE_ORDER.md when archive status changes in OpenSpec changes
Files:
openspec/CHANGE_ORDER.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Line length must be 120 characters
Python target version is 3.11+
rufflinting runs on the full repository
Files:
tests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/__init__.pytests/unit/specfact_code_review/tools/test___init__.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/test_bundle_resource_payloads.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.pyscripts/check-docs-commands.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pytests/unit/specfact_code_review/rules/test_updater.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pytests/unit/specfact_code_review/tools/test_ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pytests/unit/scripts/test_pre_commit_code_review.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pyscripts/pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.py
{src,tests,tools}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
basedpyrightandpylintare scoped tosrc/,tests/, andtools/directoriesPython source code must pass linting with pylint via
hatch run linttargeting src/, tests/, and tools/ directories
Files:
tests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/unit/specfact_code_review/tools/test___init__.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/test_bundle_resource_payloads.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/specfact_code_review/tools/test_ast_clean_code_runner.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.py
tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Use tests/ directory for bundle behavior and migration parity tests
Files:
tests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/unit/specfact_code_review/tools/test___init__.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/test_bundle_resource_payloads.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/specfact_code_review/tools/test_ast_clean_code_runner.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.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/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/unit/specfact_code_review/tools/test___init__.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/test_bundle_resource_payloads.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/specfact_code_review/tools/test_ast_clean_code_runner.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.py
openspec/changes/*/TDD_EVIDENCE.md
📄 CodeRabbit inference engine (AGENTS.md)
Record failing/passing test evidence in openspec/changes//TDD_EVIDENCE.md
Files:
openspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md
packages/*/src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/src/**/*.py: Only allowedspecfact_cli.*prefixes may be imported in bundle code (CORE/SHARED APIs only)
Cross-bundle lateral imports are forbidden except specific allowed pairs (e.g.specfact_spec->specfact_project)
Files:
packages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.py
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep bundle package code under packages/ directory
Files:
packages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.pypackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/resources/policy-packs/specfact/clean-code-principles.yamlpackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/resources/policy-packs/specfact/clean-code-principles.yamlpackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.py
packages/**/src/**/*.py
⚙️ CodeRabbit configuration file
packages/**/src/**/*.py: 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.
Files:
packages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.py
**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
All YAML configuration files must pass yaml-lint validation via
hatch run yaml-lint
Files:
openspec/config.yamlpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/resources/policy-packs/specfact/clean-code-principles.yamlpackages/specfact-code-review/src/specfact_code_review/resources/policy-packs/specfact/clean-code-principles.yaml
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/check-docs-commands.pyscripts/pre_commit_code_review.py
packages/*/module-package.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use SemVer for bundle versioning: patch (bug fix), minor (new command/option/API), major (breaking change/removal)
packages/*/module-package.yaml: Modules must have signatures verified with version bump enforcement viaverify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump
Bundle version changes must be SemVer-compliant: patch for bug fixes, minor for new commands/options/APIs, major for breaking changes
Files:
packages/specfact-code-review/module-package.yaml
{packages/*/module-package.yaml,registry/index.json}
📄 CodeRabbit inference engine (CLAUDE.md)
When bumping a bundle version, review and update
core_compatibilityin bothmodule-package.yamlandregistry/index.jsonUpdate
core_compatibilityfield in bothpackages/<bundle>/module-package.yamlandregistry/index.jsonwhen bundle requires a newer minimum specfact-cli version
Files:
packages/specfact-code-review/module-package.yamlregistry/index.json
{registry/index.json,packages/*/module-package.yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Keep registry metadata in registry/index.json and packages/*/module-package.yaml
Files:
packages/specfact-code-review/module-package.yamlregistry/index.json
packages/**/module-package.yaml
⚙️ CodeRabbit configuration file
packages/**/module-package.yaml: Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
Files:
packages/specfact-code-review/module-package.yaml
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Before changing code, verify an active OpenSpec change explicitly covers the requested scope; follow strict TDD order: spec delta → failing tests → implementation → passing tests → quality gates
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : When bumping a bundle version, review and update `core_compatibility` in both `module-package.yaml` and `registry/index.json`
Applied to files:
docs/bundles/code-review/overview.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/src/**/*.py : Only allowed `specfact_cli.*` prefixes may be imported in bundle code (CORE/SHARED APIs only)
Applied to files:
docs/bundles/code-review/overview.mdtests/unit/test_bundle_resource_payloads.py.vibe/skills/specfact-code-review/SKILL.mdskills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mddocs/modules/code-review.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : Update `core_compatibility` field in both `packages/<bundle>/module-package.yaml` and `registry/index.json` when bundle requires a newer minimum specfact-cli version
Applied to files:
docs/bundles/code-review/overview.mdregistry/modules/specfact-code-review-0.45.1.tar.gz.sha256packages/specfact-code-review/module-package.yamlregistry/index.json
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Pre-commit hooks must mirror CI configuration: run `pre-commit install && pre-commit run --all-files`
Applied to files:
.pre-commit-config.yamlREADME.mdAGENTS.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Run pre-commit hooks locally via `pre-commit install` and `pre-commit run --all-files` to mirror CI quality gates
Applied to files:
.pre-commit-config.yamlREADME.mdAGENTS.mdscripts/pre_commit_code_review.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to **/*.{yaml,yml} : All YAML configuration files must pass yaml-lint validation via `hatch run yaml-lint`
Applied to files:
.pre-commit-config.yamlREADME.mdAGENTS.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Run quality gates in order: format, type-check, lint, yaml-lint, verify-modules-signature, contract-test, smart-test, test
Applied to files:
.pre-commit-config.yamlopenspec/changes/clean-code-02-expanded-review-module/tasks.mdREADME.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdAGENTS.mddocs/modules/code-review.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Run quality gates in the specified order: format → type-check → lint → yaml-lint → verify-modules-signature → contract-test → smart-test → test
Applied to files:
.pre-commit-config.yamlREADME.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdAGENTS.mddocs/modules/code-review.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {src,tests,tools}/**/*.py : Python source code must pass linting with pylint via `hatch run lint` targeting src/, tests/, and tools/ directories
Applied to files:
.pre-commit-config.yamlREADME.mdAGENTS.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {src,tests,tools}/**/*.{ts,tsx,js,jsx} : TypeScript/JavaScript code must pass type checking via `hatch run type-check`
Applied to files:
.pre-commit-config.yamlREADME.mdAGENTS.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to openspec/CHANGE_ORDER.md : Update openspec/CHANGE_ORDER.md when archive status changes in OpenSpec changes
Applied to files:
openspec/changes/clean-code-02-expanded-review-module/tasks.mdopenspec/CHANGE_ORDER.mdopenspec/changes/docs-14-module-release-history/CHANGE_VALIDATION.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/proposal.mdopenspec/changes/docs-14-module-release-history/design.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Before changing code, verify an active OpenSpec change explicitly covers the requested scope; follow strict TDD order: spec delta → failing tests → implementation → passing tests → quality gates
Applied to files:
openspec/changes/clean-code-02-expanded-review-module/tasks.mdopenspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdREADME.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/config.yamlopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md.vibe/skills/specfact-code-review/SKILL.mdskills/specfact-code-review/SKILL.mdAGENTS.mdpackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mddocs/modules/code-review.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Follow strict TDD order: spec delta -> failing tests -> implementation -> passing tests -> quality gates. Record TDD evidence in `openspec/changes/<change-id>/TDD_EVIDENCE.md`
Applied to files:
openspec/changes/clean-code-02-expanded-review-module/tasks.mdopenspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdAGENTS.mddocs/modules/code-review.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to openspec/changes/*/TDD_EVIDENCE.md : Record failing/passing test evidence in openspec/changes/<change-id>/TDD_EVIDENCE.md
Applied to files:
openspec/changes/clean-code-02-expanded-review-module/tasks.mdopenspec/changes/docs-14-module-release-history/CHANGE_VALIDATION.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdAGENTS.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to **/*.py : `ruff` linting runs on the full repository
Applied to files:
README.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to tests/** : Use tests/ directory for bundle behavior and migration parity tests
Applied to files:
tests/unit/test_bundle_resource_payloads.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to packages/*/module-package.yaml : Modules must have signatures verified with version bump enforcement via `verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump`
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/module-package.yaml : Use SemVer for bundle versioning: patch (bug fix), minor (new command/option/API), major (breaking change/removal)
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to packages/*/module-package.yaml : Bundle version changes must be SemVer-compliant: patch for bug fixes, minor for new commands/options/APIs, major for breaking changes
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Run `hatch run check-bundle-imports` to enforce bundle import policies
Applied to files:
AGENTS.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to {src,tests,tools}/**/*.py : `basedpyright` and `pylint` are scoped to `src/`, `tests/`, and `tools/` directories
Applied to files:
tests/unit/specfact_code_review/run/test_runner.py
🪛 GitHub Actions: pr-orchestrator
packages/specfact-code-review/module-package.yaml
[error] 1-1: Module package verification failed: checksum mismatch.
🪛 LanguageTool
openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.md
[grammar] ~34-~34: Use a hyphen to join words.
Context: ...tory source #### Scenario: Release-note style guidance is available to AI copilo...
(QB_NEW_EN_HYPHEN)
openspec/changes/clean-code-02-expanded-review-module/TDD_EVIDENCE.md
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n_code_continues_after_parse_error expected a per-filetool_error` plus later-file...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~18-~18: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._tool_identifier_for_kiss_findings expectedtool="radon-kiss"` but the emitted fin...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._error_when_results_key_is_missing expected atool_error` for malformed Semgrep JS...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/docs-13-nav-search-theme-roles/tasks.md
[uncategorized] ~60-~60: The official name of this software platform is spelled with a capital “H”.
Context: ...le - [ ] 8.3 Extend the publish flow so .github/workflows/publish-modules.yml writes t...
(GITHUB)
openspec/changes/docs-14-module-release-history/tasks.md
[uncategorized] ~14-~14: The official name of this software platform is spelled with a capital “H”.
Context: ... Workflow Integration - [ ] 3.1 Extend .github/workflows/publish-modules.yml so each ...
(GITHUB)
openspec/changes/docs-14-module-release-history/proposal.md
[uncategorized] ~12-~12: The official name of this software platform is spelled with a capital “H”.
Context: ... install/search registry index - Extend .github/workflows/publish-modules.yml so each ...
(GITHUB)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/docs-14-module-release-history/tasks.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.vibe/skills/specfact-code-review/SKILL.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 26-26: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
skills/specfact-code-review/SKILL.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 27-27: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔀 Multi-repo context nold-ai/specfact-cli
Findings for nold-ai/specfact-cli [::nold-ai/specfact-cli::]
-
skills/house-rule references and install consumers
- openspec/specs/rules-commands/spec.md — GIVEN
skills/specfact-code-review/SKILL.mdexists (doc/spec expects the SKILL file) [::nold-ai/specfact-cli::] - multiple openspec archives/specs reference
skills/specfact-code-review/SKILL.mdas required (e.g., openspec/specs/house-rules-skill/spec.md, various archive entries). These are consumers of the new SKILL.md content. [::nold-ai/specfact-cli::]
- openspec/specs/rules-commands/spec.md — GIVEN
-
ReviewFinding / review model consumers
- scripts/pre_commit_code_review.py defines/uses ReviewFinding and reads/writes .specfact/code-review.json (pre-commit gate) — this script will consume reports produced by specfact-code-review and relies on the ReviewFinding payload shape. [::nold-ai/specfact-cli::]
- tests/unit/scripts/test_pre_commit_code_review.py — tests assert ReviewFinding usage/serialization and expected JSON report fields (severity buckets, overall_verdict). This test directly consumes the code-review JSON contract. [::nold-ai/specfact-cli::]
- openspec/specs/review-finding-model/spec.md — requires a ReviewFinding Pydantic model with validated category, severity, tool, rule, file, line, message, fixable. New categories added by the module (naming/kiss/yagni/dry/solid) affect this spec. [::nold-ai/specfact-cli::]
-
Module/package registry & publish consumers
- docs/modules/code-review.md and openspec/specs/code-review-module/spec.md reference the module
nold-ai/specfact-code-reviewand expect packages/specfact-code-review/module-package.yaml to exist. CI/publish tooling in this repo expects the registry/index.json in the separate modules repo (specfact-cli-modules) to be updated. [::nold-ai/specfact-cli::] - src/specfact_cli/registry/marketplace_client.py — constructs URLs to raw
nold-ai/specfact-cli-modules/{branch}/registry/index.json(consumer of registry/index.json layout). [::nold-ai/specfact-cli::] - scripts/verify-bundle-published.py and scripts/verify-bundle-published uses ../specfact-cli-modules/registry/index.json by default — tooling here depends on registry/index.json changes in the modules repo. [::nold-ai/specfact-cli::]
- docs/modules/code-review.md and openspec/specs/code-review-module/spec.md reference the module
-
Test and spec consumers of new runners/categories
- tests/unit/specfact_code_review/run/test_findings.py and other unit tests (under tests/unit/specfact_code_review/...) reference/expect the expanded category set and runners (ast/radon/semgrep) — the repo’s test-suite/specs are consumers of the new categories and runner behaviors. [::nold-ai/specfact-cli::]
- openspec/specs/radon-runner/spec.md, semgrep-runner/spec.md, ruff-runner/spec.md, etc., define expected mapping of tool outputs to ReviewFinding categories; adding new categories (naming, kiss, yagni, dry, solid) means those specs and any tooling that validates against them must accept the new values. [::nold-ai/specfact-cli::]
Implications (observed in repo files)
- The pre-commit hook script and its tests (scripts/pre_commit_code_review.py, tests/unit/scripts/test_pre_commit_code_review.py) directly consume the code-review JSON contract and will require the updated ReviewFinding payload to remain compatible (including new category literals and overall_verdict handling). [::nold-ai/specfact-cli::]
- Multiple specs and tests in openspec expect ReviewFinding model constraints; adding new Literal category values (naming/kiss/yagni/dry/solid) and new tool names (e.g., "ast", "radon-kiss", "semgrep") must be reflected in the Pydantic model/spec validation used across the repo/tests. [::nold-ai/specfact-cli::]
- Publishing and registry automation in this repo expects registry/index.json at
specfact-cli-modules; the PR’s registry/index.json and module-package.yaml changes will be consumed by marketplace_client and verify scripts that read that index. Ensure registry layout and signatures align with these consumers. [::nold-ai/specfact-cli::]
Conclusion
- I found direct consumers and specs in this repository that depend on the ReviewFinding shape, category set, runner/tool names, module package metadata, and registry/index.json layout. These are potential cross-boundary places to verify compatibility (pre-commit script/tests, openspec specs, marketplace/registry tooling). [::nold-ai/specfact-cli::]
openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md
Outdated
Show resolved
Hide resolved
…toring Changes include: - Fix B905: zip() without explicit strict parameter by using itertools.pairwise() - Fix RUF007: prefer itertools.pairwise() over zip() for successive pairs - Fix C1803: simplify empty list check from == [] to not findings - Add missing test file for specfact_code_review.__init__ module - Remove shadowing __init__.py that caused import issues - Add cleanup in tests to prevent module pollution - Remove unused _clear_pr_env fixture - Fix various other lint and code quality issues These changes address code review findings and improve overall code quality.
packages/specfact-code-review/src/specfact_code_review/ledger/commands.py
Fixed
Show fixed
Hide fixed
packages/specfact-code-review/src/specfact_code_review/rules/commands.py
Fixed
Show fixed
Hide fixed
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Dom <39115308+djm81@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md (1)
21-35:⚠️ Potential issue | 🟠 MajorThe failure/recovery record here does not match the validator.
check-docs-commands.pydoes not learn valid commands from_data/nav.yml; it derives them from the mounted Typer apps and only checksnav.ymlfor route validity. Please replace this block with the actual failing output and the real remediation forspecfact code review run, otherwise this TDD evidence is misleading.As per coding guidelines,
openspec/**/*.md: “Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and drift vs. shipped modules or docs.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md` around lines 21 - 35, The TDD evidence block is inaccurate: update the failing output and remediation to reflect how check-docs-commands.py actually operates (it derives valid commands from the mounted Typer apps, not from _data/nav.yml) — replace the current fake failure/recovery text with the real validator output showing the failing example for the command `specfact code review run` and document the true fix you performed (e.g., register the command in the Typer app or correct the command example in `_data/nav.yml` or command mounting), and ensure the updated note in openspec/docs references check-docs-commands.py, `_data/nav.yml`, and the `specfact code review run` symbol so the evidence matches the validator behavior.
♻️ Duplicate comments (3)
tests/unit/test_pre_commit_quality_parity.py (1)
22-50:⚠️ Potential issue | 🟡 MinorScope ordering assertions to local hooks to avoid false positives.
The check at Line 22–Line 50 builds first-seen order across all repos. That can misreport ordering for the actual local gate chain (
verify-module-signatures→modules-quality-checks→specfact-code-review-gate).♻️ Suggested hardening
- hook_ids: set[str] = set() - ordered_hook_ids: list[str] = [] - seen: set[str] = set() - for repo in repos: - if not isinstance(repo, dict): - continue - for hook in repo.get("hooks", []): - if not isinstance(hook, dict): - continue - hook_id = hook.get("id") - if not isinstance(hook_id, str): - continue - hook_ids.add(hook_id) - if hook_id not in seen: - ordered_hook_ids.append(hook_id) - seen.add(hook_id) + hook_ids: set[str] = set() + local_repo = next( + (repo for repo in repos if isinstance(repo, dict) and repo.get("repo") == "local"), + {}, + ) + local_hooks = local_repo.get("hooks", []) if isinstance(local_repo, dict) else [] + ordered_hook_ids = [ + hook_id + for hook in local_hooks + if isinstance(hook, dict) + for hook_id in [hook.get("id")] + if isinstance(hook_id, str) + ] + for repo in repos: + if isinstance(repo, dict): + for hook in repo.get("hooks", []): + if isinstance(hook, dict) and isinstance(hook.get("id"), str): + hook_ids.add(hook["id"])Based on learnings, quality gates are expected in a specific order:
format → type-check → lint → yaml-lint → verify-modules-signature → contract-test → smart-test → test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_pre_commit_quality_parity.py` around lines 22 - 50, The ordering check currently collects hooks from all repos (variables hook_ids, ordered_hook_ids, seen while iterating repos) which can produce false positives; restrict the collection to only the "local" repo before iterating hooks by adding a guard on repo (e.g., check repo.get("repo") == "local" or the project-specific marker for the local config) so that ordered_hook_ids and hook_ids are built only from the local hooks, then run the expected_order / index_map / itertools.pairwise assertions against that scoped ordered_hook_ids.scripts/check-docs-commands.py (1)
138-140:⚠️ Potential issue | 🟡 MinorKeep explicit empty snapshots in memory.
Line 139 still uses
text or path.read_text(...), so a caller that passes""falls back to disk again. That reintroduces snapshot drift for empty docs or ephemeral files.🔧 Proposed fix
def _extract_command_examples(path: Path, *, text: str | None = None) -> list[CommandExample]: - content = text or path.read_text(encoding="utf-8") + content = text if text is not None else path.read_text(encoding="utf-8") return _extract_command_examples_from_text(content, path)As per coding guidelines,
scripts/**/*.py: “Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-docs-commands.py` around lines 138 - 140, The function _extract_command_examples currently treats an explicit empty string as falsy and falls back to reading the file, causing snapshot drift; change the content selection to only read from path when text is None (e.g., use "content = path.read_text(...)" if text is None else text) so that an explicit "" passed into _extract_command_examples is honored and empty snapshots remain in-memory; update the call path to _extract_command_examples_from_text(content, path) unchanged.openspec/changes/docs-14-module-release-history/tasks.md (1)
1-5: 🧹 Nitpick | 🔵 TrivialConsider a descriptive top-level heading for discoverability.
The file now starts with a level-1 heading (
# 1. Change Setup), satisfying markdownlint MD041. However, per the prior suggestion, a descriptive title like# Tasks: docs-14-module-release-historywould improve navigation when scanning multiple OpenSpec task files.📝 Proposed improvement
+# Tasks: docs-14-module-release-history + -# 1. Change Setup +## 1. Change Setup🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/docs-14-module-release-history/tasks.md` around lines 1 - 5, Replace the generic top-level heading "# 1. Change Setup" with a more descriptive title like "# Tasks: docs-14-module-release-history" so the file is easily discoverable among OpenSpec task files; update the heading text in tasks.md (the line containing "1. Change Setup") and ensure any internal references/TOC entries relying on that heading are adjusted accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 17-20: The release metadata is inconsistent: CHANGELOG.md and
packages/specfact-code-review/module-package.yaml declare version 0.45.2 while
registry/index.json still lists latest_version "0.45.1" and contains 0.45.1
artifacts; fix by making the registry and artifacts match the declared
version—either update registry/index.json to latest_version "0.45.2" and add the
corresponding 0.45.2 tarball, checksum and signature entries (and ensure their
filenames and checksums match the built artifacts), or revert CHANGELOG.md and
packages/specfact-code-review/module-package.yaml back to 0.45.1 so all three
places (CHANGELOG.md, packages/specfact-code-review/module-package.yaml,
registry/index.json) consistently reference the same version.
In `@packages/specfact-code-review/.semgrep/clean_code.yaml`:
- Around line 63-77: The rule "swallowed-exception-pattern" currently warns
about "pass or silent returns" but only matches except blocks with pass; update
the rule to either (A) change the message to remove "silent returns" so it
accurately reflects that only pass is detected, or (B) expand pattern-either to
include additional patterns that match except blocks containing silent return
forms (e.g., "except Exception: return" and "except Exception: return None" and
the bare "except: return" variants) so the patterns align with the message; edit
the id/message/pattern-either entries accordingly (referencing the existing id
"swallowed-exception-pattern" and the "pattern-either" block) to implement one
of these two options.
- Around line 56-77: The ReviewFinding model's category Literal must be expanded
to include the new values (naming, kiss, yagni, dry, solid): update the
ReviewFinding model spec (the spec.md that defines the ReviewFinding category
Literal) to add these five strings, then update the semgrep-runner spec
documentation that maps rule categories to ensure it documents and maps these
new category names, and finally run/adjust the pre-commit validation and any
runner specs that validate category membership so all rules using
naming/kiss/yagni/dry/solid pass validation.
- Around line 57-61: The semgrep rule with id banned-generic-public-names uses a
case-sensitive pattern-regex that only matches lowercase names and therefore
misses CamelCase class names; update the pattern-regex (for the rule id
banned-generic-public-names) to be case-insensitive (e.g., add the inline flag
(?i) or use a case-insensitive group) so it matches both lowercase and CamelCase
variants for the banned tokens (process|handle|manager|data) while preserving
the existing negative lookahead (?!_+) that excludes private/underscored names.
In `@packages/specfact-code-review/module-package.yaml`:
- Line 2: The module-package.yaml currently declares version: 0.45.2 while
registry/index.json still lists latest_version: "0.45.1" and its artifacts;
update registry/index.json to match the new module version by setting
latest_version to "0.45.2" and replacing the artifacts/checksums/urls for that
version to the 0.45.2 artifacts (or bump module-package.yaml back to 0.45.1 if
the registry should remain unchanged), then re-run the signing workflow
(verify-modules-signature --enforce-version-bump) to ensure the registry index
and module-package.yaml are aligned.
- Around line 24-26: The module-package.yaml declares version "0.45.2" with an
integrity checksum that doesn't match the published artifacts; fix this by
either (A) building and publishing the missing tarball named
specfact-code-review-0.45.2.tar.gz, computing and updating integrity.checksum
and signature in module-package.yaml and adding the new version entry to
registry/index.json, or (B) revert module-package.yaml.version back to "0.45.1"
and replace integrity.checksum/signature with the correct values for the
existing specfact-code-review-0.45.1.tar.gz and ensure registry/index.json
continues to reference 0.45.1; also verify the module signature and enforce the
version-bump checks in your publication workflow (look for module-package.yaml,
integrity.checksum, version, registry/index.json, and the tarball filenames to
apply the changes).
In `@packages/specfact-code-review/src/specfact_code_review/review/commands.py`:
- Around line 61-127: The CLI options were hidden behind argparse in
_parse_run_invocation and ctx.args which breaks Typer/Click completion and can
SystemExit; refactor by removing _parse_run_invocation and instead declare the
flags on the run(...) function as Typer options (use typer.Option) for scope,
path (path_filters), json_output, out, score_only, no_tests, fix, and
interactive, and expose include_tests and include_noise as mutually exclusive
Optional[bool] Typer options (or two flags that set the same destination) with
the same defaults you used in parser.set_defaults; update call sites to pass
invocation.* values to run_command (or call _resolve_include_tests(files,
include_tests, interactive) inside run) and delete the argparse logic so Typer
handles validation, help, and errors.
In `@packages/specfact-code-review/src/specfact_code_review/rules/updater.py`:
- Around line 31-43: DEFAULT_DO_RULES is missing the mandatory OpenSpec gate and
explicit TDD sequence; update the tuple DEFAULT_DO_RULES to reintroduce a rule
that requires verifying an active OpenSpec change covers the requested scope and
enforces the sequence "spec delta → failing tests → implementation → passing
tests → quality gates" so that any code paths using default_skill_content(), the
rules init, and rules update flows include this requirement; locate
DEFAULT_DO_RULES in updater.py and add a concise rule string that mentions
verifying an OpenSpec change and following the full
spec→failing-test→implementation→passing-test→quality-gate sequence.
In `@packages/specfact-code-review/src/specfact_code_review/run/commands.py`:
- Around line 405-413: In _render_review_result, the request.score_only branch
returns report.reward_delta but should return the review score; change the
branch in function _render_review_result (and the symbol request.score_only) to
return report.score (and keep the ci exit code semantics: report.ci_exit_code or
0) so the tuple becomes (report.ci_exit_code or 0, str(report.score)). Ensure
you only modify the score_only branch and preserve existing behavior for
json_output and other branches.
- Around line 382-401: The _build_review_run_request adapter must reject
malformed inputs instead of quietly coercing or dropping args: first validate
that files is a list[Path] (reject a ReviewRunRequest or other types), then
validate and consume only known keys (raise ValueError listing unexpected keys)
before constructing ReviewRunRequest; for boolean flags (include_tests,
include_noise, json_output, score_only, no_tests, fix) require actual bool or
None and raise on string/number inputs (do not use bool(...) coercion), and for
scope/path fields feed through _as_auto_scope, _as_path_filters, and
_as_optional_path only after type-checking their raw values (raise on invalid
types). Ensure these checks are added in _build_review_run_request (and the
analogous code block at the other location referenced) so malformed calls are
rejected with clear errors.
In `@packages/specfact-code-review/src/specfact_code_review/run/runner.py`:
- Around line 291-301: Currently the code exempts any __init__.py with missing
coverage; change that logic in the loop that calls _coverage_for_source and uses
percent_covered and source_file.name so that __init__.py is only exempt when
it's a marker/empty module: read the source text of source_file (e.g.,
source_file.read_text()), strip whitespace and docstring/comments, and treat it
as exempt only if it contains no executable statements (or only "pass"/empty).
If the stripped content contains executable code, return the tool_error(...) for
pytest as you do for other files; otherwise continue to the next file. Ensure
you reference percent_covered, _coverage_for_source, source_file.name and
tool_error in the updated conditional.
- Around line 229-243: The spec file openspec/specs/review-finding-model/spec.md
must be updated to include the new category values emitted by ReviewFinding (add
"yagni", "dry", "solid" alongside existing categories) and the new tool values
("ast", "checklist") so consumers match what the AST runner and checklist
produce; edit the schema enumerations and any examples referencing ReviewFinding
or the "tool" and "category" fields to reflect these additions. Also revisit the
blanket coverage suppression in __init__.py (the block around lines 291–301) and
either narrow or remove the suppression so import-time logic and re-exports in
package initializers are still validated for coverage unless intentional to skip
them. Ensure references to ReviewFinding, category, and tool are used to locate
the correct schema sections to change.
In `@packages/specfact-code-review/src/specfact_code_review/run/scorer.py`:
- Around line 77-91: The score_review signature currently uses **kwargs: object
which loses call-site typing; change it to accept a typed mapping (e.g., define
a TypedDict ScoreModifierKwargs with optional keys zero_loc_violations,
zero_complexity_violations, all_apis_have_icontract, coverage_90_plus,
no_new_suppressions and then use **kwargs: Unpack[ScoreModifierKwargs]) or
revert to explicit keyword parameters for score_review so the
ReviewScoreModifiers construction remains the same (referencing score_review,
ReviewScoreModifiers, and kwargs) and keep the existing runtime ValueError guard
for unexpected keys.
In
`@packages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.py`:
- Around line 94-109: The current try/except around the subprocess.run and
_diagnostics_from_output call hides basedpyright stderr; split the concerns so
execution errors (FileNotFoundError, OSError, subprocess.TimeoutExpired) raised
by subprocess.run return a tool_error with that execution exception, while
JSON/parse errors (json.JSONDecodeError, KeyError, ValueError) raised by
_diagnostics_from_output include result.stderr (if present) in the tool_error
message; update the exception handling around the subprocess.run invocation and
the call to _diagnostics_from_output in basedpyright_runner.py so tool_error for
parsing failures references stderr and tool_error for run failures references
the run exception details.
In
`@packages/specfact-code-review/src/specfact_code_review/tools/radon_runner.py`:
- Around line 166-177: The _kiss_parameter_findings function counts the implicit
receiver (leading "self" or "cls") as a parameter causing false positives;
detect if function_node.args.args is non-empty and the first argument's name is
"self" or "cls" and subtract one from parameter_count before comparing against
_KISS_PARAMETER_WARNING so instance/class methods don't include the implicit
receiver in the KISS limit check; update the logic in _kiss_parameter_findings
(and ensure parameter_count stays >= 0) to perform this adjustment.
In `@packages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.py`:
- Around line 102-112: The subprocess run and JSON payload parsing are mixed in
one try/except which hides Ruff's stderr diagnostics; split them: call
subprocess.run (as currently invoked) in its own try that catches
FileNotFoundError, OSError, and subprocess.TimeoutExpired and returns
tool_error(tool="ruff", file_path=files[0], message=...) including result.stderr
(or the exception text if result is not available); then call
_payload_from_output(result.stdout) in a separate try that catches ValueError
and json.JSONDecodeError and returns tool_error(..., message=f"Unable to parse
Ruff output: {exc}; stderr: {result.stderr}") so parsing errors preserve Ruff's
stderr for CI/debugging.
In `@registry/index.json`:
- Around line 76-78: Update the registry entry fields to match the bumped
package version in packages/specfact-code-review/module-package.yaml: change
"latest_version" to "0.45.2", update "download_url" to point to the
modules/specfact-code-review-0.45.2.tar.gz artifact, and replace
"checksum_sha256" with the checksum for the 0.45.2 tarball (and update any
associated signature artifact references if present). Ensure the values in
registry/index.json for latest_version, download_url, and checksum_sha256
exactly match the 0.45.2 build artifacts so CI/signature checks and installs
align with the module-package.yaml version.
In `@tests/unit/scripts/test_pre_commit_code_review.py`:
- Around line 44-48: The test currently checks the command prefix up to "review"
and can miss a missing "run"; update the prefix assertion for the command
variable in test_pre_commit_code_review.py so it includes the "run" subcommand
(e.g., extend the slice or comparison to include "run" so the expected prefix is
[sys.executable, "-m", "specfact_cli.cli", "code", "review", "run"]); keep the
rest of the assertions (e.g., presence of "--json", "--out",
module.REVIEW_JSON_OUT, and the final file args) unchanged.
In `@tests/unit/specfact_code_review/test___init__.py`:
- Around line 24-28: The test test_getattr_raises_for_invalid_attribute is
incorrectly using importlib.util.find_spec which does not invoke
specfact_code_review.__getattr__; replace that check with direct attribute
access on the imported module (import specfact_code_review then access
specfact_code_review.invalid_attribute) and assert that accessing the attribute
raises AttributeError (e.g., use pytest.raises(AttributeError)) so the module's
__getattr__ lazy-export guard is exercised.
In `@tests/unit/specfact_code_review/tools/test_pylint_runner.py`:
- Around line 14-15: The test test_run_pylint_returns_empty_for_no_files should
verify run_pylint([]) takes the fast path and does not invoke the external
process; modify the test to patch/mock subprocess.run (e.g., with monkeypatch or
unittest.mock.patch targeting 'subprocess.run') before calling run_pylint([]),
assert the function still returns [] and then assert subprocess.run was never
called; reference the test name test_run_pylint_returns_empty_for_no_files and
the implementation function run_pylint to locate where to add the mock and the
"not called" assertion.
In `@tests/unit/specfact_code_review/tools/test_radon_runner.py`:
- Around line 68-98: Tests emit findings with a new "kiss" category from
run_radon (and the local ReviewFinding model uses new categories like kiss,
naming, yagni, dry, solid), but the authoritative spec and the minimal
ReviewFinding used by scripts/pre_commit_code_review.py do not document or
accept these literals; update the spec at
openspec/specs/review-finding-model/spec.md to include the new category literals
(kiss, naming, yagni, dry, solid) in the allowed enum for category and then
update scripts/pre_commit_code_review.py's minimal ReviewFinding parsing (or its
ConfigDict) so it accepts/validates the expanded set (or explicitly maps these
categories) and run downstream consumers/CI checks to verify IDEs/dashboards
handle the expanded category set.
---
Outside diff comments:
In `@openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md`:
- Around line 21-35: The TDD evidence block is inaccurate: update the failing
output and remediation to reflect how check-docs-commands.py actually operates
(it derives valid commands from the mounted Typer apps, not from _data/nav.yml)
— replace the current fake failure/recovery text with the real validator output
showing the failing example for the command `specfact code review run` and
document the true fix you performed (e.g., register the command in the Typer app
or correct the command example in `_data/nav.yml` or command mounting), and
ensure the updated note in openspec/docs references check-docs-commands.py,
`_data/nav.yml`, and the `specfact code review run` symbol so the evidence
matches the validator behavior.
---
Duplicate comments:
In `@openspec/changes/docs-14-module-release-history/tasks.md`:
- Around line 1-5: Replace the generic top-level heading "# 1. Change Setup"
with a more descriptive title like "# Tasks: docs-14-module-release-history" so
the file is easily discoverable among OpenSpec task files; update the heading
text in tasks.md (the line containing "1. Change Setup") and ensure any internal
references/TOC entries relying on that heading are adjusted accordingly.
In `@scripts/check-docs-commands.py`:
- Around line 138-140: The function _extract_command_examples currently treats
an explicit empty string as falsy and falls back to reading the file, causing
snapshot drift; change the content selection to only read from path when text is
None (e.g., use "content = path.read_text(...)" if text is None else text) so
that an explicit "" passed into _extract_command_examples is honored and empty
snapshots remain in-memory; update the call path to
_extract_command_examples_from_text(content, path) unchanged.
In `@tests/unit/test_pre_commit_quality_parity.py`:
- Around line 22-50: The ordering check currently collects hooks from all repos
(variables hook_ids, ordered_hook_ids, seen while iterating repos) which can
produce false positives; restrict the collection to only the "local" repo before
iterating hooks by adding a guard on repo (e.g., check repo.get("repo") ==
"local" or the project-specific marker for the local config) so that
ordered_hook_ids and hook_ids are built only from the local hooks, then run the
expected_order / index_map / itertools.pairwise assertions against that scoped
ordered_hook_ids.
🪄 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: 6911aac7-7c53-47c1-ba3c-9851e2279e32
📒 Files selected for processing (40)
.vibe/skills/specfact-code-review/SKILL.mdCHANGELOG.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-14-module-release-history/tasks.mdpackages/specfact-code-review/.semgrep/clean_code.yamlpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/__init__.pypackages/specfact-code-review/src/specfact_code_review/_review_utils.pypackages/specfact-code-review/src/specfact_code_review/ledger/client.pypackages/specfact-code-review/src/specfact_code_review/ledger/commands.pypackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/contract_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.pyregistry/index.jsonscripts/check-docs-commands.pyskills/specfact-code-review/SKILL.mdtests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/__init__.pytests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/test___init__.pytests/unit/specfact_code_review/test__review_utils.pytests/unit/specfact_code_review/tools/helpers.pytests/unit/specfact_code_review/tools/test_basedpyright_runner.pytests/unit/specfact_code_review/tools/test_pylint_runner.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/test_pre_commit_quality_parity.py
💤 Files with no reviewable changes (1)
- tests/unit/specfact_code_review/init.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.11)
- GitHub Check: quality (3.12)
- GitHub Check: quality (3.13)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Line length must be 120 characters
Python target version is 3.11+
rufflinting runs on the full repository
Files:
tests/unit/specfact_code_review/review/test_commands.pypackages/specfact-code-review/src/specfact_code_review/ledger/commands.pytests/unit/specfact_code_review/test__review_utils.pypackages/specfact-code-review/src/specfact_code_review/__init__.pytests/unit/specfact_code_review/tools/test_basedpyright_runner.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/helpers.pytests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/test___init__.pypackages/specfact-code-review/src/specfact_code_review/tools/contract_runner.pytests/unit/specfact_code_review/tools/test_pylint_runner.pypackages/specfact-code-review/src/specfact_code_review/ledger/client.pypackages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.pypackages/specfact-code-review/src/specfact_code_review/_review_utils.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.pytests/unit/specfact_code_review/rules/test_updater.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.pytests/unit/scripts/test_pre_commit_code_review.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pytests/unit/specfact_code_review/run/test_runner.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pyscripts/check-docs-commands.py
{src,tests,tools}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
basedpyrightandpylintare scoped tosrc/,tests/, andtools/directoriesPython source code must pass linting with pylint via
hatch run linttargeting src/, tests/, and tools/ directories
Files:
tests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/test__review_utils.pytests/unit/specfact_code_review/tools/test_basedpyright_runner.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/helpers.pytests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/test___init__.pytests/unit/specfact_code_review/tools/test_pylint_runner.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.py
tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Use tests/ directory for bundle behavior and migration parity tests
Files:
tests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/test__review_utils.pytests/unit/specfact_code_review/tools/test_basedpyright_runner.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/helpers.pytests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/test___init__.pytests/unit/specfact_code_review/tools/test_pylint_runner.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.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/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/test__review_utils.pytests/unit/specfact_code_review/tools/test_basedpyright_runner.pytests/unit/specfact_code_review/tools/test_radon_runner.pytests/unit/specfact_code_review/tools/helpers.pytests/unit/test_pre_commit_quality_parity.pytests/unit/specfact_code_review/test___init__.pytests/unit/specfact_code_review/tools/test_pylint_runner.pytests/unit/specfact_code_review/rules/test_updater.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/src/**/*.py: Only allowedspecfact_cli.*prefixes may be imported in bundle code (CORE/SHARED APIs only)
Cross-bundle lateral imports are forbidden except specific allowed pairs (e.g.specfact_spec->specfact_project)
Files:
packages/specfact-code-review/src/specfact_code_review/ledger/commands.pypackages/specfact-code-review/src/specfact_code_review/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/contract_runner.pypackages/specfact-code-review/src/specfact_code_review/ledger/client.pypackages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.pypackages/specfact-code-review/src/specfact_code_review/_review_utils.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.py
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep bundle package code under packages/ directory
Files:
packages/specfact-code-review/src/specfact_code_review/ledger/commands.pypackages/specfact-code-review/src/specfact_code_review/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/contract_runner.pypackages/specfact-code-review/src/specfact_code_review/ledger/client.pypackages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.pypackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.pypackages/specfact-code-review/src/specfact_code_review/_review_utils.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.pypackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.py
packages/**/src/**/*.py
⚙️ CodeRabbit configuration file
packages/**/src/**/*.py: 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.
Files:
packages/specfact-code-review/src/specfact_code_review/ledger/commands.pypackages/specfact-code-review/src/specfact_code_review/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/contract_runner.pypackages/specfact-code-review/src/specfact_code_review/ledger/client.pypackages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/tools/pylint_runner.pypackages/specfact-code-review/src/specfact_code_review/_review_utils.pypackages/specfact-code-review/src/specfact_code_review/tools/radon_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.pypackages/specfact-code-review/src/specfact_code_review/rules/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/__init__.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.py
openspec/changes/**
📄 CodeRabbit inference engine (CLAUDE.md)
Never manually move folders under
openspec/changes/intoarchive/. Archiving MUST useopenspec archive <change-id>command
Files:
openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.md
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-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.md
openspec/changes/*/TDD_EVIDENCE.md
📄 CodeRabbit inference engine (AGENTS.md)
Record failing/passing test evidence in openspec/changes//TDD_EVIDENCE.md
Files:
openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md
packages/*/module-package.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use SemVer for bundle versioning: patch (bug fix), minor (new command/option/API), major (breaking change/removal)
packages/*/module-package.yaml: Modules must have signatures verified with version bump enforcement viaverify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump
Bundle version changes must be SemVer-compliant: patch for bug fixes, minor for new commands/options/APIs, major for breaking changes
Files:
packages/specfact-code-review/module-package.yaml
{packages/*/module-package.yaml,registry/index.json}
📄 CodeRabbit inference engine (CLAUDE.md)
When bumping a bundle version, review and update
core_compatibilityin bothmodule-package.yamlandregistry/index.jsonUpdate
core_compatibilityfield in bothpackages/<bundle>/module-package.yamlandregistry/index.jsonwhen bundle requires a newer minimum specfact-cli version
Files:
packages/specfact-code-review/module-package.yamlregistry/index.json
**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
All YAML configuration files must pass yaml-lint validation via
hatch run yaml-lint
Files:
packages/specfact-code-review/module-package.yaml
{registry/index.json,packages/*/module-package.yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Keep registry metadata in registry/index.json and packages/*/module-package.yaml
Files:
packages/specfact-code-review/module-package.yamlregistry/index.json
packages/**/module-package.yaml
⚙️ CodeRabbit configuration file
packages/**/module-package.yaml: Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
Files:
packages/specfact-code-review/module-package.yaml
registry/**
⚙️ CodeRabbit configuration file
registry/**: Registry and index consistency: bundle listings, version pins, and compatibility with
published module artifacts.
Files:
registry/index.json
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/check-docs-commands.py
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Before changing code, verify an active OpenSpec change explicitly covers the requested scope; follow strict TDD order: spec delta → failing tests → implementation → passing tests → quality gates
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : Update `core_compatibility` field in both `packages/<bundle>/module-package.yaml` and `registry/index.json` when bundle requires a newer minimum specfact-cli version
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to packages/*/module-package.yaml : Bundle version changes must be SemVer-compliant: patch for bug fixes, minor for new commands/options/APIs, major for breaking changes
Applied to files:
CHANGELOG.mdpackages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/module-package.yaml : Use SemVer for bundle versioning: patch (bug fix), minor (new command/option/API), major (breaking change/removal)
Applied to files:
CHANGELOG.mdpackages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : When bumping a bundle version, review and update `core_compatibility` in both `module-package.yaml` and `registry/index.json`
Applied to files:
CHANGELOG.mdpackages/specfact-code-review/module-package.yamlregistry/index.json
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/src/**/*.py : Only allowed `specfact_cli.*` prefixes may be imported in bundle code (CORE/SHARED APIs only)
Applied to files:
packages/specfact-code-review/src/specfact_code_review/__init__.pytests/unit/specfact_code_review/test___init__.pypackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md.vibe/skills/specfact-code-review/SKILL.mdskills/specfact-code-review/SKILL.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/src/**/*.py : Cross-bundle lateral imports are forbidden except specific allowed pairs (e.g. `specfact_spec` -> `specfact_project`)
Applied to files:
packages/specfact-code-review/src/specfact_code_review/__init__.pytests/unit/specfact_code_review/test___init__.py.vibe/skills/specfact-code-review/SKILL.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to {src,tests,tools}/**/*.py : `basedpyright` and `pylint` are scoped to `src/`, `tests/`, and `tools/` directories
Applied to files:
tests/unit/specfact_code_review/tools/test_basedpyright_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/basedpyright_runner.pytests/unit/specfact_code_review/run/test_runner.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to openspec/CHANGE_ORDER.md : Update openspec/CHANGE_ORDER.md when archive status changes in OpenSpec changes
Applied to files:
openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to openspec/changes/*/TDD_EVIDENCE.md : Record failing/passing test evidence in openspec/changes/<change-id>/TDD_EVIDENCE.md
Applied to files:
openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.mdopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Run quality gates in order: format, type-check, lint, yaml-lint, verify-modules-signature, contract-test, smart-test, test
Applied to files:
tests/unit/test_pre_commit_quality_parity.pyopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Run quality gates in the specified order: format → type-check → lint → yaml-lint → verify-modules-signature → contract-test → smart-test → test
Applied to files:
tests/unit/test_pre_commit_quality_parity.pyopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.md
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Before changing code, verify an active OpenSpec change explicitly covers the requested scope; follow strict TDD order: spec delta → failing tests → implementation → passing tests → quality gates
Applied to files:
tests/unit/test_pre_commit_quality_parity.pyopenspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.mdpackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md.vibe/skills/specfact-code-review/SKILL.mdskills/specfact-code-review/SKILL.mdtests/unit/specfact_code_review/run/test_runner.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Run pre-commit hooks locally via `pre-commit install` and `pre-commit run --all-files` to mirror CI quality gates
Applied to files:
tests/unit/test_pre_commit_quality_parity.py
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Pre-commit hooks must mirror CI configuration: run `pre-commit install && pre-commit run --all-files`
Applied to files:
tests/unit/test_pre_commit_quality_parity.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {src,tests,tools}/**/*.py : Python source code must pass linting with pylint via `hatch run lint` targeting src/, tests/, and tools/ directories
Applied to files:
tests/unit/specfact_code_review/tools/test_pylint_runner.py
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Follow strict TDD order: spec delta -> failing tests -> implementation -> passing tests -> quality gates. Record TDD evidence in `openspec/changes/<change-id>/TDD_EVIDENCE.md`
Applied to files:
openspec/changes/docs-13-nav-search-theme-roles/TDD_EVIDENCE.mdopenspec/changes/docs-14-module-release-history/tasks.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to **/*.py : `ruff` linting runs on the full repository
Applied to files:
packages/specfact-code-review/src/specfact_code_review/tools/ruff_runner.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : Update `core_compatibility` field in both `packages/<bundle>/module-package.yaml` and `registry/index.json` when bundle requires a newer minimum specfact-cli version
Applied to files:
packages/specfact-code-review/module-package.yamlregistry/index.json
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to packages/*/module-package.yaml : Modules must have signatures verified with version bump enforcement via `verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump`
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {registry/index.json,packages/*/module-package.yaml} : Keep registry metadata in registry/index.json and packages/*/module-package.yaml
Applied to files:
registry/index.json
🪛 LanguageTool
openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.md
[grammar] ~34-~34: Use a hyphen to join words.
Context: ...tory source #### Scenario: Release-note style guidance is available to AI copilo...
(QB_NEW_EN_HYPHEN)
openspec/changes/docs-14-module-release-history/tasks.md
[uncategorized] ~14-~14: The official name of this software platform is spelled with a capital “H”.
Context: ... Workflow Integration - [ ] 3.1 Extend .github/workflows/publish-modules.yml so each ...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli
nold-ai/specfact-cli — findings
-
ReviewFinding model consumers / code-review JSON
- scripts/pre_commit_code_review.py defines a local ReviewFinding Pydantic model and reads/writes .specfact/code-review.json; the pre-commit hook and its tests expect the report shape and category set. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:32]
- tests/unit/scripts/test_pre_commit_code_review.py exercises the JSON summary/overall_verdict behavior and expects specific severity buckets. [::nold-ai/specfact-cli::tests/unit/scripts/test_pre_commit_code_review.py:1]
-
OpenSpec ReviewFinding / spec validation points
- The repo contains an authoritative spec for the ReviewFinding Pydantic model; many spec files assume the allowed categories/severity/tool fields (openspec/specs/review-finding-model/spec.md). Changes that add new category literals must be accepted by this model/spec. [::nold-ai/specfact-cli::openspec/specs/review-finding-model/spec.md:6]
- Runner specs (semgrep/ruff/radon/pylint/basedpyright/contract) map tool outputs to ReviewFinding and expect category values; they will need to accept the new categories (naming,kiss,yagni,dry,solid). [::nold-ai/specfact-cli::openspec/specs/semgrep-runner/spec.md, openspec/specs/radon-runner/spec.md, openspec/specs/ruff-runner/spec.md, openspec/specs/pylint-runner/spec.md, openspec/specs/basedpyright-runner/spec.md, openspec/specs/contract-runner/spec.md]
-
Registry / marketplace consumers of module metadata
- scripts/verify-bundle-published.py defaults to ../specfact-cli-modules/registry/index.json and other scripts/docs expect specfact-cli-modules/registry/index.json layout; registry index changes in the modules repo must match these consumers. [::nold-ai/specfact-cli::scripts/verify-bundle-published.py:56]
- src/specfact_cli/registry/marketplace_client.py constructs raw GitHub URLs to specfact-cli-modules/{branch}/registry/index.json — ensure registry/index.json schema and field names (e.g., core_compatibility, checksum fields, signatures) remain compatible. [::nold-ai/specfact-cli::src/specfact_cli/registry/marketplace_client.py:24]
- Docs and guides reference registry/index.json and publishing workflows (docs/module-system/marketplace.md, docs/guides/publishing-modules.md). [::nold-ai/specfact-cli::docs/module-system/marketplace.md:19][::nold-ai/specfact-cli::docs/guides/publishing-modules.md:88]
-
Code-review evidence / gating expectations
- Multiple docs, AGENTS.md, and openspec/config.yaml treat .specfact/code-review.json as mandatory evidence and describe commands/integration; consumers expect the JSON envelope fields (overall_verdict, ci_exit_code, findings[]). Any change in ReviewReport shape or field names must be reflected here. [::nold-ai/specfact-cli::AGENTS.md:42][::nold-ai/specfact-cli::openspec/config.yaml:64]
-
VALID_CATEGORIES / module grouping (possible collision)
- src/specfact_cli/registry/module_grouping.py defines VALID_CATEGORIES for marketplace grouping; confirm new code-review categories are only added to review findings and do not conflict with module registry category names. [::nold-ai/specfact-cli::src/specfact_cli/registry/module_grouping.py:11]
Notes / risks to verify before merge
- Ensure the ReviewFinding Pydantic model/spec and any JSON-report consumers accept the expanded category Literal set (naming,kiss,yagni,dry,solid) and any new tool identifiers (e.g., "ast", "radon-kiss", "radon-kiss" tool labels). Verify tests that validate ReviewFinding shape are updated accordingly. [::nold-ai/specfact-cli::openspec/specs/review-finding-model/spec.md]
- Confirm registry/index.json field additions (core_compatibility, checksum/signature file names) in the modules repo match consumers (marketplace_client, verify scripts) and docs. [::nold-ai/specfact-cli::src/specfact_cli/registry/marketplace_client.py:24][::nold-ai/specfact-cli::scripts/verify-bundle-published.py:56]
🔇 Additional comments (11)
packages/specfact-code-review/src/specfact_code_review/ledger/client.py (2)
187-193: Good extraction of payload-entry validation into a single helper.This keeps Supabase row parsing logic centralized and preserves the existing fail-closed validation behavior.
217-220: Run parsing loop is clearer and keeps adapter behavior consistent.Delegating validation to
_ledger_run_from_payload_entrymakes_read_supabase_runseasier to maintain while preserving ordering and fallback semantics.openspec/changes/docs-14-module-release-history/specs/module-release-history-docs/spec.md (2)
34-34: Minor: Hyphenate "Release-note" for grammatical consistency.Static analysis flagged that "Release-note" as a compound modifier should be hyphenated.
📝 Proposed fix
-#### Scenario: Release-note style guidance is available to AI copilot +#### Scenario: Release-note style guidance is available to AI copilotWait—the line already shows "Release-note". Let me re-check: the static analysis hint says to "Use a hyphen to join words" for "Release-note style". The current text appears to already have the hyphen. This may be a false positive from the linter if the hyphen is present.
1-38: Spec requirements are well-structured and testable.The two requirements cover:
- Build-time rendering of release history with graceful degradation for sparse data
- OpenSpec rule integration for future release-history updates with AI drafting guidance
The GIVEN/WHEN/THEN scenarios are specific and verifiable. The informal wording from the prior review ("technical bla bla") has been corrected to "unnecessary technical jargon or generic filler text" (line 38).
tests/unit/specfact_code_review/tools/test_radon_runner.py (1)
11-11: Good: Duplicate fixture setup extracted to shared helper.The
create_noisy_filehelper extraction addresses the prior review feedback about DRY violations. This improves test maintainability.registry/index.json (1)
79-79: Good:core_compatibilityfield now present.This addresses the prior review feedback. The registry entry now declares
"core_compatibility": ">=0.40.0,<1.0.0", matchingmodule-package.yaml. Downstream consumers likemarketplace_client.pycan now properly validate version compatibility.packages/specfact-code-review/src/specfact_code_review/run/scorer.py (1)
29-37: Good: Clean dataclass encapsulation for score modifiers.The
ReviewScoreModifiersfrozen dataclass provides clear documentation of the five boolean modifiers and their defaults. This is a clean refactor from the previous five explicit parameters.packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md (2)
11-12: Good: Blank lines around headings added.This addresses the prior markdownlint feedback (MD022). The
## DOand## DON'Theadings now have proper blank line spacing.Also applies to: 24-25
14-22: Clean-code charter principles clearly codified.The refreshed DO section explicitly maps to the five clean-code categories now covered by the review module:
- KISS: Line 15 (120 LOC, shallow nesting, ≤5 parameters)
- YAGNI: Line 16 (delete unused helpers quickly)
- DRY: Line 17 (extract repeated shapes)
- SOLID: Line 18 (split persistence/transport concerns)
This aligns well with the expanded review coverage mentioned in the CHANGELOG. The guidance is actionable and specific.
openspec/changes/docs-14-module-release-history/tasks.md (1)
6-42: Task plan is comprehensive and well-structured.The tasks cover the full release-history docs workflow:
- Data model definition (canonical schema, ownership)
- Publish workflow integration with AI drafting rules
- Historical backfill with human review gates
- Build-time docs rendering with graceful degradation
- OpenSpec rule updates for future maintainability
- Verification checkpoints
This aligns with the spec requirements in
specs/module-release-history-docs/spec.md.packages/specfact-code-review/src/specfact_code_review/tools/ast_clean_code_runner.py (1)
115-119: Verify the coreReviewFindingcontract before publishing these new literals.This runner now emits
yagni/dry/solidcategories andtool="ast". Linkednold-ai/specfact-cliconsumers and OpenSpec specs validate those fields as explicit enums, so the bundle-side change needs the paired core acceptance before release. If that raises the minimum supported core version, ship the matchingcore_compatibilitybump in bundle metadata and the registry. Based on learnings: Applies to {packages/*/module-package.yaml,registry/index.json} : Updatecore_compatibilityfield in bothpackages/<bundle>/module-package.yamlandregistry/index.jsonwhen bundle requires a newer minimum specfact-cli version.Also applies to: 141-145, 162-166, 194-196
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/specfact-code-review/module-package.yaml (1)
24-26:⚠️ Potential issue | 🔴 CriticalIntegrity checksum and signature must match the published 0.45.3 tarball—verify signing workflow completed.
The declared
integrity.checksum(c5bccded...) andsignaturehave been updated, but the registry still references the 0.45.1 artifacts with checksum72372145.... Before merging:
- Ensure the
specfact-code-review-0.45.3.tar.gztarball has been built and placed inregistry/modules/- Verify the checksum matches the actual tarball:
sha256sum registry/modules/specfact-code-review-0.45.3.tar.gz- Confirm the signing workflow completed:
hatch run sign-modules --module specfact-code-reviewandhatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump- Update
registry/index.jsonto reference version 0.45.3 with the matching checksum and download URLThe PR objectives mention "signature and version integrity checks marked complete" and "manifests re-signed," but the registry artifacts lag behind the manifest declaration. This will fail CI signature verification and block installation in the linked specfact-cli repository.
Based on coding guidelines, modules must have signatures verified with version bump enforcement via
verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/specfact-code-review/module-package.yaml` around lines 24 - 26, The manifest's integrity.checksum and signature were updated to 0.45.3 but the registry still references 0.45.1 artifacts; build the specfact-code-review-0.45.3.tar.gz and place it into registry/modules/, compute and replace the integrity.checksum with the sha256sum of that tarball and ensure integrity.signature is produced by the signing workflow; run hatch run sign-modules --module specfact-code-review and then hatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump to confirm signing, and finally update registry/index.json to reference version 0.45.3 with the exact checksum and download URL so the manifest (integrity.checksum and signature) and registry artifacts match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/specfact-code-review/module-package.yaml`:
- Line 2: module-package.yaml declares version 0.45.3 but registry/index.json
still lists nold-ai/specfact-code-review at latest_version "0.45.1" (mismatched
download_url/checksum); update registry/index.json to match module-package.yaml
by setting the module entry for "nold-ai/specfact-code-review" to latest_version
"0.45.3", update download_url and checksum_sha256 to point to the 0.45.3 tarball
and signature, and ensure registry/modules/specfact-code-review-0.45.3.tar.gz
and registry/signatures/specfact-code-review-0.45.3.* exist (or publish
artifacts first) so verify-modules-signature --enforce-version-bump will pass.
In `@tests/unit/specfact_code_review/test___init__.py`:
- Around line 42-46: The current manual cleanup only pops
"specfact_code_review.review.app" and "specfact_code_review.review" from
sys.modules but misses transitive imports like
"specfact_code_review.review.commands" due to the package __getattr__ logic;
either remove this manual cleanup entirely (rely on pytest isolation) or replace
it with a pytest fixture that, before/after the test, iterates sys.modules keys
and removes any entry that startswith "specfact_code_review.review" (thereby
clearing "specfact_code_review.review.commands" and any submodules) so
__getattr__, specfact_code_review.review.app, and related imports are fully
reset.
---
Duplicate comments:
In `@packages/specfact-code-review/module-package.yaml`:
- Around line 24-26: The manifest's integrity.checksum and signature were
updated to 0.45.3 but the registry still references 0.45.1 artifacts; build the
specfact-code-review-0.45.3.tar.gz and place it into registry/modules/, compute
and replace the integrity.checksum with the sha256sum of that tarball and ensure
integrity.signature is produced by the signing workflow; run hatch run
sign-modules --module specfact-code-review and then hatch run
verify-modules-signature --require-signature --payload-from-filesystem
--enforce-version-bump to confirm signing, and finally update
registry/index.json to reference version 0.45.3 with the exact checksum and
download URL so the manifest (integrity.checksum and signature) and registry
artifacts match.
🪄 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: 1965a853-e500-4ad7-8e15-9ad9bdfd0915
📒 Files selected for processing (3)
packages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/rules/commands.pytests/unit/specfact_code_review/test___init__.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Line length must be 120 characters
Python target version is 3.11+
rufflinting runs on the full repository
Files:
tests/unit/specfact_code_review/test___init__.pypackages/specfact-code-review/src/specfact_code_review/rules/commands.py
{src,tests,tools}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
basedpyrightandpylintare scoped tosrc/,tests/, andtools/directoriesPython source code must pass linting with pylint via
hatch run linttargeting src/, tests/, and tools/ directories
Files:
tests/unit/specfact_code_review/test___init__.py
tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Use tests/ directory for bundle behavior and migration parity tests
Files:
tests/unit/specfact_code_review/test___init__.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/specfact_code_review/test___init__.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/src/**/*.py: Only allowedspecfact_cli.*prefixes may be imported in bundle code (CORE/SHARED APIs only)
Cross-bundle lateral imports are forbidden except specific allowed pairs (e.g.specfact_spec->specfact_project)
Files:
packages/specfact-code-review/src/specfact_code_review/rules/commands.py
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep bundle package code under packages/ directory
Files:
packages/specfact-code-review/src/specfact_code_review/rules/commands.pypackages/specfact-code-review/module-package.yaml
packages/**/src/**/*.py
⚙️ CodeRabbit configuration file
packages/**/src/**/*.py: 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.
Files:
packages/specfact-code-review/src/specfact_code_review/rules/commands.py
packages/*/module-package.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use SemVer for bundle versioning: patch (bug fix), minor (new command/option/API), major (breaking change/removal)
packages/*/module-package.yaml: Modules must have signatures verified with version bump enforcement viaverify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump
Bundle version changes must be SemVer-compliant: patch for bug fixes, minor for new commands/options/APIs, major for breaking changes
Files:
packages/specfact-code-review/module-package.yaml
{packages/*/module-package.yaml,registry/index.json}
📄 CodeRabbit inference engine (CLAUDE.md)
When bumping a bundle version, review and update
core_compatibilityin bothmodule-package.yamlandregistry/index.jsonUpdate
core_compatibilityfield in bothpackages/<bundle>/module-package.yamlandregistry/index.jsonwhen bundle requires a newer minimum specfact-cli version
Files:
packages/specfact-code-review/module-package.yaml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
All YAML configuration files must pass yaml-lint validation via
hatch run yaml-lint
Files:
packages/specfact-code-review/module-package.yaml
{registry/index.json,packages/*/module-package.yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Keep registry metadata in registry/index.json and packages/*/module-package.yaml
Files:
packages/specfact-code-review/module-package.yaml
packages/**/module-package.yaml
⚙️ CodeRabbit configuration file
packages/**/module-package.yaml: Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
Files:
packages/specfact-code-review/module-package.yaml
🧠 Learnings (7)
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/src/**/*.py : Only allowed `specfact_cli.*` prefixes may be imported in bundle code (CORE/SHARED APIs only)
Applied to files:
tests/unit/specfact_code_review/test___init__.py
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/src/**/*.py : Cross-bundle lateral imports are forbidden except specific allowed pairs (e.g. `specfact_spec` -> `specfact_project`)
Applied to files:
tests/unit/specfact_code_review/test___init__.py
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : Update `core_compatibility` field in both `packages/<bundle>/module-package.yaml` and `registry/index.json` when bundle requires a newer minimum specfact-cli version
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to packages/*/module-package.yaml : Modules must have signatures verified with version bump enforcement via `verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump`
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/module-package.yaml : Use SemVer for bundle versioning: patch (bug fix), minor (new command/option/API), major (breaking change/removal)
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-26T23:11:11.388Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T23:11:11.388Z
Learning: Applies to packages/*/module-package.yaml : Bundle version changes must be SemVer-compliant: patch for bug fixes, minor for new commands/options/APIs, major for breaking changes
Applied to files:
packages/specfact-code-review/module-package.yaml
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : When bumping a bundle version, review and update `core_compatibility` in both `module-package.yaml` and `registry/index.json`
Applied to files:
packages/specfact-code-review/module-package.yaml
🔀 Multi-repo context nold-ai/specfact-cli
Findings (repo: nold-ai/specfact-cli)
-
Local pre-commit hook / JSON consumer
- scripts/pre_commit_code_review.py defines a ReviewFinding BaseModel and writes/reads .specfact/code-review.json; tests expect the report shape and severity bucketing. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:3,32,61,117]
- tests referencing the file and expected behavior: tests/unit/scripts/test_pre_commit_code_review.py. [::nold-ai/specfact-cli::tests/unit/scripts/test_pre_commit_code_review.py:1]
-
ReviewFinding model / specs consumers
- OpenSpec requirement and runner specs assume a ReviewFinding Pydantic model and a limited set of categories; spec file: openspec/specs/review-finding-model/spec.md. Changes that add categories must be accepted here. [::nold-ai/specfact-cli::openspec/specs/review-finding-model/spec.md:6]
- Multiple runner specs (semgrep/ruff/radon/pylint/basedpyright/contract) map tool outputs to ReviewFinding; these are in openspec/specs/* (e.g., semgrep-runner, ruff-runner). [::nold-ai/specfact-cli::openspec/specs/semgrep-runner/spec.md:7][::nold-ai/specfact-cli::openspec/specs/ruff-runner/spec.md:6][::nold-ai/specfact-cli::openspec/specs/radon-runner/spec.md:12]
-
Registry index / core_compatibility consumers
- marketplace client builds raw GitHub URLs to specfact-cli-modules/registry/index.json: src/specfact_cli/registry/marketplace_client.py. Any schema additions (e.g., core_compatibility) must be compatible with this client. [::nold-ai/specfact-cli::src/specfact_cli/registry/marketplace_client.py:24]
- Module manifest parsing and compatibility checks reference core_compatibility: src/specfact_cli/registry/module_packages.py and module_installer use this field to decide installs. [::nold-ai/specfact-cli::src/specfact_cli/registry/module_packages.py:289,373][::nold-ai/specfact-cli::src/specfact_cli/registry/module_installer.py:726]
- Tests and docs assert core_compatibility semantics (tests/unit/specfact_cli/registry/test_version_constraints.py; docs/guides/publishing-modules.md). [::nold-ai/specfact-cli::tests/unit/specfact_cli/registry/test_version_constraints.py:5][::nold-ai/specfact-cli::docs/guides/publishing-modules.md:88]
-
VALID_CATEGORIES / module grouping
- Module grouping defines VALID_CATEGORIES used when validating module metadata; new review-only categories (naming,kiss,yagni,dry,solid) should not conflict with these registry category names. File: src/specfact_cli/registry/module_grouping.py. [::nold-ai/specfact-cli::src/specfact_cli/registry/module_grouping.py:11]
-
Documentation / gating for .specfact/code-review.json
- Several docs and openspec config treat .specfact/code-review.json as mandatory evidence and reference the exact envelope fields (overall_verdict, findings[]). Consumers (CI/docs) will expect the report shape unchanged. Files: openspec/config.yaml, docs/modules/code-review.md. [::nold-ai/specfact-cli::openspec/config.yaml:64][::nold-ai/specfact-cli::docs/modules/code-review.md:128,146]
Implication summary
- The modules-repo changes that (a) add new ReviewFinding categories/tools, (b) change report fields/type constraints, or (c) alter registry/index.json schema (core_compatibility, checksum/signature fields) are directly consumed by the CLI repo in the places above and may require coordinated updates (Pydantic model/spec acceptance, marketplace_client parsing, module install compatibility checks, and tests that validate report/index shapes).
🔇 Additional comments (5)
packages/specfact-code-review/src/specfact_code_review/rules/commands.py (3)
24-24: Handler rename is safe with explicit Typer command binding.
_showremains correctly exposed asshowthrough@app.command("show"); no CLI surface break here.
37-43:initcommand update looks good and keeps adapter boundary clean.The private handler rename plus wrapped
--idehelp text are non-breaking and keep command intent clear.
61-66:updatecommand rename/help formatting is non-functional and correct.No behavior change to command registration or downstream contracts; this is a safe cleanup.
tests/unit/specfact_code_review/test___init__.py (2)
24-28:find_spec()does not exercise the module's__getattr__guard — issue persists.This test still uses
find_specwhich queries the import system for submodules rather than invokingspecfact_code_review.__getattr__. The lazy-export guard in__init__.py(lines 21-24 in the source) only triggers on direct attribute access. The past review comment's suggested fix usingpytest.raises(AttributeError)with direct attribute access remains the correct approach.Additionally, once this is fixed, the top-level
import importlib.util(line 5) becomes unused and should be removed.
11-21: LGTM — solid contract verification for the lazy-import adapter boundary.This properly validates the module's public API surface (
__all__) which is the contract between the bundled module and CLI core consumers.
chore(registry): publish changed modules
| return False | ||
|
|
||
| # Strip whitespace, comments, and docstrings | ||
| stripped_content = re.sub(r'"""[^"""]*"""', "", content, flags=re.DOTALL) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Promote the current
devbranch intomain. This branch includes the OpenSpec workflow updates, review-gate automation, docs alignment, and supporting validation/scripts work that landed ondevsince the last main merge.Refs:
Scope
packages/registry/index.json,packages/*/module-package.yaml).github/workflows/*)docs/*,README.md,AGENTS.md)scripts/sign-modules.py,scripts/verify-modules-signature.py)Bundle Impact
No bundle version bumps in this PR. The branch primarily updates repo workflow, docs, and review tooling.
Validation Evidence
Core repo validation and review automation were updated on
dev.Required local gates
hatch run formathatch run type-checkhatch run linthatch run yaml-linthatch run check-bundle-importshatch run contract-testhatch run smart-test(orhatch run test)Signature + version integrity (required)
hatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bumpCI and Branch Protection
verify-module-signaturesquality (3.11)quality (3.12)quality (3.13)Docs / Pages
docs/)docs-pages.yml, if changed)specfact-clidocs updated (if applicable)Checklist