Skip to content

Commit f972b11

Browse files
authored
Merge pull request #88 from nold-ai/dev
Release dev to main: specfact-code-review 0.44.0
2 parents e09c494 + 1d796a1 commit f972b11

23 files changed

Lines changed: 962 additions & 86 deletions

File tree

CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@ All notable changes to this repository will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project follows SemVer for bundle versions.
77

8+
## [0.44.0] - 2026-03-17
9+
10+
### Added
11+
12+
- Add `--scope changed|full` and repeatable repo-relative `--path` filters to
13+
`specfact code review run` for deterministic changed-only, full-repository,
14+
and subtree-limited review selection.
15+
16+
### Changed
17+
18+
- Keep changed-only auto-discovery as the default, allow explicit test subtrees
19+
to opt matching tests back into scope, and extend the review-run docs plus
20+
cli-contract scenarios to cover the new targeting controls.
21+
822
## [0.43.0] - 2026-03-16
923

1024
### Added

docs/modules/code-review.md

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ Options:
1717
- `--score-only`: print only the integer `reward_delta`
1818
- `--fix`: apply Ruff autofixes and re-run the review before printing results
1919
- `--no-tests`: skip the targeted TDD gate
20+
- `--scope changed|full`: choose changed-only or full-repository auto-discovery
21+
when positional files are omitted
22+
- `--path PATH`: repeatable repo-relative subtree filter for auto-discovered
23+
review targets
2024
- `--include-tests/--exclude-tests`: include or exclude changed test files when
2125
review scope is auto-detected from `git diff`
2226
- `--include-noise/--suppress-noise`: include or suppress known low-signal
@@ -34,7 +38,27 @@ git ls-files --others --exclude-standard
3438
Only existing Python files from tracked or untracked workspace changes are
3539
reviewed. Test files under `tests/` are excluded by default for auto-detected
3640
review scope unless you pass `--include-tests` or answer yes in
37-
`--interactive` mode.
41+
`--interactive` mode. Explicit `--path tests/...` filters count as intentional
42+
targeting and keep matching tests in scope even with the default test
43+
exclusion.
44+
45+
Use `--scope full` to review the governed repository file set instead of just
46+
current changes:
47+
48+
```bash
49+
specfact code review run --scope full
50+
```
51+
52+
Use repeatable `--path` filters to limit either scope to one package or a
53+
focused source-plus-test slice:
54+
55+
```bash
56+
specfact code review run --scope full --path packages/specfact-code-review
57+
specfact code review run --scope changed --path packages/specfact-code-review --path tests/unit/specfact_code_review
58+
```
59+
60+
Positional `FILES...` cannot be mixed with `--scope` or `--path`. Choose one
61+
targeting style per invocation.
3862

