Conversation
- Add beartype to default Hatch env so standalone scripts import cleanly. - Hierarchy cache: paginate subIssues; fingerprint rendered fields (not updated_at). - Pre-commit Block 2: narrow safe-change skip; review gate uses staged contract paths. - apply_specfact_workspace_env always pins SPECFACT_* to current checkout. - validate_agent_rule_applies_when: robust reads; strict applies_when list types. - contract_first_smart_test: treat contracts/ as contract-relevant. - Docs/openspec: quality gate order, cache freshness, CHANGE_ORDER, backlog-sync paths. Co-authored-by: Dom <djm81@users.noreply.github.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughCross-repo contract: Workspace environment variable synchronizationKey change in Development dependenciesAdded Quality gate pipeline expansions
Pre-commit review gate hardening
GitHub hierarchy cache robustness
Agent-rule validation tightening
Contract-first smart test scope
No module-package.yaml, semver, or signature registry changesModule bundles, version declarations, and package metadata remain unchanged. No impact to OpenSpec governance documentationArchived WalkthroughAdds a path-prefix-based pre-commit SpecFact code-review gate and inserts a bundle-imports check into quality gates; refactors GitHub hierarchy cache syncing (paginated sub-issue fetch, fingerprint changes); updates OpenSpec governance/docs and tests; adds Changes
Sequence Diagram(s)mermaid Dev->>Hook: commit (staged paths) Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
![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: 85477fd8a2
ℹ️ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/agent-rules/50-quality-gates-and-review.md (1)
41-49:⚠️ Potential issue | 🟠 MajorAdd the final
specfact code reviewstep to the ordered gate list.This sequence now ends at
hatch run test, but the same governance page still makes unresolved review findings blocking and.specfact/code-review.jsonmandatory evidence. As written, the numbered order is incomplete and can send contributors down the wrong release path.📝 Suggested doc fix
8. `hatch run smart-test` 9. `hatch run test` +10. `hatch run specfact code review run --json --out .specfact/code-review.json`Based on learnings: Run quality gates in order: format → type-check → lint → yaml-lint → verify-modules-signature → contract-test → smart-test → test → specfact code review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agent-rules/50-quality-gates-and-review.md` around lines 41 - 49, The ordered gate list is missing the final "specfact code review" step; update the sequence in the list (the block containing the numbered hatch run commands: `hatch run format`, `hatch run type-check`, `hatch run lint`, `hatch run yaml-lint`, `hatch run check-bundle-imports`, `hatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump`, `hatch run contract-test`, `hatch run smart-test`, `hatch run test`) by appending the final step `hatch run specfact code review` (and mention the required evidence `.specfact/code-review.json` if present in the surrounding text) so the doc reflects the governance requirement that unresolved review findings block release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.md`:
- Around line 23-24: Update the quality-gate checklist entries 4.3 and 4.4 to
include the missing gate and explicit full review command: add `hatch run
check-bundle-imports` into the ordered sequence after `hatch run yaml-lint` (so
the sequence becomes format → type-check → lint → yaml-lint →
check-bundle-imports → verify-modules-signature → contract-test →
smart-test/test), and replace the vague freshness check in 4.4 with an explicit
final governance step that runs `hatch run specfact code review run --json --out
.specfact/code-review.json --scope full` and requires recording the resulting
`.specfact/code-review.json` and the final review command/timestamp in
`TDD_EVIDENCE.md` or PR notes before merge.
In `@scripts/pre-commit-quality-checks.sh`:
- Around line 94-95: Remove the whitelist entry that treats gate-defining
scripts as "safe changes" — specifically remove
scripts/pre_commit_code_review.py and tools/contract_first_smart_test.py from
the case/list that skips Block 2 (the whitelist in
scripts/pre-commit-quality-checks.sh), and instead ensure modifications to
packages/, registry/, scripts/, tools/, tests/, or openspec/changes/<change-id>/
(except TDD_EVIDENCE.md) cause the code-review gate to re-run; locate the
whitelist logic (the case/esac or skip list handling in
scripts/pre-commit-quality-checks.sh) and update it accordingly so gate-defining
scripts no longer bypass Block 2.
In `@scripts/sync_github_hierarchy_cache.py`:
- Around line 313-318: The call to json.loads(completed.stdout) in
_fetch_all_subissue_nodes() can raise JSONDecodeError which currently escapes
main(); catch json.JSONDecodeError around the json.loads(...) call and re-raise
it as a RuntimeError (including the raw completed.stdout or a short excerpt and
the issue_number/issue_url context) so main() which only handles
RuntimeError/OSError will report a controlled "GitHub hierarchy cache sync
failed" error; update the error handling immediately around the payload =
json.loads(completed.stdout) line and preserve the existing GraphQL "errors"
check.
In `@src/specfact_cli_modules/dev_bootstrap.py`:
- Around line 60-63: resolve_core_repo(repo_root) may return None but the
existing code only sets os.environ["SPECFACT_REPO_ROOT"] when core is truthy,
leaving a stale SPECFACT_REPO_ROOT in the environment; update the logic around
resolve_core_repo and the os.environ manipulation (reference resolve_core_repo,
core, and SPECFACT_REPO_ROOT) so that when core is None you explicitly
remove/unset SPECFACT_REPO_ROOT (e.g., del os.environ[...] or pop it) and when
core is present you set it to str(core).
In `@tests/unit/test_dev_bootstrap.py`:
- Around line 85-97: Add a regression test that ensures stale SPECFACT_REPO_ROOT
is removed when no core repo is found: in tests/unit/test_dev_bootstrap.py add a
case similar to test_apply_specfact_workspace_env_overwrites_stale_exports but
monkeypatch specfact_cli_modules.dev_bootstrap.resolve_core_repo to return None,
preset SPECFACT_MODULES_REPO and SPECFACT_REPO_ROOT to stale values, call
apply_specfact_workspace_env(repo_root), then assert SPECFACT_MODULES_REPO is
updated to repo_root.resolve() and SPECFACT_REPO_ROOT is no longer in os.environ
(or has been deleted); reference resolve_core_repo and
apply_specfact_workspace_env to locate the logic to exercise.
---
Outside diff comments:
In `@docs/agent-rules/50-quality-gates-and-review.md`:
- Around line 41-49: The ordered gate list is missing the final "specfact code
review" step; update the sequence in the list (the block containing the numbered
hatch run commands: `hatch run format`, `hatch run type-check`, `hatch run
lint`, `hatch run yaml-lint`, `hatch run check-bundle-imports`, `hatch run
verify-modules-signature --require-signature --payload-from-filesystem
--enforce-version-bump`, `hatch run contract-test`, `hatch run smart-test`,
`hatch run test`) by appending the final step `hatch run specfact code review`
(and mention the required evidence `.specfact/code-review.json` if present in
the surrounding text) so the doc reflects the governance requirement that
unresolved review findings block release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc2b2d59-a896-4af0-999c-43f673005c9f
📒 Files selected for processing (27)
README.mddocs/agent-rules/50-quality-gates-and-review.mddocs/agent-rules/60-github-change-governance.mddocs/agent-rules/70-release-commit-and-docs.mdopenspec/CHANGE_ORDER.mdopenspec/changes/archive/2026-04-09-governance-03-github-hierarchy-cache/specs/backlog-sync/spec.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/proposal.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-add/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-sync/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdopenspec/specs/backlog-sync/spec.mdopenspec/specs/github-hierarchy-cache/spec.mdpyproject.tomlscripts/pre-commit-quality-checks.shscripts/pre_commit_code_review.pyscripts/sync_github_hierarchy_cache.pyscripts/validate_agent_rule_applies_when.pysrc/specfact_cli_modules/dev_bootstrap.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/scripts/test_sync_github_hierarchy_cache.pytests/unit/scripts/test_validate_agent_rule_applies_when.pytests/unit/test_dev_bootstrap.pytests/unit/tools/test_contract_first_smart_test.pytools/contract_first_smart_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: quality (3.11)
- GitHub Check: quality (3.12)
- GitHub Check: quality (3.13)
🧰 Additional context used
📓 Path-based instructions (9)
openspec/changes/**
📄 CodeRabbit inference engine (CLAUDE.md)
Never manually move folders under
openspec/changes/intoarchive/. Archiving MUST useopenspec archive <change-id>command
Files:
openspec/changes/governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-add/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-sync/spec.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/proposal.mdopenspec/changes/archive/2026-04-09-governance-03-github-hierarchy-cache/specs/backlog-sync/spec.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-add/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-sync/spec.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/proposal.mdopenspec/changes/archive/2026-04-09-governance-03-github-hierarchy-cache/specs/backlog-sync/spec.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdopenspec/specs/github-hierarchy-cache/spec.mdopenspec/CHANGE_ORDER.mdopenspec/specs/backlog-sync/spec.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Line length must be 120 characters
Python target version is 3.11+
rufflinting runs on the full repository
Files:
src/specfact_cli_modules/dev_bootstrap.pytests/unit/scripts/test_validate_agent_rule_applies_when.pytools/contract_first_smart_test.pytests/unit/tools/test_contract_first_smart_test.pytests/unit/scripts/test_sync_github_hierarchy_cache.pytests/unit/test_dev_bootstrap.pyscripts/validate_agent_rule_applies_when.pyscripts/pre_commit_code_review.pyscripts/sync_github_hierarchy_cache.pytests/unit/scripts/test_pre_commit_code_review.py
{src,tests,tools}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
basedpyrightandpylintare scoped tosrc/,tests/, andtools/directories
Files:
src/specfact_cli_modules/dev_bootstrap.pytests/unit/scripts/test_validate_agent_rule_applies_when.pytools/contract_first_smart_test.pytests/unit/tools/test_contract_first_smart_test.pytests/unit/scripts/test_sync_github_hierarchy_cache.pytests/unit/test_dev_bootstrap.pytests/unit/scripts/test_pre_commit_code_review.py
src/**/*.py
⚙️ CodeRabbit configuration file
src/**/*.py: Repo infrastructure (not bundle code): keep parity with specfact-cli quality patterns;
contract-first public helpers where applicable; avoid print() in library paths.
Files:
src/specfact_cli_modules/dev_bootstrap.py
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/agent-rules/70-release-commit-and-docs.mddocs/agent-rules/50-quality-gates-and-review.mddocs/agent-rules/60-github-change-governance.md
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/unit/scripts/test_validate_agent_rule_applies_when.pytests/unit/tools/test_contract_first_smart_test.pytests/unit/scripts/test_sync_github_hierarchy_cache.pytests/unit/test_dev_bootstrap.pytests/unit/scripts/test_pre_commit_code_review.py
tools/**/*.py
⚙️ CodeRabbit configuration file
tools/**/*.py: Developer tooling aligned with pyproject Hatch scripts and CI expectations.
Files:
tools/contract_first_smart_test.py
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/validate_agent_rule_applies_when.pyscripts/pre_commit_code_review.pyscripts/sync_github_hierarchy_cache.py
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Run quality gates in order: format → type-check → lint → yaml-lint → verify-modules-signature → contract-test → smart-test → test → specfact code review
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: dev-deps installs specfact-cli from $SPECFACT_CLI_REPO when set, otherwise ../specfact-cli; in worktrees, bootstrap should prefer matching specfact-cli-worktrees/<branch> checkout before falling back to canonical sibling repo
Applied to files:
src/specfact_cli_modules/dev_bootstrap.pydocs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Keep worktree paths under ../specfact-cli-modules-worktrees/<branch-type>/<branch-slug> and forbid dev/main branches in worktrees
Applied to files:
src/specfact_cli_modules/dev_bootstrap.pydocs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-04-02T21:49:11.371Z
Learnt from: djm81
Repo: nold-ai/specfact-cli-modules PR: 136
File: registry/modules/specfact-spec-0.40.17.tar.gz.sha256:1-1
Timestamp: 2026-04-02T21:49:11.371Z
Learning: In nold-ai/specfact-cli-modules, module tarball signatures (registry/signatures/*.tar.sig) are generated by the `publish-modules` GitHub Actions runner during the publish workflow, not committed locally to the branch. Missing signature files should NOT be flagged as a pre-merge blocker in PRs.
Applied to files:
docs/agent-rules/70-release-commit-and-docs.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdREADME.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Use feature branches for implementation: feature/*, bugfix/*, hotfix/*, chore/* naming convention
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Use feature branches (`feature/*`, `bugfix/*`, `hotfix/*`, `chore/*`) for development. Never work directly on `dev` and `main` branches
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Run publish pre-check with 'python scripts/publish-module.py --bundle <bundle>' before publishing
Applied to files:
docs/agent-rules/70-release-commit-and-docs.mdREADME.mddocs/agent-rules/50-quality-gates-and-review.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Use Git worktrees for parallel branch work with paths: `../specfact-cli-modules-worktrees/<branch-type>/<branch-slug>`
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Update core_compatibility in packages/<bundle>/module-package.yaml and registry/index.json when a bundle requires a newer minimum specfact-cli version
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to {packages/*/module-package.yaml,registry/index.json} : When bumping a bundle version, review and update `core_compatibility` in both `module-package.yaml` and `registry/index.json`
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Update registry/index.json with new latest_version, artifact URL, and checksum during release process
Applied to files:
docs/agent-rules/70-release-commit-and-docs.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to openspec/changes/** : Never manually move folders under `openspec/changes/` into `archive/`. Archiving MUST use `openspec archive <change-id>` command
Applied to files:
openspec/changes/archive/2026-04-09-governance-03-github-hierarchy-cache/specs/backlog-sync/spec.mdopenspec/CHANGE_ORDER.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Use 'openspec archive <change-id>' for archiving changes; do not manually move folders under openspec/changes/ into openspec/changes/archive/
Applied to files:
openspec/changes/archive/2026-04-09-governance-03-github-hierarchy-cache/specs/backlog-sync/spec.mdopenspec/CHANGE_ORDER.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Record failing/passing test evidence in openspec/changes/<change-id>/TDD_EVIDENCE.md and record review commands/timestamps when changes touch behavior or quality gates
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdREADME.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdtests/unit/scripts/test_pre_commit_code_review.py
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Follow strict TDD order: spec delta -> failing tests -> implementation -> passing tests -> quality gates. Record TDD evidence in `openspec/changes/<change-id>/TDD_EVIDENCE.md`
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Verify an active OpenSpec change explicitly covers the requested scope before changing code; follow strict TDD order: spec delta → failing tests → implementation → passing tests → quality gates
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdREADME.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdopenspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.mdtests/unit/scripts/test_pre_commit_code_review.py
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Re-run code review when files in packages/, registry/, scripts/, tools/, tests/, or openspec/changes/<change-id>/ (excluding TDD_EVIDENCE.md) are modified
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdREADME.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdscripts/pre_commit_code_review.pytests/unit/scripts/test_pre_commit_code_review.py
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Run quality gates in order: format → type-check → lint → yaml-lint → verify-modules-signature → contract-test → smart-test → test → specfact code review
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdREADME.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.mdscripts/pre_commit_code_review.pytests/unit/scripts/test_pre_commit_code_review.py
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Generate and maintain .specfact/code-review.json using 'hatch run specfact code review run --json --out .specfact/code-review.json' before marking OpenSpec changes as complete
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.mdREADME.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdscripts/pre_commit_code_review.pytests/unit/scripts/test_pre_commit_code_review.py
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Applies to packages/*/src/**/*.py : Only allowed `specfact_cli.*` prefixes may be imported in bundle code (CORE/SHARED APIs only)
Applied to files:
openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Run `hatch run check-bundle-imports` to enforce bundle import policies
Applied to files:
README.mddocs/agent-rules/50-quality-gates-and-review.md
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Run quality gates in order: format, type-check, lint, yaml-lint, verify-modules-signature, contract-test, smart-test, test
Applied to files:
README.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdopenspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Install and configure pre-commit hooks to mirror CI quality gates, running in order: module signature verification → scripts/pre-commit-quality-checks.sh → scripts/pre_commit_code_review.py
Applied to files:
README.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.mdscripts/pre_commit_code_review.py
📚 Learning: 2026-03-25T21:31:11.712Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T21:31:11.712Z
Learning: Pre-commit hooks must mirror CI configuration: run `pre-commit install && pre-commit run --all-files`
Applied to files:
README.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.md
📚 Learning: 2026-03-31T23:13:02.695Z
Learnt from: CR
Repo: nold-ai/specfact-cli-modules PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-31T23:13:02.695Z
Learning: Scope type-check and lint quality gates to src/, tests/, and tools/ directories for repo tooling quality
Applied to files:
README.mdscripts/pre-commit-quality-checks.shdocs/agent-rules/50-quality-gates-and-review.md
🪛 LanguageTool
openspec/specs/backlog-sync/spec.md
[uncategorized] ~11-~11: The official name of this software platform is spelled with a capital “H”.
Context: ...Epic and Feature planning metadata from .specfact/backlog/github_hierarchy_cache.md, with determinist...
(GITHUB)
[uncategorized] ~11-~11: The official name of this software platform is spelled with a capital “H”.
Context: ...md**, with deterministic sync state in **.specfact/backlog/github_hierarchy_cache_state.json`**, before p...
(GITHUB)
[uncategorized] ~11-~11: The official name of this software platform is spelled with a capital “H”.
Context: ... and write those exact paths (or invoke python scripts/sync_github_hierarchy_cache.py, which uses them by...
(GITHUB)
[uncategorized] ~38-~38: The official name of this software platform is spelled with a capital “H”.
Context: ...eparing GitHub sync metadata - WHEN .specfact/backlog/github_hierarchy_cache.md is present and curr...
(GITHUB)
[uncategorized] ~38-~38: The official name of this software platform is spelled with a capital “H”.
Context: ...y_cache.mdis present and current (per.specfact/backlog/github_hierarchy_cache_state.json` / refresh r...
(GITHUB)
docs/agent-rules/60-github-change-governance.md
[uncategorized] ~62-~62: The official name of this software platform is spelled with a capital “H”.
Context: ... a current view of GitHub state: 1. If .specfact/backlog/github_hierarchy_cache.md is missing, or its ...
(GITHUB)
[uncategorized] ~62-~62: The official name of this software platform is spelled with a capital “H”.
Context: ...nds** compared to current UTC time, run python scripts/sync_github_hierarchy_cache.py. Treat the cache as...
(GITHUB)
[uncategorized] ~62-~62: The official name of this software platform is spelled with a capital “H”.
Context: ...ache.py. Treat the cache as fresh when .specfact/backlog/github_hierarchy_cache_state.json` exists and ...
(GITHUB)
🔀 Multi-repo context nold-ai/specfact-cli
Findings for nold-ai/specfact-cli
-
scripts/pre_commit_code_review.py
- File present and inspected. Contains old function filter_review_files(paths: Sequence[str]) -> list[str] that filters only Python suffixes and builds the review command. Tests and README changes in the PR indicate this function is renamed/replaced by filter_review_gate_paths and _is_review_gate_path. Review: callers/tests in this repo will need the new symbol/signature. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py]
-
tests referencing pre-commit review behavior
- tests/unit/scripts/test_pre_commit_code_review.py contains tests for the review-file filtering and for build_review_command/main behavior; these tests were changed in the PR to reflect new path-based filtering. Verify test updates match the new function name/behavior. [::nold-ai/specfact-cli::tests/unit/scripts/test_pre_commit_code_review.py]
-
scripts/sync_github_hierarchy_cache.py
- compute_hierarchy_fingerprint defined at scripts/sync_github_hierarchy_cache.py:340 (reported by ripgrep). The PR modifies fingerprint composition and subIssues pagination; tests target compute_hierarchy_fingerprint and fetch flow. Consumers/docs expect deterministic cache files and a companion state JSON (openspec/docs reference). Verify fingerprint changes are compatible with tools that read/write .specfact/backlog/github_hierarchy_cache.md and github_hierarchy_cache_state.json. [::nold-ai/specfact-cli::scripts/sync_github_hierarchy_cache.py]
-
Tests for hierarchy cache
- tests/unit/scripts/test_sync_github_hierarchy_cache.py references compute_hierarchy_fingerprint heavily and was updated to assert behavior about ignoring updated_at and including rendered fields. Ensure test expectations align with the new fingerprint implementation. [::nold-ai/specfact-cli::tests/unit/scripts/test_sync_github_hierarchy_cache.py]
-
Environment variable SPECFACT_MODULES_REPO usage
- Many scripts and modules read SPECFACT_MODULES_REPO (examples: scripts/verify-bundle-published.py, scripts/publish-module.py, scripts/validate-modules-repo-sync.py, tools/command_package_runtime_validation.py, tests and docs). The PR changes apply_specfact_workspace_env to unconditionally overwrite SPECFACT_MODULES_REPO / SPECFACT_REPO_ROOT; that can affect tools/tests that expect preexisting values or sibling-discovery fallback. Review callers that set or rely on SPECFACT_* being preserved. [::nold-ai/specfact-cli::scripts/verify-bundle-published.py][::nold-ai/specfact-cli::scripts/publish-module.py][::nold-ai/specfact-cli::scripts/validate-modules-repo-sync.py][::nold-ai/specfact-cli::tools/command_package_runtime_validation.py]
-
contracts/ prefix and contract-first smart test
- tools/contract_first_smart_test.py and many code paths reference contracts/ paths. The PR adds "contracts/" to the relevant prefixes so staged contract changes now trigger contract-test gating. Confirm CI and local workflows that stage contract files expect this behavior and that any tooling invoking contract-test-status is updated accordingly. [::nold-ai/specfact-cli::tools/contract_first_smart_test.py]
-
Documentation & governance consumers
- docs/agent-rules/60-github-change-governance.md, openspec/* and AGENTS.md reference the hierarchy cache files and refresh/freshness rules. The sync_github_hierarchy_cache.py changes must preserve the documented cache-first contracts and path/state JSON expectations so cross-tooling remains compatible. [::nold-ai/specfact-cli::docs/agent-rules/60-github-change-governance.md][::nold-ai/specfact-cli::openspec/specs/github-hierarchy-cache/spec.md]
Summary: the PR touches repo-local tooling and gate scripts that have many local consumers (tests, scripts, docs). Key cross-boundary impacts to validate:
- rename/behavior change of the pre-commit review filter (update callers/tests).
- hierarchy cache fingerprint and subissue pagination (ensure other tools/readers of .specfact/backlog/* remain compatible).
- unconditional overwrite of SPECFACT_MODULES_REPO / SPECFACT_REPO_ROOT (affects scripts/tests relying on previous preservation).
- contracts/ prefix being treated as contract-relevant (affects contract-test gating).
🔇 Additional comments (20)
tests/unit/scripts/test_validate_agent_rule_applies_when.py (1)
77-91: Good targeted coverage for strictapplies_whenlist typing.This test cleanly exercises the non-string list-entry rejection path and verifies the emitted error context.
scripts/validate_agent_rule_applies_when.py (2)
55-59: Per-file read/decode failure handling is robust and non-blocking.Continuing after file-level I/O/UTF-8 errors keeps validation resilient across mixed repositories.
71-76: Strict list-entry type checks remove unsafe coercion.Rejecting non-string
applies_whenentries (while allowingnull) enforces the frontmatter contract deterministically.pyproject.toml (1)
19-19: No issues on this change.docs/agent-rules/70-release-commit-and-docs.md (1)
45-45: Good governance hardening for release flow entrypoint.This branch/worktree requirement is clear and aligned with the repo’s workflow constraints.
Based on learnings: "Use feature branches (
feature/*,bugfix/*,hotfix/*,chore/*) for development. Never work directly ondevandmainbranches".openspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-add/spec.md (1)
1-13: Looks good — spec framing is clearer without changing behavior.openspec/changes/project-runtime-01-safe-artifact-write-policy/specs/backlog-sync/spec.md (1)
1-13: Strong spec delta for adapter safety at local export boundaries.This clearly codifies non-silent overwrite behavior and output transparency.
As per coding guidelines: "Ensure
specfact backlog synclocal export/materialization avoids silent overwrites... and surface the chosen behavior in command output."openspec/changes/governance-04-deterministic-agent-governance-loading/proposal.md (1)
1-2: Header addition is clean and improves document scannability.openspec/changes/governance-04-deterministic-agent-governance-loading/specs/agent-governance-loading/spec.md (1)
1-2: Spec title update is good and consistent with neighboring OpenSpec docs.openspec/changes/archive/2026-04-09-governance-03-github-hierarchy-cache/specs/backlog-sync/spec.md (1)
1-57: Archived spec readability update is fine; no semantic drift detected.As per coding guidelines: "Never manually move folders under
openspec/changes/intoarchive/. Archiving MUST useopenspec archive <change-id>command".tools/contract_first_smart_test.py (1)
17-17: Contract-trigger scope extension looks correct.Including
contracts/in_RELEVANT_PREFIXEScorrectly routes staged contract changes throughcontract-test-status.openspec/changes/governance-04-deterministic-agent-governance-loading/CHANGE_VALIDATION.md (1)
5-5: Validation status wording is correctly hardened.The updated blocker language clearly prevents premature “fully validated” status while full-scope review remains failing.
tests/unit/tools/test_contract_first_smart_test.py (1)
32-32: Good coverage for the newcontracts/trigger path.These assertions lock both helper-level and status-level behavior for staged contract files.
Also applies to: 65-73
README.md (2)
43-43: Quality-gate command sequence update is solid.Adding
check-bundle-importsand the explicit code-review artifact command improves local/CI parity in the docs.Also applies to: 48-48
58-58:contracts/is correctly excluded from the review-gate filter.README.md line 58 accurately reflects the implementation: only
packages/,registry/,scripts/,tools/,tests/, andopenspec/changes/are forwarded to the code review gate. This matches the prefixes defined in_is_review_gate_pathand test expectations. No documentation update needed.docs/agent-rules/60-github-change-governance.md (1)
62-62: Cache freshness guidance is now correctly precise.The state-file
generated_atprimary check plus markdown mtime fallback is clear and operationally consistent.openspec/CHANGE_ORDER.md (1)
35-35: Change-order updates are consistent and well-linked.The archived
governance-03entry and thegovernance-04dependency mapping read cleanly and preserve traceability.Also applies to: 79-79
tests/unit/scripts/test_sync_github_hierarchy_cache.py (2)
78-105: Fingerprint regression coverage is strong.This test captures the intended contract: ignore volatile
updated_at, but react to rendered content changes.
498-505: Good reliability hardening in the fallback test.Using
try/finallyensures module cache cleanup even if assertions fail mid-test.openspec/changes/governance-04-deterministic-agent-governance-loading/TDD_EVIDENCE.md (1)
56-58: Release-gate blocker note is appropriately explicit.The new section cleanly documents the full-scope review requirement before promotion.
openspec/changes/project-runtime-01-safe-artifact-write-policy/tasks.md
Outdated
Show resolved
Hide resolved
Codex P2: pre_commit_code_review.py and contract_first_smart_test.py must not bypass Block 2 when staged alone—they define the review and contract-test gates. Co-authored-by: Dom <djm81@users.noreply.github.com>
- project-runtime tasks: add check-bundle-imports; explicit full code review step. - Agent rules: append specfact code review to canonical gate order with evidence path. - sync_github_hierarchy_cache: wrap subIssues JSON parse in RuntimeError for main(). - dev_bootstrap: pop SPECFACT_REPO_ROOT when core is unresolved; regression test. Co-authored-by: Dom <djm81@users.noreply.github.com>
2 participants
Summary
Addresses Codex P1 feedback and valid CodeRabbit findings from the dev→main promotion review thread: declare beartype in the default Hatch environment, harden GitHub hierarchy cache sync (sub-issue pagination + fingerprint aligned with rendered markdown), narrow Block 2 safe-change short-circuit so pipeline-defining edits still run review/contract gates, broaden the pre-commit code review input set to staged contract-relevant paths (not only Python), always overwrite
SPECFACT_*workspace env from the active checkout, and tighten agent-rule validation. Documentation, OpenSpec deltas, and contract-test status (contracts/prefix) are updated to match.Validation
hatch run formathatch run type-checkhatch run linthatch run yaml-linthatch run check-bundle-importshatch run test(full tree via project helper; 545 passed in this environment)hatch run verify-modules-signature --require-signature …was not re-run here because the CI/agent environment lacks the bundle verification public key (same failure mode as other cloud runs).Notes
specfact code review --scope fullremains a known separate blocker for release promotion; governance-04 validation docs now state PENDING/BLOCKED until that gate is green or an approved exception exists.