review --since filters by feature status before intersecting with the diff, silently skipping changed code

[youhaowei]

Summary

clawpatch review --since <base> is documented as reviewing what changed since a base ref, and it is a natural fit for a pre-push or CI gate. But selection applies the status filter first and the changed-files filter second, so any feature not currently in pending or error is dropped before the diff is consulted — even when the diff modifies files that feature owns.

The result is that a review of changed code can complete successfully having reviewed none of it, and exit 0.

Where it happens

review.js:398-400:

const candidates = selectReviewCandidates(features, flags);          // status filter
const sinceFiltered = await filterFeaturesByFilesSince(loaded.root, candidates, flags);  // diff filter
return limitFeatures(sinceFiltered, flags);

selection.js:5-7:

const selected = featureId === undefined
    ? projectFeatures.filter((feature) => ["pending", "error"].includes(feature.status))
    : projectFeatures.filter((feature) => feature.featureId === featureId);

So reviewed, needs-fix, fixed, skipped, and claimed features are never candidates for a --since run, regardless of what the diff touches.

When the intersection ends up empty, review.js:27-36 returns {"next": "no features touched by diff"} — which reads as "your change touched nothing interesting" but actually means "everything your change touched was filtered out by status."

Why this matters

The status filter is sensible for the backlog workflow it was written for: clawpatch review with no diff scope should work through features that still need attention, and skipping already-reviewed ones is right. But --since expresses a different question — "review this change" — and for that question a feature's prior status is not a reason to skip it. A feature reviewed last month, then modified today, is exactly what a gate needs to look at.

The failure is silent and self-reinforcing. The more a repo uses clawpatch, the more features leave pending, so coverage decreases over time. On our repo:

total features                 490
eligible (pending|error)       237
silently skipped               253  (52%)

Over half the map cannot be selected by any --since run. In practice this meant a security-fix branch passed our gate with {"next": "no features touched by diff"} and exit 0, while the same diff resolves to nine owning features.

Reproduction

1. clawpatch map
2. clawpatch review --feature <id> on some feature, so it leaves pending (any terminal status works).
3. Modify a file that feature owns, and commit.
4. clawpatch review --since <base> — the feature is not reviewed, and if it was the only one touched, the command reports no features touched by diff and exits 0.

Suggested fix

Make the diff filter authoritative when a file filter is present — i.e. when hasFileFilter(flags) is true (review.js:421-423), select by changed files and skip the status narrowing:

const scoped = hasFileFilter(flags);
const candidates = scoped
    ? filterFeaturesByProject(features, stringFlag(flags, "project"))
    : selectReviewCandidates(features, flags);

If changing the default is too disruptive, an explicit opt-out (--ignore-status, or --all-statuses) would be enough — the important part is that a gate can ask "review everything this diff touches" and get exactly that.

A smaller, separate improvement: distinguish the two ways the result set empties. "No feature owns any changed file" and "every owning feature was filtered out by status" are very different messages to a user, and today both print no features touched by diff.

Environment

clawpatch 0.7.1 (latest at time of filing), macOS, node global install.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed August 1, 2026, 9:42 PM ET / August 2, 2026, 01:42 UTC.

Summary
This issue remains necessary: current main still applies the pending/error backlog filter before changed-file matching, so diff-scoped review can silently omit changed feature slices and report success. The reporter’s offline reproduction matches the source and documented CI contract; this is a bounded existing-behavior bug suitable for a focused repair PR.

Reproducibility: yes. Current source provides a high-confidence path: make a changed feature non-pending/error, then run review --since <base> --dry-run; it is excluded before file matching. The reporter also supplied an offline status-varied reproduction against v0.7.1.

Next step
The bug is source-proven on both v0.7.1 and current main, with a narrow selection boundary and an established focused-test location.

Review details

Best possible solution:

Keep the normal unscoped pending/error queue, but make --since and --include-dirty begin with project-filtered features and then select every feature touching the changed set; add mixed-status and all-status-excluded regression coverage.

Do we have a high-confidence way to reproduce the issue?

Yes. Current source provides a high-confidence path: make a changed feature non-pending/error, then run review --since <base> --dry-run; it is excluded before file matching. The reporter also supplied an offline status-varied reproduction against v0.7.1.

Is this the best way to solve the issue?

Yes. Making changed-file scope authoritative only when --since or --include-dirty is present preserves the existing unscoped backlog policy and matches the documented diff-review contract without adding a new option.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• A repair that bypasses status filtering only in the review selector but lacks CI-path regression coverage could allow the documented CI gate to regress later.
• Changing unscoped selection rather than only file-scoped selection would alter the established pending/error backlog workflow.

Codex review notes: model internal, reasoning high; reviewed against 4a5028c27f01.

Label changes

Label justifications:

• P1: The documented diff-scoped review and CI workflow can exit successfully while skipping changed code.
• impact:other: This is a concrete CLI/CI review-coverage failure that does not fit a more specific owned impact class.

Evidence reviewed

Acceptance criteria:

• pnpm test src/workflow.test.ts
• pnpm test src/app.test.ts
• pnpm typecheck
• pnpm lint
• pnpm build

What I checked:

• Current source preserves the reported ordering: selectReviewFeatures calls selectReviewCandidates before filterFeaturesByFilesSince; the former retains only pending and error features unless an explicit feature is chosen, so changed records in other statuses never reach file matching. (src/review.ts:548, 4a5028c27f01)
• Docs establish the expected scoped behavior: The --since documentation says review is restricted to features whose owned or context files changed and presents it as the CI workflow; it does not qualify that scope by historical feature status. (docs/code-review.md:53, 4a5028c27f01)
• Shared CI path carries the bug: ciCommand maps first and then calls reviewCommand using reviewFlagSubset, which forwards since and includeDirty; a warm state directory therefore uses the same status-first selection path. (src/app.ts:193, 4a5028c27f01)
• Existing tests omit status interaction: The workflow suite covers changed owned/context files, no changed files, limits, and dirty-file union, but no test changes a touched feature from pending to needs-fix, reviewed, or another non-backlog status before scoped review. (src/workflow.test.ts:857, 4a5028c27f01)
• Release and history provenance: The status-selection lines blame to the v0.7.1 release commit, and the current branch has no subsequent changes to the review, selection, CI, mapper, workflow-test, or scoped-review-doc files; the defect remains in both the latest release and current main. (src/selection.ts:5, f6e00010cc2a)
• Maturity label unavailable: The required shortlist helper could not find qa/maturity-scores.yaml in this checkout, so applying maturity:stable would not be evidence-based.

Likely related people:

• Peter Steinberger: The available history attributes the review-selection code to the v0.7.1 release commit, and subsequent current-main commits are authored by Peter Steinberger; the checkout’s seven-commit history does not establish a more specific original author. (role: current feature-history owner; confidence: medium; commits: f6e00010cc2a, 4a5028c27f01; files: src/selection.ts, src/review.ts, src/app.ts)

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.