3963
With default noise suppression, the review also hides known low-signal test
4064
findings such as:
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-03-17
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
## Verification Report: code-review-10-review-scope-modes
2+
3+
### Summary
4+
5+
| Dimension | Status |
6+
| --- | --- |
7+
| Completeness | 4/4 apply-ready artifacts present (`proposal`, `design`, `specs`, `tasks`) |
8+
| Correctness | Scope-mode and path-filter requirements align with upstream `code-review-10-review-scope-modes` |
9+
| Coherence | Proposal, design, and tasks consistently keep changed-only as the default auto-scope |
10+
11+
### Validation Checks
12+
13+
- Reviewed upstream `specfact-cli` proposal, design, and spec delta for `code-review-10-review-scope-modes`.
14+
- Reviewed local artifacts: `proposal.md`, `design.md`, `tasks.md`, and delta specs under `specs/`.
15+
- Checked current modules-repo review-run capabilities from `code-review-08-review-run-integration` to ensure this is a requirement change on an existing command, not a new command surface.
16+
17+
### CRITICAL
18+
19+
None.
20+
21+
### WARNING
22+
23+
- The derived change assumes the bundle command can identify a stable governed
24+
repository file set for `--scope full`. If existing runtime logic only
25+
supports changed-file discovery, implementation may need a new helper to keep
26+
full-review selection deterministic across unit and e2e tests.
27+
Recommendation: verify current target-resolution boundaries before coding and
28+
centralize scope resolution rather than duplicating it across command tests
29+
and runtime entry points.
30+
31+
### SUGGESTION
32+
33+
- Keep cli-val scenarios narrow and representative. One changed-only example,
34+
one full-review example, one subtree-filter example, and one invalid
35+
invocation are enough to prove the contract without making scenario YAML noisy.
36+
37+
### Dependency Analysis
38+
39+
- Modified capabilities: `review-run-command`, `review-cli-contracts`
40+
- Primary implementation touchpoint: `packages/specfact-code-review/src/specfact_code_review/run/`
41+
- Primary regression surface: `tests/unit/`, `tests/e2e/`, and `tests/cli-contracts/`
42+
- Upstream source dependency: `nold-ai/specfact-cli` change `code-review-10-review-scope-modes`
43+
44+
### Final Assessment
45+
46+
No critical issues found. The derived change is internally consistent, matches
47+
the upstream intent, and is ready for implementation after strict OpenSpec
48+
validation passes.
49+
50+
### OpenSpec Validation
51+
52+
- **Status**: Pass
53+
- **Command**: `openspec validate code-review-10-review-scope-modes --strict`
54+
- **Issues Found/Fixed**: 0
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# TDD Evidence: code-review-10 review scope modes
2+
3+
## Failing
4+
5+
### 2026-03-17 `tests/unit/specfact_code_review/run/test_commands.py`
6+
7+
Command:
8+
9+
```bash
10+
HATCH_DATA_DIR=/tmp/hatch-data \
11+
HATCH_CACHE_DIR=/tmp/hatch-cache \
12+
VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \
13+
hatch run pytest tests/unit/specfact_code_review/run/test_commands.py -q
14+
```
15+
16+
Result: `5 failed, 14 passed`
17+
18+
Key failures before implementation:
19+
20+
- `test_run_command_supports_full_scope_and_path_filters`
21+
- exit code was `2` because `--scope` was not supported yet
22+
- `test_run_command_supports_changed_scope_with_repeatable_path_filters`
23+
- exit code was `2` because `--scope`/`--path` were not supported yet
24+
- `test_run_command_rejects_scope_mixed_with_positional_files`
25+
- CLI returned the generic option error instead of the governed mixed-targeting message
26+
- `test_run_command_rejects_path_mixed_with_positional_files`
27+
- CLI returned the generic option error instead of the governed mixed-targeting message
28+
- `test_run_command_fails_when_scope_and_paths_match_no_files`
29+
- CLI returned the generic option error instead of an actionable empty-scope failure
30+
31+
## Passing
32+
33+
### 2026-03-17 Focused scope-mode tests
34+
35+
Commands:
36+
37+
```bash
38+
HATCH_DATA_DIR=/tmp/hatch-data \
39+
HATCH_CACHE_DIR=/tmp/hatch-cache \
40+
VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \
41+
hatch run pytest tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/run/test_runner.py -q
42+
43+
HATCH_DATA_DIR=/tmp/hatch-data \
44+
HATCH_CACHE_DIR=/tmp/hatch-cache \
45+
VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \
46+
hatch run pytest tests/e2e/specfact_code_review/test_review_run_e2e.py -q
47+
48+
HATCH_DATA_DIR=/tmp/hatch-data \
49+
HATCH_CACHE_DIR=/tmp/hatch-cache \
50+
VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \
51+
hatch run validate-cli-contracts
52+
```
53+
54+
Results:
55+
56+
- `tests/unit/specfact_code_review/run/test_commands.py` and `tests/unit/specfact_code_review/run/test_runner.py`: `36 passed`
57+
- `tests/e2e/specfact_code_review/test_review_run_e2e.py`: `2 passed`
58+
- CLI contract validation: `Validated 3 CLI contract scenario files.`
59+
60+
### 2026-03-17 SpecFact dogfood review
61+
62+
Command:
63+
64+
```bash
65+
SPECFACT_ALLOW_UNSIGNED=1 \
66+
HATCH_DATA_DIR=/tmp/hatch-data \
67+
HATCH_CACHE_DIR=/tmp/hatch-cache \
68+
VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \
69+
hatch run specfact code review run \
70+
--scope changed \
71+
--path packages/specfact-code-review \
72+
--path tests/unit/specfact_code_review \
73+
--json \
74+
--out /tmp/code-review-10-report.json
75+
```
76+
77+
Result:
78+
79+
- verdict: `PASS`
80+
- score: `115`
81+
- findings: `0`
82+
83+
### 2026-03-17 Worktree quality gates
84+
85+
Commands completed successfully:
86+
87+
```bash
88+
HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata hatch run format
89+
HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata hatch run type-check
90+
HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata hatch run lint
91+
HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata hatch run check-bundle-imports
92+
HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata hatch run smart-test
93+
HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata hatch run contract-test
94+
```
95+
96+
Results:
97+
98+
- `type-check`: `0 errors, 0 warnings, 0 notes`
99+
- `lint`: `10.00/10`
100+
- `smart-test`: `383 passed`
101+
- `contract-test`: `384 passed`
102+
103+
Pending:
104+
105+
- `verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump`
106+
- blocked until the final `packages/specfact-code-review/module-package.yaml` changes in this worktree are re-signed
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# Design: code-review scope modes for modules repo
2+
3+
## Context
4+
5+
`code-review-08-review-run-integration` gave the modules repo a working
6+
`specfact code review run` command, but its implicit changed-file discovery is
7+
too narrow for stepwise review debt cleanup. The upstream `specfact-cli`
8+
contract now defines two auto-discovery modes, `changed` and `full`, plus
9+
repo-relative path filters that can recursively narrow either mode.
10+
11+
This repository owns the runtime command implementation, test fixtures, cli-val
12+
scenario YAML, and bundle docs. The design therefore focuses on deterministic
13+
file selection inside the bundle rather than on higher-level GitHub or
14+
automation wiring.
15+
16+
## Goals / Non-Goals
17+
18+
**Goals:**
19+
- Keep default no-argument behavior aligned with existing changed-file review
20+
- Add `--scope changed|full` and repeatable `--path` filters at the bundle
21+
command boundary
22+
- Make subtree filtering recursive so users can review one package or test area
23+
at a time
24+
- Cover the new behavior with unit, e2e, and cli-val fixtures
25+
26+
**Non-Goals:**
27+
- Changing scoring, verdict mapping, or ledger integration
28+
- Adding globbing or exclusion syntax beyond repo-relative path prefixes
29+
- Reworking the internal runner ordering established in `code-review-08`
30+
31+
## Decisions
32+
33+
### Decision: Resolve scope before filtering by path
34+
35+
The bundle should first compute the candidate file set from the selected scope,
36+
then filter that set by any `--path` prefixes. That keeps the behavior stable
37+
between tests and runtime, and makes it easy to explain to users.
38+
39+
### Decision: Reject positional files mixed with scope controls
40+
41+
Positional files already provide an explicit review target. Mixing them with
42+
`--scope` or `--path` would create ambiguous precedence rules, so the command
43+
should fail fast and tell the user to choose one targeting style.
44+
45+
### Decision: Treat `--path` values as repo-relative recursive path segments
46+
47+
Users asked for folder and subfolder limiting. Prefix-based matching is enough
48+
for that need, but matching must happen on normalized path boundaries rather
49+
than raw strings. That keeps `--path packages/specfact-code-review` scoped to
50+
that package and its descendants instead of accidentally including sibling
51+
paths such as `packages/specfact-code-review-old`, while still avoiding new
52+
pattern syntax in the CLI contract.
53+
54+
### Decision: Explicit path filters may opt matched tests back in
55+
56+
The existing changed-only workflow excludes tests by default unless users pass
57+
`--include-tests`. For scope-mode filtering, explicit `--path tests/...`
58+
selection should count as intentional targeting, so matching test files remain
59+
reviewable even when the global include-tests toggle stays at its default.
60+
61+
## Risks / Trade-offs
62+
63+
- [Risk] Full-review mode may surface far more findings than users expect.
64+
Mitigation: keep it opt-in and document subtree filtering prominently.
65+
- [Risk] Scope/path selection may vary between unit tests and e2e execution if
66+
repo root detection is inconsistent.
67+
Mitigation: centralize target resolution in one helper and exercise it
68+
through both direct tests and installed-command tests.
69+
- [Risk] cli-val scenarios could lag behind actual command behavior.
70+
Mitigation: update review-run scenario YAML in the same change as the command
71+
implementation and validate them together.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# Change: Reapply code-review-10 review scope modes in modules repo
2+
3+
## Why
4+
5+
The upstream `specfact-cli` change `code-review-10-review-scope-modes` defines
6+
the governed contract for explicit changed-only vs. full review modes plus
7+
repo-relative path filtering. The modules repository needs a matching active
8+
change because the bundle-owned `specfact code review run` implementation,
9+
runtime fixtures, and cli-val scenarios live here.
10+
11+
Without this derived change, the upstream contract would exist only on paper:
12+
the installed `specfact-code-review` bundle would still lack the runtime
13+
behavior needed to review a full repo or a limited subtree in a controlled way.
14+
15+
## What Changes
16+
17+
- Extend the `specfact-code-review` bundle command to support `--scope changed`
18+
and `--scope full`.
19+
- Add repeatable `--path` filtering for repo-relative files and subfolders in
20+
auto-discovery mode.
21+
- Keep changed-only auto-discovery as the default behavior when users do not
22+
pass positional files or an explicit scope.
23+
- Treat explicit `--path tests/...` filters as an intentional opt-in for those
24+
matched test files without changing the default auto-scope exclusion of tests.
25+
- Add targeted tests, fixtures, and cli-val scenarios for changed-only reviews,
26+
full reviews, filtered subtree reviews, and invalid scope combinations.
27+
- Update bundle docs, changelog, and signed package metadata to describe the
28+
new review-scope controls.
29+
30+
## Capabilities
31+
32+
### New Capabilities
33+
34+
None.
35+
36+
### Modified Capabilities
37+
38+
- `review-run-command`: add explicit review-scope selection and repo-relative
39+
path filtering to the bundle command
40+
- `review-cli-contracts`: extend review-run scenario coverage for changed-only,
41+
full-review, and subtree-limited invocations
42+
43+
## Impact
44+
45+
- Affects `packages/specfact-code-review/src/specfact_code_review/run/` command
46+
parsing and target-file resolution
47+
- Expands runtime validation in `tests/unit/`, `tests/e2e/`, and
48+
`tests/cli-contracts/`
49+
- Requires documentation, changelog, and module metadata updates for the
50+
`specfact-code-review` bundle
51+
52+
## Source Tracking
53+
54+
<!-- source_repo: nold-ai/specfact-cli -->
55+
- **Source Change**: `code-review-10-review-scope-modes`
56+
- **Repository**: nold-ai/specfact-cli
57+
- **Last Synced Status**: re-applied in modules repo
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: cli-val scenarios exist for review command groups
4+
The modules repository SHALL define cli-val-compatible scenario YAML files for
5+
the `specfact code review run`, `ledger`, and `rules` command groups, including
6+
scope-mode and path-filter coverage for the review run command.
7+
8+
#### Scenario: review-run scenarios cover success, scope selection, and error paths
9+
- **GIVEN** `tests/cli-contracts/specfact-code-review-run.scenarios.yaml`
10+
- **WHEN** it is validated
11+
- **THEN** it includes success coverage, changed-only and full-review scope examples, subtree-filtered examples, and an error or anti-pattern scenario
12+
13+
#### Scenario: ledger scenarios cover update, status, and reset guardrails
14+
- **GIVEN** `tests/cli-contracts/specfact-code-review-ledger.scenarios.yaml`
15+
- **WHEN** it is validated
16+
- **THEN** it includes `update`, `status`, and reset guardrail coverage
17+
18+
#### Scenario: rules scenarios cover init, show, and update
19+
- **GIVEN** `tests/cli-contracts/specfact-code-review-rules.scenarios.yaml`
20+
- **WHEN** it is validated
21+
- **THEN** it includes the supported rules subcommands

0 commit comments

Comments
 (0)