Harden CLI command reliability gates#595
Conversation
|
Strix is installed on this repository, but we could not run this PR security review because this workspace does not have an active plan. If you'd like to continue receiving code reviews, you can add a payment method or manage billing here. |
📝 WalkthroughPR
|
| Layer / File(s) | Summary |
|---|---|
Generated command overview & contract checks scripts/generate-command-overview.py, scripts/check-command-contract.py, docs/reference/commands.generated.*, llms.txt |
Deterministic CLI introspection producing JSON/MD/llms artifacts and a contract checker that validates live Typer apps (--help, missing-subcommand, missing-argument shapes`). |
Docs & examples updates docs/**, README.md, CHANGELOG.md, resources/templates/pr-template.md.j2, .markdownlint.json |
Mass updates to examples and docs to use new command shapes (notably specfact code import --repo . <bundle>), added generated command overview links and artifacts. |
Docs command validation scripts/check-docs-commands.py, tests/unit/docs/* |
Guidance/template scanning extended to non-Markdown assets, flag-preserving tokenization, code import ordering checks, prefix-matching against generated command contract, and tests. |
CLI runtime & progressive-disclosure src/specfact_cli/cli.py, src/specfact_cli/utils/progressive_disclosure.py, src/specfact_cli/__init__.py |
Lazy-delegate refactor centralizing real-command invocation, Typer context-param detection wrapper, broadened Click/Typer monkey-patches, and dynamic progressive-disclosure import during package init. |
IDE exports: skill.md src/specfact_cli/utils/ide_setup.py, tests/unit/utils/test_ide_setup.py, tests/unit/modules/init/* |
Added grouped SKILL.md export format, deterministic ordering, stale-prune behavior, and Codex/skill export tests. |
Upgrade/runtime detection & launcher repair src/specfact_cli/modules/upgrade/src/commands.py, src/specfact_cli/utils/env_manager.py, tests tests/unit/commands/test_update.py, tests/unit/utils/test_env_manager.py |
Detect uv run contexts, probe tools in active env via subprocess, verify and optionally pipx reinstall repaired launchers, with tests exercising repair/failure branches. |
Runtime discovery smoke & worktrees scripts/runtime_discovery_smoke.py, tests/integration/scripts/test_runtime_discovery_smoke.py, tests/conftest.py |
Added hatch-source launcher mode, child PYTHONPATH for hatch-source, and worktree-aware modules repo resolution with updated tests. |
CI/workflows & pre-commit gates .github/workflows/*.yml, scripts/pre-commit-quality-checks.sh, tests/unit/workflows/* |
Workflows now resolve/checkout paired specfact-cli-modules ref, run command-overview/contract/docs checks, adjust contract-test scoping, and pre-commit regenerates/validates command artifacts when docs change. |
Version, packaging & small updates pyproject.toml, setup.py, src/__init__.py, module-package.yaml, suggestions/annotations/structure |
Bumped version to 0.47.2, added Hatch scripts for generation/checks, adjusted suggestion strings, PR comment hints, and migration/help strings. |
Tests: additions and updates tests/unit/cli/*, many unit/integration tests |
Added CLI error-guidance and lazy-delegate help tests, extended docs validation tests, IDE export tests, pipx/upgrade tests, workflow assertions, and helper refactors. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
- [User Story] Harden CLI command guidance, command inventory, and package-manager runtime checks #594 — Broad tester CLI reliability workstream overlaps with generator/contract validation and CI gating here.
- Docs outdated: core-cli modes page shows old
code importsyntax (--repono longer valid) #588 — Docs examples updated tospecfact code import --repo . <bundle>address the ordering problem reported there. - [User Story] Fix tester-reported module CLI command contracts specfact-cli-modules#306 — Paired modules checkout and worktree handling relate to cross-repo module pairing concerns.
Possibly related PRs
- nold-ai/specfact-cli#547 — Overlaps on module-package manifest updates.
- Make
specfact upgradeinstall-method-aware (uv/uvx support, pipx/pip detection) #539 — Related upgrade/runtime install-method detection changes. - Release dev to main: lazy subcommand delegation fix #550 — Related lazy delegation refactor and tests.
Note: Focus review on contract-generator correctness, validator edge cases (prefix matching and ordering), Typer/Click monkey-patch safety, pipx repair side-effects, CI paired-repo checkout security, and IDE export pruning data-loss risks.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/tester-command-reliability
djm81
added
bug
![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: 5297ea5111
ℹ️ 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: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/check-docs-commands.py (1)
115-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
code importoption-order checks are currently bypassed for real scanned text.The parser stops at the first flag, so scanned commands like
specfact code import legacy-api --repo .never reach_invalid_code_import_order_messagewith--repopresent. That means invalid ordering can slip through docs/guidance validation.Suggested fix
def _tokens_from_specfact_line(line: str) -> list[str] | None: @@ - out: list[str] = [] - for part in parts: - if part.startswith("-"): - break - out.append(part) - return out if out else None + return parts if parts else Nonedef validate_command_tokens(tokens: list[str]) -> tuple[bool, str]: @@ - if _generated_prefix_match(tokens): + command_prefix: list[str] = [] + for token in tokens: + if token.startswith("-"): + break + command_prefix.append(token) + if _generated_prefix_match(command_prefix): return True, "" @@ - for k in range(len(tokens), 0, -1): - prefix = tokens[:k] + for k in range(len(command_prefix), 0, -1): + prefix = command_prefix[:k] ok, msg = _eval_prefix_help(runner, prefix)As per coding guidelines, validators “must fail … on invalid documented option placement (e.g., rejected ordering for
specfact code import <bundle> --repo .).”Also applies to: 267-315
🤖 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-docs-commands.py` around lines 115 - 138, _tokens_from_specfact_line currently stops collecting tokens at the first flag which prevents later validation (e.g., in _invalid_code_import_order_message) from seeing options like --repo; change the logic in _tokens_from_specfact_line (and the similar block around lines 267-315) to collect all parts after stripping global options (use _strip_leading_global_options(parts)) instead of breaking on part.startswith("-"), so flags are preserved in the returned list and downstream validators (such as _invalid_code_import_order_message and the code import ordering checks) can enforce option placement; keep the early returns for non-specfact lines and failed shlex parsing unchanged.openspec/changes/tester-cli-reliability/tasks.md (1)
35-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate checklist status to match executed gates and review evidence.
Lines 38–39 are still unchecked even though this PR’s recorded evidence says those gates and code review were executed. This creates OpenSpec traceability drift for completion status.
As per coding guidelines, “After implementation, validate the change with
openspec validate <change-id> --strict… before considering the change complete.”🤖 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 `@openspec/changes/tester-cli-reliability/tasks.md` around lines 35 - 40, Update the checklist in tasks.md so the status reflects the executed gates and review evidence: mark 6.1, 6.2 and 6.3 as completed if TDD_EVIDENCE.md and the PR show the re-run targeted tests, required quality gates (format, type-check, lint, YAML lint, contract-test/smart-test or targeted equivalent) and SpecFact code review were performed or document explicit exceptions for any unresolved findings; then run openspec validate <change-id> --strict to confirm traceability and only commit the checklist update after validation passes.
🟡 Minor comments (3)
src/specfact_cli/utils/structure.py-978-987 (1)
978-987:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix mixed
code importsyntax in generated README snippet.This block still contains the legacy ordering (
specfact code import <bundle-name> --repo .) while adjacent examples were migrated. Please keep the snippet fully canonical to avoid onboarding regressions in scaffolded repos.Suggested patch
# Analyze existing code -specfact code import <bundle-name> --repo . +specfact code import --repo . <bundle-name>🤖 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 `@src/specfact_cli/utils/structure.py` around lines 978 - 987, The README snippet contains legacy ordering for the code import command; update the example to use the canonical flag order used elsewhere. Replace the line containing "specfact code import <bundle-name> --repo ." with the canonical form used in adjacent examples (i.e., put the --repo flag before the positional bundle name or follow the project's canonical CLI ordering), ensuring consistency with other examples like "specfact project plan init --interactive" and "specfact project plan compare ..."; update the snippet in the generator function/schema that emits this block (refer to the snippet around the call that constructs the README in structure.py and the text containing "specfact code import") so all scaffolded repos show the canonical syntax.llms.txt-1-13 (1)
1-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPlace front matter at the top and keep a single H1 in
llms.txt.This file currently has content before front matter and two H1 headers. If this artifact is rendered by Jekyll/docs tooling, metadata may not be parsed correctly and heading validation can fail.
As per coding guidelines, “Headers should increment by one level at a time (MD001: Header Increment)” and “Only one top-level header (H1) is allowed per document (MD025: Single H1 Header).”
🤖 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 `@llms.txt` around lines 1 - 13, Move the Jekyll front matter block (the lines between the leading --- markers) to the very top of llms.txt so it appears before any other content, then ensure only a single H1 remains by removing or converting the extra "# Generated SpecFact CLI Command Overview" header to a lower-level header (e.g., H2) or deleting it; update the remaining top-level header so there is exactly one H1 in the file and headings increment by one level and follow MD001/MD025 guidelines.docs/reference/directory-structure.md-414-414 (1)
414-414:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
code importsyntax examples consistent in this page.Line 414 uses the new canonical ordering, but the earlier syntax snippet still shows the old positional-first form. Please align both to one contract form to avoid stale-command drift in docs checks.
As per coding guidelines, “docs/prompt/template/guidance examples [must] be checked against the generated command overview contract … and invalid documented option placement must fail validation.”
🤖 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 `@docs/reference/directory-structure.md` at line 414, The docs contain inconsistent examples for the "specfact code import" command: update the earlier snippet(s) to match the canonical option ordering used on line 414 (e.g., "specfact code import --repo . legacy-api --confidence 0.7"); locate other occurrences of the old positional-first form related to the "specfact code import" example and reorder arguments so flags (--repo, --confidence) appear in the canonical positions and the positional target (legacy-api) stays in the same place as in the line 414 example.
🤖 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 @.github/workflows/docs-review.yml:
- Around line 90-98: The secondary checkout step "Checkout module command
sources" that checks out repository nold-ai/specfact-cli-modules is using a
floating actions/checkout ref and leaving credentials in the cloned workspace;
update that step to use a pinned actions/checkout commit SHA (pin the action
version to a specific commit) and add persist-credentials: false to the checkout
step so the GITHUB_TOKEN is not persisted into specfact-cli-modules; keep the
same repository, path, and ref usage for the module content (or replace ref
usage with the intended module tag/commit if needed) and leave the "Export
module command source path" step unchanged.
In @.github/workflows/pr-orchestrator.yml:
- Around line 234-240: The checkout steps that pull repository
nold-ai/specfact-cli-modules (the actions/checkout@v4 blocks such as the step
named "Checkout module bundles repo" and the three other identical module
checkouts) are not hardened: pin each actions/checkout invocation to a specific
commit SHA instead of a floating ref and add with: persist-credentials: false to
each block so the checked-out code cannot use inherited repository credentials;
update all four checkout blocks (the ones using repository:
nold-ai/specfact-cli-modules) to use a fixed commit SHA for the action and
include persist-credentials: false in their with: section.
In @.github/workflows/specfact.yml:
- Around line 86-87: The workflow step currently masks failures by appending "||
true" and then recording "$?" which always yields 0; remove the "|| true" so the
real exit status of the "specfact code repro" command is preserved, and ensure
the echo that writes the step output (echo "exit_code=$?" >> "$GITHUB_OUTPUT")
runs immediately after that command so it captures the actual exit code; also
defensively quote/validate the budget interpolation by passing the budget as a
quoted argument (use the workflow expression form for the budget that the step
already references, e.g., wrap steps.validation.outputs.budget in quotes) to
avoid word-splitting or injection when invoking "specfact code repro".
In @.markdownlint.json:
- Around line 12-14: Restore enforcement for the MD040 rule by removing or
changing the "MD040": false entry in the .markdownlint.json configuration so
fenced code blocks must include a language; locate the "MD040" key in the JSON
(alongside "MD036" and "MD051") and either delete that property or set it to
true (or remove the rule from the disabled list) to re-enable the repository's
fenced-code-language requirement.
In `@docs/examples/integration-showcases/integration-showcases-testing-guide.md`:
- Around line 1159-1162: Replace the two occurrences of the CLI invocation
"specfact --no-banner project version check" with the correct
active-baseline/plan selection and verification subcommands so the example
actually sets and then verifies the active plan used for later drift comparison;
ensure the first call (previously passing --bundle "$BUNDLE_NAME") uses the
subcommand that sets the active plan for the project from the given bundle, and
the second call (previously passing --repo .) uses the subcommand that
shows/verifies the currently active plan in the repo, preserving the same flags
(--bundle and --repo) and surrounding context so the pre-commit example compares
against the intended baseline.
In `@docs/migration/migration-cli-reorganization.md`:
- Around line 140-141: The CLI examples use an invalid flat form "specfact code
import --repo . legacy-api" but the PR defines "code import" as a command group
with subcommands "from-bridge" and "from-code"; update each example to call the
correct subcommand (e.g., replace the invalid "specfact code import --repo .
legacy-api" with "specfact code import from-code --repo . legacy-api" or the
appropriate "from-bridge" variant) and apply the same fix to the other
occurrences noted (around the later example lines). Ensure examples match the
new command contract (use "specfact code import from-code" or "specfact code
import from-bridge" as appropriate) so copy-paste will work.
In `@docs/reference/commands.generated.json`:
- Around line 645-651: The "specfact code" command's subcommands array is
missing the "review" entry which breaks the command tree consistency; update the
"subcommands" array for the "specfact code" command to include "review" so it
matches the later definition of "specfact code review" (apply the same fix where
the other occurrence appears around the other block), ensuring the "subcommands"
arrays for "specfact code" list
["analyze","drift","import","repro","validate","review"] to restore the
contract/docs tree integrity.
In `@docs/reference/commands.generated.md`:
- Line 43: The generated command overview for the `specfact code` command is
missing the `review` subcommand and therefore produces an inconsistent
parent-child tree; update the generator or the data used to render the table so
that the `specfact code` row lists all subcommands including `review` (and
ensure the subsequent rows that render `specfact code review ...` are reflected
in the parent row), and re-generate the docs so the `specfact code` entry and
the rows covering the same command (lines around the block containing `specfact
code review`) are consistent.
In `@openspec/CHANGE_ORDER.md`:
- Line 81: The CHANGE_ORDER.md row for version "0.5" (entry with
`tester-cli-reliability` and paired modules `tester-module-cli-reliability`) is
missing source bug `#593`; update that row's source bugs list to include `#593`
alongside the existing `#585,`#587`,`#588`,`#589`,`#590`` so the authoritative
cross-change plan maps all core-owned bugs consistently with the
`tester-cli-reliability` evidence.
In `@openspec/changes/tester-cli-reliability/proposal.md`:
- Around line 42-43: Update the "Source Bugs" bullet list in the proposal's
header to include issue `#593` so the proposal's source bug map matches the change
scope; specifically, edit the "Source Bugs" list (the bullet starting with
"**Source Bugs**") to add ",
[`#593`](https://github.com/nold-ai/specfact-cli/issues/593)" so the proposal
tracks that core reliability bug alongside the other referenced issues.
In `@src/specfact_cli/__init__.py`:
- Around line 65-67: The bootstrap currently inserts
_specfact_progressive_disclosure_bootstrap into sys.modules before calling
spec.loader.exec_module(module), which can leave a partially-initialized module
cached if exec_module raises; wrap the call to spec.loader.exec_module(module)
inside a try/except (or try/finally) in _install_progressive_disclosure(), and
on exception remove the entry via sys.modules.pop(module_name, None) before
re-raising so subsequent attempts can retry installation.
In `@src/specfact_cli/cli.py`:
- Around line 732-735: The except block in _invoke_real_command currently
normalizes any "*Exit" by using getattr(exc, "exit_code", 0), which loses plain
SystemExit.code (e.g., SystemExit(2) becomes 0); change the logic so that if exc
is an instance of SystemExit you re-raise SystemExit with exc.code preserved,
otherwise for other delegated "*Exit" exceptions use getattr(exc, "exit_code",
getattr(exc, "code", 0)) to derive the exit code; update the exception handling
in _invoke_real_command to prefer exc.code for SystemExit and fall back to
exit_code for custom Exit classes.
In `@src/specfact_cli/modules/init/src/commands.py`:
- Around line 714-718: The public Typer callback init is missing the `@beartype`
decorator; add `@beartype` above the function (consistent with other public
commands like init_ide) so the signature is runtime type-checked, keeping the
existing `@app.callback`, `@require`, and `@ensure` decorators in place; ensure
beartype is imported and apply it to the init function definition.
In `@src/specfact_cli/modules/upgrade/src/commands.py`:
- Around line 293-323: The code currently treats missing pipx launcher (when
shutil.which("specfact") returns falsy) as a successful upgrade and returns
True; change that so a missing launcher is handled like a repairable failure: do
not return True on missing launcher, print the existing warning, then proceed to
run _run_pipx_reinstall(), replay its stdout/stderr with
_replay_upgrade_output/_coerce_subprocess_output, check reinstall.returncode
(print the same failure message and return False on non-zero), then re-check the
launcher by calling shutil.which("specfact") again and/or calling
_run_launcher_version_check(launcher) like the stale-path branch, replay its
outputs and return success only if the post-reinstall check.returncode == 0
(otherwise print the existing failure message and return False); update or
remove the early return in the missing-launcher branch so the flow matches the
stale/broken launcher repair flow.
In `@src/specfact_cli/utils/suggestions.py`:
- Around line 93-94: The suggestion "specfact project plan select" is invalid;
update the suggestions list by removing or replacing the line added via
suggestions.append("specfact project plan select # Select an active plan
bundle") with a valid command from the current command contract (e.g., a working
plan-list or plan-select invocation used elsewhere), e.g. modify the
suggestions.append call in src/specfact_cli/utils/suggestions.py to use the
correct command name used in the codebase (verify against the command contract
or existing commands like any "plan list" or "plan select" variants) so the
recovery suggestion is not a dead-end.
In `@tests/unit/cli/test_error_guidance.py`:
- Around line 20-24: The assertions in test_error_guidance.py are brittle
because they check raw output that may include ANSI escape sequences; before
asserting on the variable output, normalize it by stripping ANSI sequences
(e.g., with a helper like strip_ansi or using a standard utility such as
click.utils.strip_ansi or a small regex) and then run the existing contains
checks against the cleaned string (references: test variable output and the
assertion lines checking "usage: specfact", "hello", "not a valid command"/"no
such command", "try", and "specfact --help"/"specfact -h").
- Around line 92-99: The misuse-matrix in tests/unit/cli/test_error_guidance.py
includes entries like ("code missing subcommand", ["code"]) and other
module-backed commands which can produce “module not installed” guidance and
make tests flaky; restrict the matrix to guaranteed-core paths (remove or
replace "code", "backlog", "project sync bridge", etc.) or explicitly control
module availability in the test setup by mocking the CLI's module-detection
function (e.g., the function that checks whether a plugin/module is installed,
or an import check used by the CLI) using a fixture/monkeypatch so those entries
always produce the expected usage guidance instead of installation errors.
Ensure the same change is applied to the similar list referenced later in the
file (the other matrix at the end).
In `@tests/unit/cli/test_lean_help_output.py`:
- Around line 151-169: Normalize result.output by stripping ANSI color/styling
before asserting the usage banner and related substrings: in
test_lazy_delegate_bare_group_shows_full_help_and_missing_subcommand and
test_lazy_delegate_missing_option_value_shows_leaf_help, replace direct checks
against result.output with checks against a cleaned variable (e.g., cleaned =
strip_ansi(result.output) or an equivalent helper) and use cleaned for the
"Usage: specfact ..." and other substring assertions so the tests match
semantics rather than terminal styling while keeping the existing exit_code and
message checks.
---
Outside diff comments:
In `@openspec/changes/tester-cli-reliability/tasks.md`:
- Around line 35-40: Update the checklist in tasks.md so the status reflects the
executed gates and review evidence: mark 6.1, 6.2 and 6.3 as completed if
TDD_EVIDENCE.md and the PR show the re-run targeted tests, required quality
gates (format, type-check, lint, YAML lint, contract-test/smart-test or targeted
equivalent) and SpecFact code review were performed or document explicit
exceptions for any unresolved findings; then run openspec validate <change-id>
--strict to confirm traceability and only commit the checklist update after
validation passes.
In `@scripts/check-docs-commands.py`:
- Around line 115-138: _tokens_from_specfact_line currently stops collecting
tokens at the first flag which prevents later validation (e.g., in
_invalid_code_import_order_message) from seeing options like --repo; change the
logic in _tokens_from_specfact_line (and the similar block around lines 267-315)
to collect all parts after stripping global options (use
_strip_leading_global_options(parts)) instead of breaking on
part.startswith("-"), so flags are preserved in the returned list and downstream
validators (such as _invalid_code_import_order_message and the code import
ordering checks) can enforce option placement; keep the early returns for
non-specfact lines and failed shlex parsing unchanged.
---
Minor comments:
In `@docs/reference/directory-structure.md`:
- Line 414: The docs contain inconsistent examples for the "specfact code
import" command: update the earlier snippet(s) to match the canonical option
ordering used on line 414 (e.g., "specfact code import --repo . legacy-api
--confidence 0.7"); locate other occurrences of the old positional-first form
related to the "specfact code import" example and reorder arguments so flags
(--repo, --confidence) appear in the canonical positions and the positional
target (legacy-api) stays in the same place as in the line 414 example.
In `@llms.txt`:
- Around line 1-13: Move the Jekyll front matter block (the lines between the
leading --- markers) to the very top of llms.txt so it appears before any other
content, then ensure only a single H1 remains by removing or converting the
extra "# Generated SpecFact CLI Command Overview" header to a lower-level header
(e.g., H2) or deleting it; update the remaining top-level header so there is
exactly one H1 in the file and headings increment by one level and follow
MD001/MD025 guidelines.
In `@src/specfact_cli/utils/structure.py`:
- Around line 978-987: The README snippet contains legacy ordering for the code
import command; update the example to use the canonical flag order used
elsewhere. Replace the line containing "specfact code import <bundle-name>
--repo ." with the canonical form used in adjacent examples (i.e., put the
--repo flag before the positional bundle name or follow the project's canonical
CLI ordering), ensuring consistency with other examples like "specfact project
plan init --interactive" and "specfact project plan compare ..."; update the
snippet in the generator function/schema that emits this block (refer to the
snippet around the call that constructs the README in structure.py and the text
containing "specfact code import") so all scaffolded repos show the canonical
syntax.
🪄 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: 324aa60f-e7e6-479c-adef-06e7b67e9597
📒 Files selected for processing (85)
.github/ISSUE_TEMPLATE/augmentation_metadata.md.github/ISSUE_TEMPLATE/bug_report.md.github/pull_request_template.md.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.yml.github/workflows/specfact.yml.markdownlint.jsonCHANGELOG.mdREADME.mddocs/core-cli/debug-logging.mddocs/core-cli/init.mddocs/core-cli/modes.mddocs/examples/dogfooding-specfact-cli.mddocs/examples/integration-showcases/integration-showcases-quick-reference.mddocs/examples/integration-showcases/integration-showcases-testing-guide.mddocs/examples/integration-showcases/integration-showcases.mddocs/examples/integration-showcases/setup-integration-tests.shdocs/examples/quick-examples.mddocs/getting-started/README.mddocs/getting-started/installation.mddocs/getting-started/quickstart.mddocs/getting-started/tutorial-openspec-speckit.mddocs/getting-started/where-to-start.mddocs/guides/ai-ide-workflow.mddocs/guides/command-chains.mddocs/guides/copilot-mode.mddocs/guides/ide-integration.mddocs/guides/openspec-journey.mddocs/guides/speckit-journey.mddocs/guides/troubleshooting.mddocs/migration/migration-cli-reorganization.mddocs/reference/commands.generated.jsondocs/reference/commands.generated.mddocs/reference/commands.mddocs/reference/directory-structure.mddocs/technical/code2spec-analysis-logic.mdllms.txtopenspec/CHANGE_ORDER.mdopenspec/changes/tester-cli-reliability/.openspec.yamlopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.mdopenspec/changes/tester-cli-reliability/proposal.mdopenspec/changes/tester-cli-reliability/specs/ci-integration/spec.mdopenspec/changes/tester-cli-reliability/specs/cli-error-guidance/spec.mdopenspec/changes/tester-cli-reliability/specs/command-package-runtime-validation/spec.mdopenspec/changes/tester-cli-reliability/specs/core-cli-reference/spec.mdopenspec/changes/tester-cli-reliability/specs/generated-command-overview/spec.mdopenspec/changes/tester-cli-reliability/specs/runtime-tool-probing/spec.mdopenspec/changes/tester-cli-reliability/tasks.mdpyproject.tomlresources/templates/pr-template.md.j2scripts/check-command-contract.pyscripts/check-docs-commands.pyscripts/generate-command-overview.pyscripts/pre-commit-quality-checks.shscripts/runtime_discovery_smoke.pysetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/init/module-package.yamlsrc/specfact_cli/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/module-package.yamlsrc/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/utils/bundle_loader.pysrc/specfact_cli/utils/env_manager.pysrc/specfact_cli/utils/github_annotations.pysrc/specfact_cli/utils/ide_setup.pysrc/specfact_cli/utils/progressive_disclosure.pysrc/specfact_cli/utils/structure.pysrc/specfact_cli/utils/suggestions.pytests/conftest.pytests/integration/scripts/test_runtime_discovery_smoke.pytests/unit/cli/test_error_guidance.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_backlog_daily.pytests/unit/commands/test_update.pytests/unit/docs/test_docs_validation_scripts.pytests/unit/migration/test_module_migration_07_cleanup.pytests/unit/modules/init/test_init_ide_prompt_selection.pytests/unit/registry/test_category_groups.pytests/unit/registry/test_module_installer.pytests/unit/specfact_cli/registry/test_profile_presets.pytests/unit/utils/test_env_manager.pytests/unit/utils/test_ide_setup.pytests/unit/workflows/test_trustworthy_green_checks.py
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/examples/integration-showcases/integration-showcases-testing-guide.md (1)
1231-1247:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the pre-commit comparison explicitly use the documented baseline.
The docs say baseline selection is explicit, but the hook command still relies on implicit plan selection (
--repo .only), and the summary still describes the oldplan compare --code-vs-planflow. This can compare against the wrong plan when multiple bundles exist.As per coding guidelines `docs/**/*.md`: User-facing accuracy: CLI examples match current behavior; preserve Jekyll front matter; call out when README/docs index need sync.Suggested fix
-# Then compare against the explicitly named auto-derived bundle -specfact --no-banner code drift detect auto-derived --repo . +# Then compare explicit baseline vs explicit auto-derived bundle +BASELINE_BUNDLE="example4_precommit" +specfact --no-banner code drift detect auto-derived \ + --manual ".specfact/projects/${BASELINE_BUNDLE}" \ + --auto ".specfact/projects/auto-derived" -- **Manual plan**: The baseline bundle named by the workflow or `main.bundle.yaml` as fallback +- **Manual plan**: The baseline bundle path passed explicitly via `--manual` - **Auto plan**: The latest `auto-derived` project bundle (from `code import from-code auto-derived` or default bundle name) -- ✅ Pre-commit hook workflow: `code import from-code` → `plan compare --code-vs-plan` works correctly +- ✅ Pre-commit hook workflow: `code import from-code` → `code drift detect auto-derived --manual ... --auto ...` works correctlyAlso applies to: 1653-1653
🤖 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 `@docs/examples/integration-showcases/integration-showcases-testing-guide.md` around lines 1231 - 1247, The pre-commit example uses an implicit plan selection (`specfact --no-banner code drift detect auto-derived --repo .`) which can pick the wrong bundle when multiple bundles exist; update the example to pass an explicit baseline/plan bundle (e.g., add the explicit baseline argument used by `plan compare --code-vs-plan` such as `--baseline` or the explicit bundle name instead of relying on `--repo .` and `auto-derived`) and revise the surrounding text that describes `--code-vs-plan` so it documents the explicit baseline resolution (mentioning the manual plan fallback like `main.bundle.yaml` and the auto plan `auto-derived`) to ensure the CLI example and description match current behavior.
🤖 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/examples/integration-showcases/integration-showcases-testing-guide.md`:
- Around line 1153-1162: The snippet uses a hard-coded BUNDLE_NAME and an
undefined PLAN_FILE which breaks copy/paste and points health checks at the
wrong baseline; change it to derive the bundle and plan names from the actual
file variables used earlier (ensure PLAN_FILE is defined in the preceding steps
or replace PLAN_FILE with the correct existing variable), set BUNDLE_NAME from
that derived value (instead of "example4_github_actions"), compute PLAN_NAME
from the same PLAN_FILE/descriptor, and update both specfact --no-banner project
health-check calls to use these derived BUNDLE_NAME and PLAN_NAME so they target
the correct baseline.
In `@src/specfact_cli/modules/init/src/commands.py`:
- Line 16: Replace the hard dependency on the private typer internals by
removing the import of typer._click.core.Context and using the public context
type instead: change the import to a public Context (e.g., import typer and use
typer.Context or import click and use click.Context) and update the
`@beartype-decorated` init callback signature to reference that public Context
type; ensure you only modify the import and the type used in the init function
signature (symbols to update: the import of typer._click.core.Context and the
init callback that currently uses that type).
---
Outside diff comments:
In `@docs/examples/integration-showcases/integration-showcases-testing-guide.md`:
- Around line 1231-1247: The pre-commit example uses an implicit plan selection
(`specfact --no-banner code drift detect auto-derived --repo .`) which can pick
the wrong bundle when multiple bundles exist; update the example to pass an
explicit baseline/plan bundle (e.g., add the explicit baseline argument used by
`plan compare --code-vs-plan` such as `--baseline` or the explicit bundle name
instead of relying on `--repo .` and `auto-derived`) and revise the surrounding
text that describes `--code-vs-plan` so it documents the explicit baseline
resolution (mentioning the manual plan fallback like `main.bundle.yaml` and the
auto plan `auto-derived`) to ensure the CLI example and description match
current behavior.
🪄 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: 830ef8fc-c832-46c5-b03d-9f4e8ca02114
📒 Files selected for processing (32)
.github/workflows/docs-review.yml.github/workflows/pr-orchestrator.yml.github/workflows/specfact.yml.markdownlint.jsonCHANGELOG.mddocs/examples/integration-showcases/integration-showcases-testing-guide.mddocs/migration/migration-cli-reorganization.mddocs/reference/commands.generated.jsondocs/reference/commands.generated.mdllms.txtopenspec/CHANGE_ORDER.mdopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.mdopenspec/changes/tester-cli-reliability/tasks.mdpyproject.tomlscripts/check-command-contract.pyscripts/check-docs-commands.pyscripts/generate-command-overview.pysetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/cli.pysrc/specfact_cli/modules/init/module-package.yamlsrc/specfact_cli/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/module-package.yamlsrc/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/utils/structure.pysrc/specfact_cli/utils/suggestions.pytests/unit/cli/test_error_guidance.pytests/unit/cli/test_lean_help_output.pytests/unit/commands/test_update.pytests/unit/docs/test_docs_validation_scripts.pytests/unit/utils/test_suggestions.py
✅ Files skipped from review due to trivial changes (11)
- tests/unit/utils/test_suggestions.py
- openspec/CHANGE_ORDER.md
- CHANGELOG.md
- src/specfact_cli/utils/structure.py
- .markdownlint.json
- openspec/changes/tester-cli-reliability/tasks.md
- setup.py
- openspec/changes/tester-cli-reliability/TDD_EVIDENCE.md
- docs/reference/commands.generated.json
- llms.txt
- docs/reference/commands.generated.md
🚧 Files skipped from review as they are similar to previous changes (11)
- src/init.py
- src/specfact_cli/modules/init/module-package.yaml
- docs/migration/migration-cli-reorganization.md
- src/specfact_cli/init.py
- tests/unit/cli/test_error_guidance.py
- scripts/check-command-contract.py
- tests/unit/docs/test_docs_validation_scripts.py
- scripts/generate-command-overview.py
- .github/workflows/pr-orchestrator.yml
- tests/unit/commands/test_update.py
- src/specfact_cli/cli.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). (5)
- GitHub Check: Type Checking (basedpyright)
- GitHub Check: CLI Command Validation
- GitHub Check: Contract-First CI
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
🧰 Additional context used
📓 Path-based instructions (19)
**/*.yaml
📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)
YAML files must pass linting using: hatch run yaml-lint with relaxed policy.
Files:
src/specfact_cli/modules/upgrade/module-package.yaml
**/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)
Validate YAML configuration files locally using
hatch run yaml-lintbefore committing
**/*.{yml,yaml}: Format all YAML and workflow files usinghatch run yaml-fix-allbefore committing
Use Prettier to fix whitespace, indentation, and final newline across YAML files
Use yamllint with the repo .yamllint configuration (line-length 140, trailing spaces and final newline enforced) to lint non-workflow YAML files
Files:
src/specfact_cli/modules/upgrade/module-package.yaml
**/*.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/specfact_cli/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/src/commands.pytests/unit/cli/test_lean_help_output.pysrc/specfact_cli/utils/suggestions.pyscripts/check-docs-commands.py
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/specfact_cli/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/utils/suggestions.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/specfact_cli/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/src/commands.pytests/unit/cli/test_lean_help_output.pysrc/specfact_cli/utils/suggestions.py
**/*.{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/specfact_cli/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/src/commands.pytests/unit/cli/test_lean_help_output.pysrc/specfact_cli/utils/suggestions.pyscripts/check-docs-commands.py
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/modules/init/src/commands.pysrc/specfact_cli/modules/upgrade/src/commands.pysrc/specfact_cli/utils/suggestions.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:
pyproject.toml
{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:
pyproject.toml
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
.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/specfact.yml.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/specfact.yml.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/specfact.yml.github/workflows/docs-review.yml
**/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/cli/test_lean_help_output.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/cli/test_lean_help_output.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/cli/test_lean_help_output.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:
docs/examples/integration-showcases/integration-showcases-testing-guide.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/examples/integration-showcases/integration-showcases-testing-guide.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/examples/integration-showcases/integration-showcases-testing-guide.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
docs/examples/integration-showcases/integration-showcases-testing-guide.md
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/check-docs-commands.py
🪛 GitHub Actions: Docs Review / 0_Docs Review.txt
src/specfact_cli/modules/init/src/commands.py
[error] 16-16: Import error in commands.py: 'from typer._click.core import Context as TyperClickContext' failed with 'ModuleNotFoundError: No module named 'typer._click''.
🪛 GitHub Actions: Docs Review / Docs Review
src/specfact_cli/modules/init/src/commands.py
[error] 16-16: Import error: from typer._click.core import Context as TyperClickContext (typer._click module not found).
🪛 zizmor (1.25.2)
.github/workflows/specfact.yml
[info] 86-86: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Many docs and package prompts already use the new subcommand names and argument ordering introduced by the PR:
- docs/reference/command-syntax-policy.md documents the canonical ordering "specfact code import --repo . legacy-api" and explicitly enforces options-before-positional for code import examples. [::nold-ai/specfact-cli-modules::docs/reference/command-syntax-policy.md]
- Multiple docs and package prompt files reference
specfact code import --repo .andspecfact code repro --repo .(examples across docs/, packages//resources/prompts/, and openspec/). These will be validated against core’s generated command artifacts by the new checks. Representative hits: packages/specfact-project, packages/specfact-codebase, docs/guides/, docs/reference/, openspec/ . [::nold-ai/specfact-cli-modules::] (many files under docs/ and packages/)
-
Tests in this repo already exercise and expect the new command forms:
- tests/unit/test_check_prompt_commands_script.py includes
specfact code repro --repo .examples/tests. [::nold-ai/specfact-cli-modules::tests/unit/test_check_prompt_commands_script.py] - Other unit tests reference
specfact code reproandspecfact code importpatterns used by the new core scripts. [::nold-ai/specfact-cli-modules::tests/unit/]
- tests/unit/test_check_prompt_commands_script.py includes
-
Dev/bootstrap and CI integration expect/produce the SPECFACT_MODULES_REPO integration used by the core workflows:
- src/specfact_cli_modules/dev_bootstrap.py sets SPECFACT_MODULES_REPO to the modules repo root, resolves a paired core checkout via a
specfact-cli-modules-worktreesmarker, and exposes SPECFACT_REPO_ROOT for paired core checkouts. This aligns with the core PR’s checkout/WORKTREE resolution and environment variable usage (SPECFACT_MODULES_REPO / SPECFACT_REPO_ROOT). [::nold-ai/specfact-cli-modules::src/specfact_cli_modules/dev_bootstrap.py] - scripts/pre_commit_code_review.py also sets SPECFACT_MODULES_REPO for review tasks. [::nold-ai/specfact-cli-modules::scripts/pre_commit_code_review.py]
- src/specfact_cli_modules/dev_bootstrap.py sets SPECFACT_MODULES_REPO to the modules repo root, resolves a paired core checkout via a
-
No conflicting references found: the modules repo already assumes the new core command names and the SPECFACT_MODULES_REPO env pattern; therefore the core PR’s workflow steps that resolve and check out the modules repo (and generate/validate command artifacts) are consistent with this repo’s expectations. (Searches for commands.generated and llms.txt returned no direct artifacts in this repo.) [::nold-ai/specfact-cli-modules::]
Conclusion: nold-ai/specfact-cli-modules is a consumer of the CLI contract artifacts and already aligns with the new command names/ordering and environment variable integration; the core PR’s changes to generate/check command-overview artifacts and to checkout modules via SPECFACT_MODULES_REPO are expected and supported by this repo.
🔇 Additional comments (8)
src/specfact_cli/modules/upgrade/module-package.yaml (1)
2-2: LGTM!pyproject.toml (1)
7-7: LGTM!Also applies to: 250-253, 258-258
.github/workflows/specfact.yml (1)
86-96: LGTM!.github/workflows/docs-review.yml (1)
91-91: LGTM!Also applies to: 96-96
src/specfact_cli/modules/upgrade/src/commands.py (1)
296-299: LGTM!Also applies to: 310-310, 313-326
tests/unit/cli/test_lean_help_output.py (1)
5-5: LGTM!Also applies to: 20-25, 161-167, 173-173, 176-178
src/specfact_cli/utils/suggestions.py (1)
49-49: LGTM!Also applies to: 56-57, 93-95, 108-108, 130-130
scripts/check-docs-commands.py (1)
133-133: LGTM!Also applies to: 265-266, 295-296
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/unit/modules/init/test_first_run_selection.py`:
- Around line 18-21: The test
test_init_commands_avoid_private_typer_click_import uses a hardcoded
Path("src/specfact_cli/modules/init/src/commands.py") which breaks when pytest
runs outside repo root; update the test to derive the file location from the
imported module object (e.g., use the commands module's __file__ or
inspect.getsourcefile on specfact_cli.modules.init.src.commands) and read that
path's text, then assert "typer._click" not in source so the test is
location-independent.
🪄 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: 07002204-0694-49f0-89c4-7812b2acfa82
📒 Files selected for processing (14)
.github/workflows/specfact.ymlCHANGELOG.mddocs/reference/commands.generated.jsondocs/reference/commands.generated.mdllms.txtopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.mdpyproject.tomlsetup.pysrc/__init__.pysrc/specfact_cli/__init__.pysrc/specfact_cli/modules/init/module-package.yamlsrc/specfact_cli/modules/init/src/commands.pytests/unit/modules/init/test_first_run_selection.pytests/unit/workflows/test_trustworthy_green_checks.py
✅ Files skipped from review due to trivial changes (3)
- docs/reference/commands.generated.md
- docs/reference/commands.generated.json
- llms.txt
🚧 Files skipped from review as they are similar to previous changes (7)
- setup.py
- src/specfact_cli/modules/init/module-package.yaml
- pyproject.toml
- src/specfact_cli/modules/init/src/commands.py
- tests/unit/workflows/test_trustworthy_green_checks.py
- src/init.py
- src/specfact_cli/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: Compatibility (Python 3.11)
- GitHub Check: Linting (ruff, pylint, safe-write guard)
- GitHub Check: Tests (Python 3.12)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{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:
CHANGELOG.mdopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.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
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
CHANGELOG.mdopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.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/specfact.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/specfact.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/specfact.yml
**/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/modules/init/test_first_run_selection.py
**/*.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:
tests/unit/modules/init/test_first_run_selection.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/modules/init/test_first_run_selection.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/modules/init/test_first_run_selection.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:
tests/unit/modules/init/test_first_run_selection.py
**/*.{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:
tests/unit/modules/init/test_first_run_selection.py
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/tester-cli-reliability/TDD_EVIDENCE.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/changes/tester-cli-reliability/TDD_EVIDENCE.md
🪛 LanguageTool
openspec/changes/tester-cli-reliability/TDD_EVIDENCE.md
[uncategorized] ~158-~158: The official name of this software platform is spelled with a capital “H”.
Context: ... - Contract Validation failed because .github/workflows/specfact.yml ran `specfact c...
(GITHUB)
[uncategorized] ~161-~161: The official name of this software platform is spelled with a capital “H”.
Context: ...off Typer private namespaces. - Updated .github/workflows/specfact.yml to resolve the ...
(GITHUB)
[grammar] ~163-~163: Use a hyphen to join words.
Context: ...branch_when_available`. - Refreshed core generated command artifacts after the pa...
(QB_NEW_EN_HYPHEN)
🔀 Multi-repo context nold-ai/specfact-cli-modules
nold-ai/specfact-cli-modules findings
- Multiple docs and prompt templates already use the new CLI forms (
specfact code import --repo . …,specfact code repro --repo .), so they are consumers of the generated command contract and will be validated by the new core checks. Examples:- docs/reference/command-syntax-policy.md (enforces options-before-positional) [::nold-ai/specfact-cli-modules::docs/reference/command-syntax-policy.md]
- many docs under docs/, e.g. docs/reference/commands.md, docs/reference/modes.md, docs/bundles/codebase/* (multiple hits) [::nold-ai/specfact-cli-modules::docs/]
- packages/specfact-project examples and CLI help strings referencing
specfact code import … --repo .[::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/import_cmd/commands.py]
- Tests in this repo already exercise the new commands/forms and will interact with core’s validation scripts:
- tests/unit/test_check_prompt_commands_script.py contains many
specfact code repro --repo .examples [::nold-ai/specfact-cli-modules::tests/unit/test_check_prompt_commands_script.py] - tests/unit/docs/* reference
specfact code repropatterns used by new checks [::nold-ai/specfact-cli-modules::tests/unit/docs/]
- tests/unit/test_check_prompt_commands_script.py contains many
- Workflow/CI and dev helpers already integrate with the SPECFACT_MODULES_REPO env var and paired-worktree resolution that the core PR’s workflows will set/use:
- src/specfact_cli_modules/dev_bootstrap.py sets SPECFACT_MODULES_REPO and resolves paired worktrees (matching core CI checkout behavior) [::nold-ai/specfact-cli-modules::src/specfact_cli_modules/dev_bootstrap.py]
- scripts/pre_commit_code_review.py and pyproject.toml reference SPECFACT_MODULES_REPO usage [::nold-ai/specfact-cli-modules::scripts/pre_commit_code_review.py][::nold-ai/specfact-cli-modules::pyproject.toml]
- Codebase import runtime/ignore policy is present in this repo and aligns with core’s
code importruntime hardening spec (shared consumer ofcode importbehavior):- openspec/changes/codebase-import-runtime-hardening/* spec/proposal files and packages/specfact-project import/discovery helpers reference import traversal/ignore behavior [::nold-ai/specfact-cli-modules::openspec/changes/codebase-import-runtime-hardening][::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/utils/import_path_policy.py]
Conclusion: This repository is a direct consumer of the CLI command contract artifacts and the SPECFACT_MODULES_REPO checkout behavior introduced in the PR. The core PR’s generated artifacts and workflow changes align with existing usage here; expect the new core docs-validation and command-contract checks to exercise many files in this repo (docs, prompts, and unit tests).
🔇 Additional comments (3)
CHANGELOG.md (1)
13-22: LGTM!openspec/changes/tester-cli-reliability/TDD_EVIDENCE.md (1)
154-172: LGTM!.github/workflows/specfact.yml (1)
50-72: ⚡ Quick winSupply-chain sanity check: pinned
actions/checkoutSHA matches official releases 🕵️The pinned
actions/checkoutcommit34e114876b0b11c390a56381ad16ebd13914f8d5corresponds to the official tagsv4.3.1(andv4). The overall ref resolution + checkout (persist-credentials: false) wiring at.github/workflows/specfact.yml(lines ~50-72) remains solid.
SpecFact CLI Validation Report✅ All validations passed! |
SpecFact CLI Validation Report✅ All validations passed! |
SpecFact CLI Validation Report✅ All validations passed! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/examples/integration-showcases/integration-showcases-testing-guide.md (1)
1158-1158: ⚡ Quick winDoc consistency (not correctness):
--project-nameis a valid alias for--bundleinproject health-check.
Inspecfact-project’shealth_checkcommand,--project-nameis explicitly defined as “Alias for --bundle” and the code resolvesresolved_bundle = bundle or project_name, so the line 1158 snippet should work as written. Optional: switch to--bundle "$BUNDLE_NAME"to match the rest of the guide and reduce reader confusion.🤖 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 `@docs/examples/integration-showcases/integration-showcases-testing-guide.md` at line 1158, The docs show a command using the alias --project-name but the rest of the guide uses --bundle; update the snippet on line 1158 to use the canonical flag to match surrounding examples by replacing specfact --no-banner project health-check --project-name "$BUNDLE_NAME" --repo . with specfact --no-banner project health-check --bundle "$BUNDLE_NAME" --repo ., or alternatively add a brief parenthetical note that --project-name is an alias for --bundle (referencing the health_check handling where resolved_bundle = bundle or project_name) so readers aren’t confused.
🤖 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.
Nitpick comments:
In `@docs/examples/integration-showcases/integration-showcases-testing-guide.md`:
- Line 1158: The docs show a command using the alias --project-name but the rest
of the guide uses --bundle; update the snippet on line 1158 to use the canonical
flag to match surrounding examples by replacing specfact --no-banner project
health-check --project-name "$BUNDLE_NAME" --repo . with specfact --no-banner
project health-check --bundle "$BUNDLE_NAME" --repo ., or alternatively add a
brief parenthetical note that --project-name is an alias for --bundle
(referencing the health_check handling where resolved_bundle = bundle or
project_name) so readers aren’t confused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 327182df-02b3-4043-b308-4aab538c84d4
📒 Files selected for processing (5)
.github/workflows/specfact.ymldocs/examples/integration-showcases/integration-showcases-testing-guide.mdopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.mdtests/unit/modules/init/test_first_run_selection.pytests/unit/workflows/test_trustworthy_green_checks.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/specfact.yml
- tests/unit/workflows/test_trustworthy_green_checks.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). (1)
- GitHub Check: Tests (Python 3.12)
🧰 Additional context used
📓 Path-based instructions (10)
**/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/modules/init/test_first_run_selection.py
**/*.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:
tests/unit/modules/init/test_first_run_selection.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/modules/init/test_first_run_selection.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/modules/init/test_first_run_selection.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:
tests/unit/modules/init/test_first_run_selection.py
**/*.{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:
tests/unit/modules/init/test_first_run_selection.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:
docs/examples/integration-showcases/integration-showcases-testing-guide.mdopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.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/examples/integration-showcases/integration-showcases-testing-guide.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/examples/integration-showcases/integration-showcases-testing-guide.md
**/*.md
📄 CodeRabbit inference engine (.cursorrules)
Avoid markdown linting errors (refer to markdown-rules)
Files:
docs/examples/integration-showcases/integration-showcases-testing-guide.mdopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.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/tester-cli-reliability/TDD_EVIDENCE.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/changes/tester-cli-reliability/TDD_EVIDENCE.md
🪛 LanguageTool
openspec/changes/tester-cli-reliability/TDD_EVIDENCE.md
[uncategorized] ~162-~162: The official name of this software platform is spelled with a capital “H”.
Context: ...uns from the repository root. - Updated .github/workflows/specfact.yml to resolve the ...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli-modules
Linked repositories findings
nold-ai/specfact-cli-modules
-
Docs and prompts use the new CLI forms (
specfact code import --repo . ...andspecfact code repro --repo .), so they are direct consumers of the generated command contract and will be validated by the new core scripts:- docs/reference/commands.md, docs/reference/modes.md, docs/reference/command-syntax-policy.md, docs/reference/thorough-codebase-validation.md, docs/* (many) [::nold-ai/specfact-cli-modules::docs/reference/commands.md]
- packages/specfact-project/src/specfact_project/import_cmd/commands.py (examples using
specfact code import ... --repo .) [::nold-ai/specfact-cli-modules::packages/specfact-project/src/specfact_project/import_cmd/commands.py] - packages/specfact-codebase/resources/prompts/specfact.01-import.md (uses
specfact code import from-code ...) [::nold-ai/specfact-cli-modules::packages/specfact-codebase/resources/prompts/specfact.01-import.md]
-
Tests reference and exercise
specfact code reproandspecfact code importexamples that the core scripts will scan/validate:- tests/unit/test_check_prompt_commands_script.py (multiple
specfact code repro --repo .examples) [::nold-ai/specfact-cli-modules::tests/unit/test_check_prompt_commands_script.py] - tests/unit/docs/* reference repro/import patterns used by new checks [::nold-ai/specfact-cli-modules::tests/unit/docs/]
- tests/unit/test_check_prompt_commands_script.py (multiple
-
This repo already implements and tests the paired-worktree / SPECFACT_MODULES_REPO pattern the core PR’s workflows will set/use:
- src/specfact_cli_modules/dev_bootstrap.py and tests/unit/test_dev_bootstrap.py (resolve/set SPECFACT_MODULES_REPO, worktrees handling) [::nold-ai/specfact-cli-modules::src/specfact_cli_modules/dev_bootstrap.py][::nold-ai/specfact-cli-modules::tests/unit/test_dev_bootstrap.py]
- scripts/pre_commit_code_review.py uses SPECFACT_MODULES_REPO [::nold-ai/specfact-cli-modules::scripts/pre_commit_code_review.py]
-
OpenSpec and proposal/spec files in this repo already reference
code importruntime-ignore policy and command-contract validation work; these will be subject to the core-generated artifacts and freshness gates:- openspec/changes/codebase-import-runtime-hardening/* (specs, proposal, TDD_EVIDENCE) [::nold-ai/specfact-cli-modules::openspec/changes/codebase-import-runtime-hardening]
- openspec/changes/prompt-command-contract-validation/* (proposal/spec) [::nold-ai/specfact-cli-modules::openspec/changes/prompt-command-contract-validation]
-
The repository contains docs & scripts that the core PR’s new CI/docs checks may validate or fail if artifacts/examples diverge (command-ordering, examples with old positional bundle usage):
- docs/guides/command-chains.md, docs/guides/openspec-journey.md, docs/bundles/project/import-migration.md — contain legacy positional examples (some use
specfact code import legacy-api --repo .) that may be flagged by core validation [::nold-ai/specfact-cli-modules::docs/guides/command-chains.md][::nold-ai/specfact-cli-modules::docs/guides/openspec-journey.md][::nold-ai/specfact-cli-modules::docs/bundles/project/import-migration.md]
- docs/guides/command-chains.md, docs/guides/openspec-journey.md, docs/bundles/project/import-migration.md — contain legacy positional examples (some use
-
Summary: specfact-cli-modules is a direct consumer of the new generated command contract and the SPECFACT_MODULES_REPO pairing behavior; expect the core PR’s new generation/validation scripts and CI workflow changes to exercise many docs, prompts, and tests in this repo. Relevant files and tests are present and already partially aligned, but several docs/examples still show legacy argument ordering that core checks may flag. [::nold-ai/specfact-cli-modules::]
🔇 Additional comments (4)
tests/unit/modules/init/test_first_run_selection.py (1)
19-24: LGTM!docs/examples/integration-showcases/integration-showcases-testing-guide.md (2)
1223-1227: LGTM!
1230-1240: LGTM!Also applies to: 1261-1282
openspec/changes/tester-cli-reliability/TDD_EVIDENCE.md (1)
161-165: LGTM!
1 participant
Summary
Harden the core CLI reliability path for tester bug reports #585-#593:
tester-cli-reliabilityllms.txt,docs/reference/commands.generated.json, anddocs/reference/commands.generated.mdApplication Supportpath handling0.47.0and refreshes bundled module checksums for changed built-insValidation
openspec validate tester-cli-reliability --stricthatch run formathatch run type-check(0 errors; existing warning volume remains)hatch run linthatch run yaml-linthatch run check-command-overviewhatch run check-command-contracthatch run check-docs-commandsopenspec/changes/tester-cli-reliability/TDD_EVIDENCE.mdhatch run runtime-discovery-smoke --launcher directhatch run runtime-discovery-smoke --launcher uv-runspecfact code review run --scope changed --bug-hunt --include-tests --json --out .specfact/code-review.changed.json-> 0 blocking findingsNotes
Strict local release signature verification still requires approval-time/release signing keys; local signing refreshed checksum integrity only.