SDK query contracts: universal OR filters, truthful lifecycle scope, and scripting guarantees - #886
Merged
Merged
Conversation
Ship a reusable SDK multi-value filter grammar across list, search, aggregate, and update-many. Make lifecycle discovery truthful by defaulting list/search to all statuses with reproducible receipts, publish the CLI scripting contract, and gate exit/stream behavior, packed npm/Bun consumption, token budgets, and exact 100% coverage.\n\nCloses pm-999jh7, pm-gknu, and pm-psy1.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideImplements a reusable multi-value filter parser shared across SDK query surfaces, aligns CLI flag contracts and list/search/aggregate/update-many behavior around OR-composed filters and truthful all-status discovery, and documents/gates a scripting contract for machine clients with tests and changelog/docs updates. Sequence diagram for CLI list and aggregate commands using OR-composed filterssequenceDiagram
actor User
participant CliListCommand as registerListCommand
participant ListRunner as runRegisteredListCommand
participant ListQuery as runList
participant MultiFilters as parseStringFilterSet
participant AggregateCommand as runAggregate
User->>CliListCommand: pm list --status open,in_progress --type Task,Issue --tag a,b
CliListCommand-->>User: help text (status/type/tag repeatable or CSV)
User->>ListRunner: invoke list with options
ListRunner->>ListRunner: normalizeListOptions
ListRunner->>ListRunner: applyDefaultListProjection
ListRunner->>ListRunner: listOptions.status = "all" when undefined
ListRunner->>ListQuery: runList(undefined, listOptions, global)
ListQuery->>MultiFilters: parseTypeFilterSet(options.type, typeRegistry)
ListQuery->>MultiFilters: parseStringFilterSet(options.tag, {label: "--tag"})
ListQuery->>MultiFilters: parsePriorityFilterSet(options.priority)
ListQuery-->>ListRunner: items, filters.status = "all" or concrete set
User->>AggregateCommand: pm aggregate --status open,blocked --type Task,Issue
AggregateCommand->>AggregateCommand: parseStatus(options.status, statusRegistry)
AggregateCommand->>ListQuery: runList(undefined, {status: options.status, ...}, global)
AggregateCommand->>AggregateCommand: buildAggregateFilters(status_values)
AggregateCommand-->>User: filters.status_values echoed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Owner
Author
|
@greptileai review |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds shared CSV multi-value filters across query and bulk-update surfaces, changes primary list and search defaults to all lifecycle statuses, exposes aggregate status metadata, documents the CLI scripting contract, and adds integration and unit coverage. ChangesQuery contracts and parsing
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/register-list-query.ts (1)
344-367: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCollect repeatable list filter flags before normalization.
--tagusescollect, but--status,--type, and--priorityalso document repeatable input without it. Commander replaces prior values when the same scalar option is repeated, sopm list --type Task --type Issueonly keeps the last value instead of OR matching. Addcollectfor these options and add CLI tests for repeated flags and CSV input.🤖 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/cli/register-list-query.ts` around lines 344 - 367, Add the existing collect handler to the --status, --type, and --priority options in the command registration near --tag so repeated flags accumulate values before normalization. Preserve comma-separated parsing and OR matching, then add CLI tests covering repeated values and CSV input for each affected filter.
🤖 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 @.agents/pm/issues/pm-999jh7.toon:
- Around line 41-52: Add tests/integration/cli.integration.spec.ts to the
relevant PM issue projection, updating both .agents/pm/issues/pm-999jh7.toon and
.agents/pm/issues/pm-gknu.toon as needed so the integration spec is linked
before merge.
In @.agents/pm/issues/pm-gknu.toon:
- Around line 30-43: Use the PM CLI to add
tests/unit/sdk/multi-value-filters.spec.ts to the linked files for PM item
pm-gknu. Do not modify the generated .toon file or its history directly.
In `@docs/SCRIPTING.md`:
- Line 76: Update the xargs recipe in the scripting documentation to remove the
GNU-only -r option. Use a portable no-input guard while retaining -n 1, or
explicitly document GNU coreutils as a prerequisite if the existing command must
remain.
- Around line 21-24: Preserve each upstream pm exit status in the documented
recipes: in docs/SCRIPTING.md lines 21-24, return the captured pm list status
instead of exit 1; in lines 37-38, capture and forward pm contracts’ status
before jq -e; and in lines 74-75 and 82-83, capture and forward pm search and pm
list statuses before their jq pipelines.
In `@src/sdk/query/multi-value-filters.ts`:
- Around line 54-57: Update the token-processing flow in parseMultiValueFilter
to detect and reject any trimmed empty token before filtering or applying
options.normalize, rather than discarding empty values. Preserve normalization
for valid tokens, and add coverage for embedded and trailing empty CSV tokens,
including leading empties if consistent with the existing rejection behavior.
In `@tests/integration/query-filter-contract.spec.ts`:
- Around line 99-114: Strengthen the filter contract tests around runList and
the corresponding aggregate/update-many cases so every type, priority, assignee,
sprint, release, and bulk selector assertion produces a strict subset or
otherwise includes a nonmatching fixture. Verify each selector independently
while preserving the repeated CLI checks for their flags, ensuring ignored
filters cannot still return both created fixtures.
---
Outside diff comments:
In `@src/cli/register-list-query.ts`:
- Around line 344-367: Add the existing collect handler to the --status, --type,
and --priority options in the command registration near --tag so repeated flags
accumulate values before normalization. Preserve comma-separated parsing and OR
matching, then add CLI tests covering repeated values and CSV input for each
affected filter.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aebf90ac-ba68-49d1-8524-f0525186a06d
📒 Files selected for processing (29)
.agents/pm/history/pm-999jh7.jsonl.agents/pm/history/pm-gknu.jsonl.agents/pm/history/pm-psy1.jsonl.agents/pm/issues/pm-999jh7.toon.agents/pm/issues/pm-gknu.toon.agents/pm/tasks/pm-psy1.toonCHANGELOG.mdREADME.mddocs/README.mddocs/SCRIPTING.mddocs/agent-task-token-baseline.jsonsdk/public-surface.jsonsrc/cli/register-list-query.tssrc/sdk/cli-contracts/flag-contracts.tssrc/sdk/lifecycle/update-many.tssrc/sdk/query.tssrc/sdk/query/aggregate.tssrc/sdk/query/list.tssrc/sdk/query/multi-value-filters.tssrc/sdk/query/search-rendering.tssrc/sdk/query/search.tstests/fixtures/contracts/full.jsontests/integration/cli.integration.spec.tstests/integration/query-filter-contract.spec.tstests/integration/scripting-contract.spec.tstests/unit/commands/aggregate-command.spec.tstests/unit/commands/search-command.spec.tstests/unit/commands/update-many-command.spec.tstests/unit/sdk/multi-value-filters.spec.ts
Reject every empty CSV member, collect repeated list filters in direct Commander registration, isolate each selector across list/aggregate/update-many tests, preserve shell exit codes, and make xargs composition portable. Record all review-driven PM evidence and missing file links.
Owner
Author
|
First review-round inventory fully acknowledged at head e824afc:
Local correction proof: 6,800 tests, exact 100% statements/branches/functions/lines; focused query/scripting/registration tests, typecheck, ESLint, and docs-link gates pass. Requesting a fresh exact-head review now. |
Owner
Author
|
@greptileai review |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 33 minutes. |
Owner
Author
|
Second review-round acknowledgements for exact head e824afc:
Greptile was explicitly requested twice but has not emitted a PR review/comment. I am validating its repository gate separately and will preserve that service-response limitation rather than inventing approval. |
Owner
Author
|
@coderabbitai full review Final exact-head review requested after the PM evidence-only commit. Please review the complete PR. |
Owner
Author
|
@greptileai review Final exact-head review requested after the PM evidence-only commit. Please review the complete PR. |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 25 minutes. |
Owner
Author
|
Final reviewer acknowledgement for head |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
This broad SDK/CLI tranche makes project discovery and automation predictable across lifecycle history, query surfaces, package integrations, and shell clients.
Canonical PM items
Verification
Hosted analysis is intentionally left to this pushed exact SHA; the local pre-push gate correctly reported that the prior main SHA had no DeepScan result.
Summary by Sourcery
Align SDK and CLI query surfaces around truthful all-status discovery and a uniform multi-value filter grammar, and publish a tested scripting contract for machine clients.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by cubic
Unifies multi-value OR filters across the SDK/CLI, makes lifecycle discovery truthful by defaulting
list/searchto all statuses, and documents a scripting contract with stable exit codes and stream behavior. Addresses pm-999jh7, pm-gknu, and pm-psy1.New Features
\,and\\escapes and empty-value rejection; applied to--status,--type,--priority,--tag,--assignee,--sprint, and--releaseacross list/search/aggregate/update-many.listandsearchdefault tostatus=alland echofilters.status: "all"; aggregate discloses concrete selections viastatus_values.sdk/query/multi-value-filterson the public surface.Migration
--status open(or specify explicit statuses).\,and\\.Written for commit f39e5ad. Summary will update on new commits.