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
clawpatch mapclawpatch review --feature <id> on some feature, so it leaves pending (any terminal status works).- Modify a file that feature owns, and commit.
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.
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 inpendingorerroris 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:selection.js:5-7:So
reviewed,needs-fix,fixed,skipped, andclaimedfeatures are never candidates for a--sincerun, regardless of what the diff touches.When the intersection ends up empty,
review.js:27-36returns{"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 reviewwith no diff scope should work through features that still need attention, and skipping already-reviewed ones is right. But--sinceexpresses 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:Over half the map cannot be selected by any
--sincerun. 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
clawpatch mapclawpatch review --feature <id>on some feature, so it leavespending(any terminal status works).clawpatch review --since <base>— the feature is not reviewed, and if it was the only one touched, the command reportsno features touched by diffand 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: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.