fix(docs): enforce module accountability#644
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (10)
🧰 Additional context used🔀 Multi-repo context nold-ai/specfact-cli-modulesI’m unable to run repository exploration in this turn because no shell capability result was provided. The prior findings remain relevant:
📝 WalkthroughUser-visible behavior and CLI surface
Contract/API impact
Testing and quality gates
OpenSpec, documentation, and release
WalkthroughAdds a fail-closed documentation-accountability checker derived from the modules repository, wires it into local and PR validation, documents requirements and code-review module surfaces, refreshes generated command artifacts, and updates release and OpenSpec records. ChangesDocumentation accountability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant PreCommit
participant DocsReviewCI
participant AccountabilityChecker
participant ModulesRepository
Developer->>PreCommit: run documentation validation
PreCommit->>AccountabilityChecker: execute accountability check
DocsReviewCI->>AccountabilityChecker: execute required PR check
AccountabilityChecker->>ModulesRepository: load manifests and registry
ModulesRepository-->>AccountabilityChecker: official module inventory
AccountabilityChecker-->>PreCommit: pass or findings
AccountabilityChecker-->>DocsReviewCI: pass or findings
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
SpecFact CLI Validation Report✅ All validations passed! |
djm81
added
bug
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1789dce0be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
scripts/check-command-contract.py (1)
31-31: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftSingle-source the command mount inventory.
APP_MOUNTSnow duplicates the mount data inscripts/generate-command-overview.py; this PR fixes exactly the drift that allowedrequirementsto disappear from one inventory. Derive both from the same manifest/registry or add a parity test so contract validation and generated artifacts cannot diverge.As per coding guidelines, apply DRY and maintain a single source of truth for constants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-command-contract.py` at line 31, APP_MOUNTS in the command contract checker duplicates the command mount inventory used by generate-command-overview.py, allowing the two lists to drift. Create a shared manifest/registry consumed by both scripts, or add a parity test that compares their inventories, and update the requirements entry through that single source of truth.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.md`:
- Around line 140-141: Add the opt-in requirements command group to the
top-level catalogue and installed workflow-group section, documenting the
available specfact requirements subcommands and mapping them to
nold-ai/specfact-requirements. Keep the section conditional on the requirements
group being installed, consistent with the existing optional command
documentation.
In `@openspec/changes/cli-val-05-ci-integration/TDD_EVIDENCE.md`:
- Around line 76-77: The TDD evidence incorrectly declares the quality gate
complete with 64% coverage. Update the documented results only after raising
overall coverage to at least 80% and adding targeted tests and coverage evidence
that fully exercise the new fail-closed accountability path; reference the
relevant test and accountability-path symbols in the evidence and confirm the
gate passes.
In `@scripts/check-documentation-accountability.py`:
- Around line 132-140: Replace substring checks in _catalogue_findings and the
related ownership validation with semantic validation of the documented
contract. Require each official package’s explicit package ID and expected root,
and verify canonical ownership targets and handoff links rather than matching
fixed English sentences. Ensure removed-module notes, reworded claims, or
missing canonical links are reported as findings, and add coverage for these
cases.
- Around line 20-26: Extend CATALOGUE_PATHS in the documentation accountability
checker to include docs/getting-started/installation.md, ensuring the
installation overview is validated alongside the other official catalogue
documents.
- Around line 85-92: Update _manifest_inventory to detect when multiple official
manifests produce the same package_id before overwriting the existing entry;
fail closed by raising an appropriate error that identifies the duplicate
package and conflicting manifest paths, while preserving normal collection for
unique package IDs.
- Around line 115-117: Add `@beartype` to the public checker APIs, including
discover_official_modules and the APIs in the later referenced section, so
runtime type validation occurs before Path operations. Keep the existing
contracts, and ensure each decorator is applied to the appropriate public
function.
In `@scripts/pre-commit-quality-checks.sh`:
- Around line 467-473: Add scripts/check-documentation-accountability.py to the
file-pattern selector case near the existing trigger logic, so changes to the
checker set hit=1 and execute the documentation accountability gate in
pre-commit-quality-checks.sh.
In `@tests/unit/registry/test_category_groups.py`:
- Line 31: Update the assertion in the registry category-group test to require
the new “requirements” root group, rather than only permitting it in the allowed
set. Specifically, adjust the required subset checked around the existing set
comparison so “requirements” must be present, ensuring the test covers the new
mount or bootstrap behavior.
---
Nitpick comments:
In `@scripts/check-command-contract.py`:
- Line 31: APP_MOUNTS in the command contract checker duplicates the command
mount inventory used by generate-command-overview.py, allowing the two lists to
drift. Create a shared manifest/registry consumed by both scripts, or add a
parity test that compares their inventories, and update the requirements entry
through that single source of truth.
🪄 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: CHILL
Plan: Pro
Run ID: 9a79ce47-d89f-4be2-b2d4-f75b934d7560
📒 Files selected for processing (33)
.github/workflows/docs-review.ymlCHANGELOG.mdREADME.mddocs/.doc-frontmatter-enforceddocs/architecture/implementation-status.mddocs/architecture/overview.mddocs/contributing/docs-sync.mddocs/getting-started/installation.mddocs/module-system/marketplace.mddocs/reference/commands.generated.jsondocs/reference/commands.generated.mddocs/reference/commands.mddocs/reference/directory-structure.mddocs/reference/module-categories.mdllms.txtopenspec/CHANGE_ORDER.mdopenspec/changes/cli-val-05-ci-integration/CHANGE_VALIDATION.mdopenspec/changes/cli-val-05-ci-integration/TDD_EVIDENCE.mdopenspec/changes/cli-val-05-ci-integration/design.mdopenspec/changes/cli-val-05-ci-integration/proposal.mdopenspec/changes/cli-val-05-ci-integration/specs/documentation-accountability/spec.mdopenspec/changes/cli-val-05-ci-integration/tasks.mdpyproject.tomlscripts/check-command-contract.pyscripts/check-documentation-accountability.pyscripts/generate-command-overview.pyscripts/pre-commit-quality-checks.shsetup.pysrc/__init__.pysrc/specfact_cli/__init__.pytests/unit/docs/test_documentation_accountability.pytests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
nold-ai/specfact-cli-modules(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: CodeRabbit / Review
- GitHub Check: Tests (Python 3.12)
🧰 Additional context used
📓 Path-based instructions (22)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use@lru_cacheand Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...
Files:
src/__init__.pysetup.pyscripts/generate-command-overview.pysrc/specfact_cli/__init__.pyscripts/check-command-contract.pytests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.pyscripts/check-documentation-accountability.pytests/unit/docs/test_documentation_accountability.py
{src/__init__.py,pyproject.toml,setup.py}
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
{src/__init__.py,pyproject.toml,setup.py}: Update src/init.py first as primary source of truth for package version, then pyproject.toml and setup.py
Maintain version synchronization across src/init.py, pyproject.toml, and setup.py
Files:
src/__init__.pysetup.pypyproject.toml
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
src/**/*.py: All code changes must be followed by running the full test suite using the smart test system.
All Python files in src/ and tools/ directories must have corresponding test files in tests/ directory. If you modify src/common/logger_setup.py, you MUST have tests/unit/common/test_logger_setup.py. NO EXCEPTIONS - even small changes require tests.
All new Python runtime code files must have corresponding test files created BEFORE committing the code. NO EXCEPTIONS - no code without tests.
Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.
All components must support TEST_MODE=true environment variable with test-specific behavior defined as: if os.environ.get('TEST_MODE') == 'true': # test-specific behavior
Use src/common/logger_setup.py for all logging via: from common.logger_setup import get_logger; logger = get_logger(name)
Use src/common/redis_client.py with fallback for Redis operations via: from common.redis_client import get_redis_client; redis_client = get_redis_client()
Type checking must pass with no errors using: mypy .
Test coverage must meet or exceed 80% total coverage. New code must have corresponding tests. Modified code must maintain or improve coverage. Critical paths must have 100% coverage.
Use Pydantic v2 validation for all context and data schemas.Add/update contracts on new or modified public APIs, stateful classes and adapters using
icontractdecorators andbeartyperuntime type checks
src/**/*.py: Meaningful Naming — identifiers reveal intent; avoid abbreviations. Identifiers insrc/must usesnake_case(modules/functions),PascalCase(classes),UPPER_SNAKE_CASE(constants). Avoid single-letter names outside short loop variables.
KISS — keep functions and modules small and single-purpose. Maximum function length: 120 lines (Phase A error threshold). Maximum cyclomati...
Files:
src/__init__.pysrc/specfact_cli/__init__.py
@(src|tests)/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Linting must pass with no errors using: pylint src tests
Files:
src/__init__.pysrc/specfact_cli/__init__.pytests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.pytests/unit/docs/test_documentation_accountability.py
{pyproject.toml,setup.py,src/__init__.py}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Manually update version numbers in pyproject.toml, setup.py, and src/init.py when making a formal version change
Files:
src/__init__.pysetup.pypyproject.toml
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality:hatch run format, (2) Type checking:hatch run type-check(basedpyright), (3) Contract-first approach: Runhatch run contract-testfor contract validation, (4) Run full test suite:hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have@icontractdecorators and@beartypetype checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments
Files:
src/__init__.pysetup.pyscripts/generate-command-overview.pysrc/specfact_cli/__init__.pyscripts/check-command-contract.pytests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.pyscripts/check-documentation-accountability.pytests/unit/docs/test_documentation_accountability.py
**/*.{md,mdc}
📄 CodeRabbit inference engine (.cursor/rules/markdown-rules.mdc)
**/*.{md,mdc}: Do not use more than one consecutive blank line anywhere in the document (MD012: No Multiple Consecutive Blank Lines)
Fenced code blocks should be surrounded by blank lines (MD031: Fenced Code Blocks)
Lists should be surrounded by blank lines (MD032: Lists)
Files must end with a single empty line (MD047: Files Must End With Single Newline)
Lines should not have trailing spaces (MD009: No Trailing Spaces)
Use asterisks (**) for strong emphasis, not underscores (__) (MD050: Strong Style)
Fenced code blocks must have a language specified (MD040: Fenced Code Language)
Headers should increment by one level at a time (MD001: Header Increment)
Headers should be surrounded by blank lines (MD022: Headers Should Be Surrounded By Blank Lines)
Only one top-level header (H1) is allowed per document (MD025: Single H1 Header)
Use consistent list markers, preferring dashes (-) for unordered lists (MD004: List Style)
Nested unordered list items should be indented consistently, typically by 2 spaces (MD007: Unordered List Indentation)
Use exactly one space after the list marker (e.g., -, *, +, 1.) (MD030: Spaces After List Markers)
Use incrementing numbers for ordered lists (MD029: Ordered List Item Prefix)
Enclose bare URLs in angle brackets or format them as links (MD034: Bare URLs)
Don't use spaces immediately inside code spans (MD038: Spaces Inside Code Spans)
Use consistent indentation (usually 2 or 4 spaces) throughout markdown files
Keep line length under 120 characters in markdown files
Use reference-style links for better readability in markdown files
Use a trailing slash for directory paths in markdown files
Ensure proper escaping of special characters in markdown files
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/cli-val-05-ci-integration/CHANGE_VALIDATION.mddocs/contributing/docs-sync.mddocs/module-system/marketplace.mdopenspec/changes/cli-val-05-ci-integration/specs/documentation-accountability/spec.mdCHANGELOG.mddocs/architecture/implementation-status.mddocs/reference/commands.mdREADME.mddocs/reference/directory-structure.mddocs/architecture/overview.mddocs/reference/commands.generated.mddocs/getting-started/installation.mdopenspec/changes/cli-val-05-ci-integration/proposal.mddocs/reference/module-categories.mdopenspec/changes/cli-val-05-ci-integration/TDD_EVIDENCE.mdopenspec/changes/cli-val-05-ci-integration/tasks.mdopenspec/changes/cli-val-05-ci-integration/design.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/cli-val-05-ci-integration/CHANGE_VALIDATION.mddocs/contributing/docs-sync.mddocs/module-system/marketplace.mdopenspec/changes/cli-val-05-ci-integration/specs/documentation-accountability/spec.mdCHANGELOG.mddocs/architecture/implementation-status.mddocs/reference/commands.mdREADME.mddocs/reference/directory-structure.mddocs/architecture/overview.mddocs/reference/commands.generated.mddocs/getting-started/installation.mdopenspec/changes/cli-val-05-ci-integration/proposal.mddocs/reference/module-categories.mdopenspec/changes/cli-val-05-ci-integration/TDD_EVIDENCE.mdopenspec/changes/cli-val-05-ci-integration/tasks.mdopenspec/changes/cli-val-05-ci-integration/design.md
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Treat as specification source of truth: proposal/tasks/spec deltas vs. code behavior,
CHANGE_ORDER consistency, and scenario coverage. Surface drift between OpenSpec and
implementation.
Files:
openspec/CHANGE_ORDER.mdopenspec/changes/cli-val-05-ci-integration/CHANGE_VALIDATION.mdopenspec/changes/cli-val-05-ci-integration/specs/documentation-accountability/spec.mdopenspec/changes/cli-val-05-ci-integration/proposal.mdopenspec/changes/cli-val-05-ci-integration/TDD_EVIDENCE.mdopenspec/changes/cli-val-05-ci-integration/tasks.mdopenspec/changes/cli-val-05-ci-integration/design.md
openspec/changes/**/*.md
📄 CodeRabbit inference engine (.cursorrules)
For
/opsx:archive(Archive change): Include module signing and cleanup in final tasks. Agents MUST runopenspec archive <change-id>from repo root (no manualmvunderopenspec/changes/archive/)
Files:
openspec/changes/cli-val-05-ci-integration/CHANGE_VALIDATION.mdopenspec/changes/cli-val-05-ci-integration/specs/documentation-accountability/spec.mdopenspec/changes/cli-val-05-ci-integration/proposal.mdopenspec/changes/cli-val-05-ci-integration/TDD_EVIDENCE.mdopenspec/changes/cli-val-05-ci-integration/tasks.mdopenspec/changes/cli-val-05-ci-integration/design.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.
Files:
docs/contributing/docs-sync.mddocs/module-system/marketplace.mddocs/architecture/implementation-status.mddocs/reference/commands.mddocs/reference/directory-structure.mddocs/architecture/overview.mddocs/reference/commands.generated.mddocs/getting-started/installation.mddocs/reference/module-categories.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing accuracy: CLI examples match current behavior; preserve Jekyll front matter;
call out when README/docs index need sync.
Files:
docs/contributing/docs-sync.mddocs/module-system/marketplace.mddocs/architecture/implementation-status.mddocs/reference/commands.mddocs/reference/directory-structure.mddocs/architecture/overview.mddocs/reference/commands.generated.mddocs/getting-started/installation.mddocs/reference/module-categories.md
openspec/**/{proposal.md,tasks.md,design.md,spec.md}
📄 CodeRabbit inference engine (.cursor/rules/automatic-openspec-workflow.mdc)
openspec/**/{proposal.md,tasks.md,design.md,spec.md}: Applyopenspec/config.yamlproject context and per-artifact rules (for proposal, specs, design, tasks) when creating or updating any OpenSpec change artifact in the specfact-cli codebase
After implementation, validate the change withopenspec validate <change-id> --strict; fix validation errors in proposal, specs, design, or tasks and re-validate until passing before considering the change complete
Files:
openspec/changes/cli-val-05-ci-integration/specs/documentation-accountability/spec.mdopenspec/changes/cli-val-05-ci-integration/proposal.mdopenspec/changes/cli-val-05-ci-integration/tasks.mdopenspec/changes/cli-val-05-ci-integration/design.md
CHANGELOG.md
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
Include new version entries at the top of CHANGELOG.md when updating versions
Update CHANGELOG.md with all code changes as part of version control requirements.
Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change
Files:
CHANGELOG.md
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Validate GitHub workflow files using
hatch run lint-workflowsbefore committing
.github/workflows/*.{yml,yaml}: Use actionlint for semantic validation of GitHub Actions workflows
Format GitHub Actions workflows usinghatch run workflows-fmtand lint them withhatch run workflows-lintafter editing
Files:
.github/workflows/docs-review.yml
.github/workflows/!(tests).{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Do not re-run the full test suite in other CI workflows; tests are enforced only in the dedicated Tests workflow (.github/workflows/tests.yml)
Files:
.github/workflows/docs-review.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: CI safety: secrets usage, workflow dependencies, alignment with hatch test / contract-test
gates, and action versions.
Files:
.github/workflows/docs-review.yml
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: subprocess safety, Hatch integration, and parity with documented
quality gates (format, type-check, module signing).
Files:
scripts/generate-command-overview.pyscripts/check-command-contract.pyscripts/check-documentation-accountability.py
@(README.md|AGENTS.md)
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Check README.md and AGENTS.md for current project status and development guidelines. Review .cursor/rules/ for detailed development standards and testing procedures.
Files:
README.md
src/specfact_cli/**/*.py
⚙️ CodeRabbit configuration file
src/specfact_cli/**/*.py: Focus on modular CLI architecture: lazy module loading, registry/bootstrap patterns, and
dependency direction. Flag breaking changes to public APIs, Pydantic models, and resource
bundling. Verify@icontract+@beartypeon public surfaces; prefer centralized logging
(get_bridge_logger) over print().
Files:
src/specfact_cli/__init__.py
pyproject.toml
📄 CodeRabbit inference engine (.cursorrules)
When updating the version in
pyproject.toml, ensure it's newer than the latest PyPI version. The CI/CD pipeline will automatically publish to PyPI only if the new version is greater than the published version
Files:
pyproject.toml
**/test_*.py
📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)
**/test_*.py: Write tests first in test-driven development (TDD) using the Red-Green-Refactor cycle
Ensure each test is independent and repeatable with no shared state between tests
Organize Python imports in tests using unittest.mock for Mock and patch
Use setup_method() for test initialization and Arrange-Act-Assert pattern in test files
Use@pytest.mark.asynciodecorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module
Files:
tests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.pytests/unit/docs/test_documentation_accountability.py
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
Tests must be meaningful and test actual functionality, cover both success and failure cases, be independent and repeatable, and have clear, descriptive names. NO EXCEPTIONS - no placeholder or empty tests.
tests/**/*.py: Trim low-value unit tests when a contract covers the same assertion (type/shape/raises on negative checks)
Delete tests that only assert input validation, datatype/shape enforcement, or raises on negative conditions now guarded by contracts and runtime typing
Convert repeated edge-case permutations into one Hypothesis property with contracts acting as oraclesSecret redaction via
LoggerSetup.redact_secretsmust be covered by unit tests
Files:
tests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.pytests/unit/docs/test_documentation_accountability.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.
Files:
tests/unit/docs/test_llms_overview_freshness.pytests/unit/registry/test_category_groups.pytests/unit/docs/test_documentation_accountability.py
🪛 ast-grep (0.44.1)
tests/unit/docs/test_documentation_accountability.py
[info] 100-103: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
[record for record in records if record.get("owner_package") != "nold-ai/specfact-requirements"],
indent=2,
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 LanguageTool
openspec/changes/cli-val-05-ci-integration/proposal.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: ...lection, and upgrade/version logic. - NEW: Fail-closed documentation-accountabi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (28)
CHANGELOG.md (1)
17-18: 🔒 Security & PrivacyVerify that manifest signatures are actually enforced.
The supplied checker proves manifest/registry agreement, but the shown implementation does not establish signature verification. If signatures are not validated elsewhere, this release note overstates the security guarantee.
openspec/CHANGE_ORDER.md (1)
99-99: LGTM!setup.py (1)
10-10: LGTM!src/__init__.py (1)
6-6: LGTM!src/specfact_cli/__init__.py (1)
79-81: LGTM!scripts/check-command-contract.py (1)
249-252: LGTM!scripts/generate-command-overview.py (1)
35-40: LGTM!Also applies to: 366-370
docs/reference/commands.generated.json (2)
26-35: LGTM!
2083-2084: 🗄️ Data Integrity & IntegrationConfirm the
owner_repocontract for the requirements records.Each new record identifies
nold-ai/specfact-requirementsas the package owner butnold-ai/specfact-cliasowner_repo, while the generator mount also namesnold-ai/specfact-requirements. Ifowner_repodenotes the owning repository, this misattributes all five commands and can break accountability checks or ownership reporting. Verify the generator's field semantics and regenerate the artifact from source.Also applies to: 2105-2106, 2123-2124, 2141-2142, 2159-2160
docs/reference/commands.generated.md (1)
15-15: LGTM!Also applies to: 116-120
llms.txt (1)
17-17: LGTM!Also applies to: 118-122
tests/unit/registry/test_category_groups.py (1)
19-19: LGTM!Also applies to: 119-124
tests/unit/docs/test_llms_overview_freshness.py (2)
11-11: LGTM!Also applies to: 29-38, 51-56, 57-71
73-80: 🎯 Functional CorrectnessKeep the skip for local runs.
.github/workflows/docs-review.ymlalready checks outnold-ai/specfact-cli-modules, exportsSPECFACT_MODULES_REPO, and runs this suite, so CI is not bypassing the freshness gate here. The skip only keeps standalone localpytestruns usable when the companion repo is absent.> Likely an incorrect or invalid review comment.README.md (1)
166-166: LGTM!docs/.doc-frontmatter-enforced (1)
9-15: LGTM!docs/architecture/implementation-status.md (1)
9-13: LGTM!Also applies to: 31-31
docs/architecture/overview.md (1)
10-14: LGTM!Also applies to: 24-24
docs/reference/module-categories.md (1)
6-10: LGTM!Also applies to: 50-50, 64-64, 74-81, 117-120
docs/contributing/docs-sync.md (2)
13-13: LGTM!
53-58: 🩺 Stability & AvailabilityNo issue: the modules repo is provisioned before the accountability check. The workflow checks out
nold-ai/specfact-cli-modulesand exportsSPECFACT_MODULES_REPObefore runninghatch run check-documentation-accountability, so this gate is already covered.> Likely an incorrect or invalid review comment.docs/getting-started/installation.md (1)
8-12: LGTM!Also applies to: 218-218, 234-238
docs/module-system/marketplace.md (1)
12-16: LGTM!Also applies to: 36-36, 47-47
docs/reference/commands.md (1)
8-12: LGTM!docs/reference/directory-structure.md (1)
7-11: LGTM!Also applies to: 82-85
pyproject.toml (1)
7-7: LGTM!Also applies to: 259-265
.github/workflows/docs-review.yml (1)
24-24: LGTM!Also applies to: 53-53, 126-128
openspec/changes/cli-val-05-ci-integration/CHANGE_VALIDATION.md (1)
38-42: 📐 Maintainability & Code QualityNo issue here
The wiki note is scoped to the sibling internal checkout and matches the documented
wikiupdate task; it does not make the strict OpenSpec PASS incomplete.> Likely an incorrect or invalid review comment.
SpecFact CLI Validation Report✅ All validations passed! |
SpecFact CLI Validation Report✅ All validations passed! |
SpecFact CLI Validation Report✅ All validations passed! |
1 participant
Summary
Root cause
The command overview and contract used a stale hard-coded module inventory, so the requirements module was omitted while existing documentation checks still passed.
Validation
Follow-up
The internal wiki source is already modified outside this worktree; reconcile the public change scope there and rebuild its graph before merge.