Repository navigation
Content-field & governance introspection: utilization stats + field-existence & missing-metadata filters (GH-241/242/236) - #251
Conversation
…2/236) Make under-contextualized items queryable — "project management = context management". Combines three GitHub issues behind one shared, pure primitive. GH-242 (pm-hntj): content-field existence filters on list/list-open/list-all and search, plus --filter-* bulk variants on update-many/close-many: --has-notes/--no-notes, --has-learnings/--no-learnings, --has-files/--no-files, --has-docs/--no-docs, --has-tests/--no-tests, --has-comments/--no-comments, --has-deps/--no-deps, --has-body/--empty-body, --has-linked-command/--no-linked-command. list/list-all force the heavy/with-body reader only when a content filter needs collections/body, keeping the light hot path otherwise. GH-236 (pm-mfl1): governance metadata missing-field selectors on the same commands: --filter-reviewer-missing/--filter-risk-missing/ --filter-confidence-missing/--filter-sprint-missing/--filter-release-missing. Wired as independent AND predicates; deliberately NOT folded into the existing --filter-metadata-missing union so that long-standing flag keeps its meaning. GH-241 (pm-7snq): pm stats --field-utilization — content-field utilization report (present/total/percent per field + body_populated/empty_body) so dormant features (e.g. notes/learnings at 0%) become discoverable. Stats switches to the with-body reader only when requested. New pure primitive: src/core/governance/content-fields.ts (ContentFieldFilters, isContentFieldPresent, computeContentFieldUtilization) + governance-missing predicates extending core/governance/metadata-coverage.ts. MCP param names align to ListOptions fields (the MCP path forwards params verbatim); pm_list/pm_search/ pm_update/pm_stats expose the new options. Full contracts/MCP/docs/completion surface; contracts golden regenerated. Security: bump vite override 7.3.2 -> 7.3.5 (GHSA fs.deny bypass / NTLMv2) and add @opentelemetry/core >=2.8.0 override (W3C Baggage unbounded allocation, runtime). Resolves Dependabot alerts #39/#40/#41. Coverage 100/100/100/100 (4310 tests). Closes #241 #242 #236. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @unbraind, your pull request is larger than the review limit of 150000 diff characters
|
@codex review |
|
Warning Review limit reached
More reviews will be available in 29 minutes and 51 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (38)
📝 WalkthroughWalkthroughThree governance features are added: (1) ChangesContent-field and governance filter features (GH-241/242/236)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
|
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements content-field existence filters (GH-242) and governance metadata missing-field filters (GH-236) across the list, search, update-many, and close-many commands, as well as content-field utilization metrics (GH-241) for the stats command. These features are supported by new core primitives, CLI options, completion scripts, and MCP server integrations. Feedback was provided to replace the default switch case in isContentFieldPresent with an exhaustive compile-time check to ensure TypeScript safety when handling the ContentField union type.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| default: | ||
| return hasLinkedCommand(item.tests); |
There was a problem hiding this comment.
Using a default case in the switch statement for a union type like ContentField bypasses TypeScript's exhaustiveness checking. If a new content field is added to the ContentField type in the future, the compiler won't warn us that it is unhandled here. It is safer to explicitly handle linked_command and use the default block to perform a compile-time exhaustiveness check.
case "linked_command":
return hasLinkedCommand(item.tests);
default: {
const _exhaustiveCheck: never = field;
return false;
}
There was a problem hiding this comment.
Code Review
This pull request implements content-field presence/absence filters and governance metadata missing-field filters across the list, search, update-many, and close-many commands, alongside content-field utilization metrics in the stats command. It introduces new core primitives for content-field and metadata-coverage governance, updates CLI contracts, shell completions, and documentation, and adds comprehensive unit tests. The review feedback suggests improving the switch statement in isContentFieldPresent by explicitly handling all union cases and adding a TypeScript exhaustiveness check to catch unhandled fields at compile time.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function isContentFieldPresent(item: ContentFieldItem, field: ContentField): boolean { | ||
| switch (field) { | ||
| case "notes": | ||
| return hasEntries(item.notes); | ||
| case "learnings": | ||
| return hasEntries(item.learnings); | ||
| case "files": | ||
| return hasEntries(item.files); | ||
| case "docs": | ||
| return hasEntries(item.docs); | ||
| case "tests": | ||
| return hasEntries(item.tests); | ||
| case "comments": | ||
| return hasEntries(item.comments); | ||
| case "deps": | ||
| return hasEntries(item.dependencies); | ||
| case "body": | ||
| return hasBody(item.body); | ||
| default: | ||
| return hasLinkedCommand(item.tests); | ||
| } | ||
| } |
There was a problem hiding this comment.
Using a default branch in the switch statement bypasses TypeScript's exhaustiveness checks. If a new ContentField is added to the union type in the future, the compiler won't warn about the missing case. It is highly recommended to explicitly handle "linked_command" and use an exhaustive check in the default branch to catch unhandled cases at compile time.
export function isContentFieldPresent(item: ContentFieldItem, field: ContentField): boolean {
switch (field) {
case "notes":
return hasEntries(item.notes);
case "learnings":
return hasEntries(item.learnings);
case "files":
return hasEntries(item.files);
case "docs":
return hasEntries(item.docs);
case "tests":
return hasEntries(item.tests);
case "comments":
return hasEntries(item.comments);
case "deps":
return hasEntries(item.dependencies);
case "body":
return hasBody(item.body);
case "linked_command":
return hasLinkedCommand(item.tests);
default: {
const _exhaustiveCheck: never = field;
return false;
}
}
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@package.json`:
- Around line 148-154: The pnpm.overrides field in package.json is deprecated in
pnpm v9+ and is being completely ignored, meaning version constraints for vite
and `@opentelemetry/core` are not enforced. Create a new pnpm-workspace.yaml file
in the repository root and move the overrides for vite (7.3.5) and
`@opentelemetry/core` (>=2.8.0) into it under an overrides section. Then remove
the entire pnpm.overrides block from the package.json file to complete the
migration.
🪄 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: CHILL
Plan: Pro
Run ID: 66045e06-f258-44f9-a7a6-f8874a10426e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
.agents/pm/features/pm-7snq.toon.agents/pm/features/pm-hntj.toon.agents/pm/features/pm-mfl1.toon.agents/pm/history/pm-7snq.jsonl.agents/pm/history/pm-hntj.jsonl.agents/pm/history/pm-mfl1.jsonl.agents/pm/history/pm-php4.jsonl.agents/pm/plans/pm-php4.toonCHANGELOG.mddocs/COMMANDS.mdpackage.jsonsrc/cli/commands/completion.tssrc/cli/commands/list-filter-shared.tssrc/cli/commands/list.tssrc/cli/commands/search.tssrc/cli/commands/stats.tssrc/cli/register-list-query.tssrc/cli/register-mutation.tssrc/cli/register-operations.tssrc/cli/registration-helpers.tssrc/core/governance/content-fields.tssrc/core/governance/metadata-coverage.tssrc/mcp/server.tssrc/sdk/cli-contracts.tssrc/sdk/cli-contracts/tool-option-contracts.tssrc/sdk/cli-contracts/tool-parameter-tables.tstests/fixtures/contracts/full.jsontests/unit/cli/registration-helpers.spec.tstests/unit/commands/completion-command.spec.tstests/unit/commands/content-governance-filter-resolvers.spec.tstests/unit/commands/contracts-command.spec.tstests/unit/commands/list-command.spec.tstests/unit/commands/metadata-governance-commands.spec.tstests/unit/commands/register-commands.spec.tstests/unit/commands/search-command.spec.tstests/unit/commands/stats-command.spec.tstests/unit/core/governance/content-fields.spec.tstests/unit/core/governance/metadata-coverage.spec.ts
👮 Files not reviewed due to content moderation or server errors (1)
- tests/fixtures/contracts/full.json
| "overrides": { | ||
| "rollup": ">=4.59.0", | ||
| "brace-expansion": ">=5.0.6", | ||
| "vite": "7.3.2", | ||
| "esbuild": "0.28.1" | ||
| "vite": "7.3.5", | ||
| "esbuild": "0.28.1", | ||
| "@opentelemetry/core": ">=2.8.0" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
jq -r '.pnpm.overrides.vite, .pnpm.overrides["`@opentelemetry/core`"]' package.json
pnpm why vite `@opentelemetry/core`
pnpm list vite `@opentelemetry/core` --depth InfinityRepository: unbraind/pm-cli
Length of output: 2235
🌐 Web query:
pnpm overrides configuration 2024 2025 new format package.json
💡 Result:
As of pnpm version 11 and later, configuration for overrides has moved from the package.json file to the pnpm-workspace.yaml file [1][2]. pnpm no longer reads overrides or other settings from the pnpm field in package.json [1][2]. The current standard format for defining overrides is within the root pnpm-workspace.yaml file using the following YAML structure [3][4]: overrides: "package-name": "version" "package-a@version-range>package-b": "new-version" Key changes and usage details: - Relocation: All overrides must now be defined at the root of the project in pnpm-workspace.yaml [3][2]. Definitions inside package.json are ignored and will not take effect [2]. - Syntax: The syntax allows for direct overrides, nested dependency overrides (using the > separator), and removing dependencies entirely using the - character [3][5][4]. - Catalogs: You can maintain version consistency by defining versions in a catalog and referencing them in your overrides using the catalog: protocol [3][4]. - Yarn Resolutions: While pnpm historically supported the Yarn-style resolutions field in package.json, this is now deprecated. Recent updates to pnpm (v11+) have moved to strictly enforcing overrides in pnpm-workspace.yaml [6][7]. Using resolutions at the root may trigger errors or deprecation warnings, and pnpm recommends migrating entirely to the overrides field [7]. If you are upgrading from an older version of pnpm, you must manually move these configurations from your package.json (under the pnpm field) to your pnpm-workspace.yaml file to ensure they are applied [2].
Citations:
- 1: https://pnpm.io/package_json
- 2: pnpm 11 silently ignores
pnpm.overridesandpnpm.patchedDependenciesinpackage.json(no deprecation warning, security-critical) pnpm/pnpm#11536 - 3: https://pnpm.io/settings
- 4: https://pnpm.io/next/settings
- 5: https://pnpm.io/10.x/settings
- 6: Overrides from package.json resolutions are lost when pnpm-workspace.yaml has overrides pnpm/pnpm#10675
- 7: fix: error when both resolutions and overrides exist, restore root resolutions support pnpm/pnpm#11941
🏁 Script executed:
#!/bin/bash
# Check for pnpm-workspace.yaml and its overrides configuration
if [ -f pnpm-workspace.yaml ]; then
echo "=== pnpm-workspace.yaml found ==="
cat pnpm-workspace.yaml
else
echo "=== pnpm-workspace.yaml NOT found ==="
fi
# Also check for .pnpmfile.cjs as an alternative override location
if [ -f .pnpmfile.cjs ]; then
echo "=== .pnpmfile.cjs found ==="
cat .pnpmfile.cjs
fiRepository: unbraind/pm-cli
Length of output: 97
Migrate dependency overrides to pnpm-workspace.yaml — current configuration is non-functional.
The pnpm.overrides field in package.json is deprecated and no longer read by pnpm (v9+). The warning in your pnpm output confirms this: "The 'pnpm' field in package.json is no longer read by pnpm... pnpm.overrides." These overrides are being ignored entirely, meaning vite@7.3.5 and @opentelemetry/core@>=2.8.0 constraints won't be enforced. When the lockfile is regenerated, transitive dependencies can drift to incompatible releases without protection.
Move overrides to a pnpm-workspace.yaml file in the repository root:
Migration required
overrides:
"vite": "7.3.5"
"`@opentelemetry/core`": ">=2.8.0"Remove the pnpm.overrides block from package.json after migrating.
🤖 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 `@package.json` around lines 148 - 154, The pnpm.overrides field in
package.json is deprecated in pnpm v9+ and is being completely ignored, meaning
version constraints for vite and `@opentelemetry/core` are not enforced. Create a
new pnpm-workspace.yaml file in the repository root and move the overrides for
vite (7.3.5) and `@opentelemetry/core` (>=2.8.0) into it under an overrides
section. Then remove the entire pnpm.overrides block from the package.json file
to complete the migration.
…emini review) Handle "linked_command" as an explicit case and drop the default branch so TypeScript enforces ContentField exhaustiveness at compile time (adding a field without a case becomes a compile error). No unreachable branch is introduced, so the literal-100% coverage gate is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review responses@gemini-code-assist (medium — exhaustiveness in @coderabbitai (critical — "overrides non-functional, migrate to pnpm-workspace.yaml"): This is a false positive for this repo's pnpm version (10.x). Verified empirically:
The package.json Re-triggering reviews on the updated branch. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
✏️ Learnings added
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements content-field presence filters, governance metadata missing-field filters, and stats content-field utilization metrics across list, search, stats, and bulk mutation commands. It introduces new core primitives for content-field tracking and extends metadata coverage predicates, along with updating CLI completions, documentation, and test suites. The review feedback suggests validating mutually exclusive bulk filters in mapBulkContentAndGovernanceFilters to provide clearer error messages, and correcting a signature mismatch in the hasListFilters test call.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function mapBulkContentAndGovernanceFilters(options: Record<string, unknown>): Record<string, unknown> { | ||
| const presence = (key: string): true | undefined => (options[key] === true ? true : undefined); | ||
| return { |
There was a problem hiding this comment.
When mutually exclusive bulk filters (e.g., --filter-has-notes and --filter-no-notes) are both passed, the conflict is currently caught downstream in resolveContentFieldFilters which throws an error referencing the non-bulk flags (e.g., --has-notes and --no-notes). This can be confusing to users. Consider adding a validation check inside mapBulkContentAndGovernanceFilters to throw a more accurate error message with the correct --filter- prefix.
function mapBulkContentAndGovernanceFilters(options: Record<string, unknown>): Record<string, unknown> {
const presence = (key: string): true | undefined => (options[key] === true ? true : undefined);
const conflicts: [string, string, string][] = [
["filterHasNotes", "filterNoNotes", "notes"],
["filterHasLearnings", "filterNoLearnings", "learnings"],
["filterHasFiles", "filterNoFiles", "files"],
["filterHasDocs", "filterNoDocs", "docs"],
["filterHasTests", "filterNoTests", "tests"],
["filterHasComments", "filterNoComments", "comments"],
["filterHasDeps", "filterNoDeps", "deps"],
["filterHasBody", "filterEmptyBody", "body"],
["filterHasLinkedCommand", "filterNoLinkedCommand", "linked-command"],
];
for (const [hasKey, noKey, label] of conflicts) {
if (options[hasKey] === true && options[noKey] === true) {
const hasFlag = hasKey === "filterEmptyBody" ? "--filter-empty-body" : `--filter-has-${label}`;
const noFlag = noKey === "filterEmptyBody" ? "--filter-empty-body" : `--filter-no-${label}`;
throw new PmCliError(
`Cannot combine ${hasFlag} with ${noFlag} for the same field.`,
EXIT_CODE.USAGE,
);
}
}
return {| }); | ||
|
|
||
| it("returns false when no filter is active", () => { | ||
| expect(hasListFilters({}, undefined, { includePagination: false })).toBe(false); |
There was a problem hiding this comment.
The hasListFilters function only accepts two arguments: list and includePagination?: boolean. Passing a third argument { includePagination: false } is a signature mismatch and completely redundant. Consider correcting it to hasListFilters({}, false).
| expect(hasListFilters({}, undefined, { includePagination: false })).toBe(false); | |
| expect(hasListFilters({}, false)).toBe(false); |
…emini review) When --filter-has-X and --filter-no-X are both passed to update-many/close-many, catch the conflict in mapBulkContentAndGovernanceFilters so the error names the bulk --filter-* flags the user typed, instead of the downstream list-level --has-/--no- flags that resolveContentFieldFilters would otherwise report. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round 2 review responses@gemini-code-assist (medium — bulk conflict error names the wrong flags, @gemini-code-assist (medium — Re-triggering reviews. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Bulk conflict error naming (applied in 0cc5e8c): That's the right fix — surfacing
Triggering the full review now. ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements three major governance and content-field introspection features: content-field utilization metrics for stats (GH-241), field-existence filters for list and search (GH-242), and missing-field filters for governance metadata (GH-236). It introduces a new pure primitive module core/governance/content-fields.ts to handle presence checks and compute utilization, and extends metadata-coverage.ts with predicates for missing reviewer, risk, confidence, sprint, and release fields. These filters are fully integrated into the CLI commands (list, search, update-many, close-many, and stats), shell completions, and MCP contracts. Extensive unit and integration tests have been added to verify the new filtering and stats behavior. There are no review comments to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
Makes under-contextualized items queryable — directly serving the project philosophy project management = context management. Combines three GitHub issues behind one shared, pure core primitive so an auditor/agent can answer "which items lack notes/body/tests?" and "which open work is missing governance metadata?" without external scripting.
pm stats --field-utilizationWhat's new
Content-field filters (#242) —
pm list/list-open/list-all/search--has-notes/--no-notes,--has-learnings/--no-learnings,--has-files/--no-files,--has-docs/--no-docs,--has-tests/--no-tests,--has-comments/--no-comments,--has-deps/--no-deps,--has-body/--empty-body,--has-linked-command/--no-linked-command. Bulk--filter-*variants land onupdate-many/close-manyfor deterministic backfill targeting.list/list-allforce the heavy / with-body reader only when a content filter needs collections/body — the light hot path is untouched otherwise.Governance missing-field selectors (#236) — same commands
--filter-reviewer-missing,--filter-risk-missing,--filter-confidence-missing,--filter-sprint-missing,--filter-release-missing. Wired as independent AND predicates and deliberately not folded into the existing--filter-metadata-missingunion (so that long-standing flag keeps its meaning and stays useful).pm stats --field-utilization(#241)Per-field present/total/percent (+
body_populated/empty_body) so dormant features (e.g. notes/learnings at 0%) become immediately discoverable. Switches to the with-body reader only when requested.Architecture
src/core/governance/content-fields.ts(ContentFieldFilters,isContentFieldPresent,computeContentFieldUtilization) + governance-missing predicates extendingcore/governance/metadata-coverage.ts.ListOptionsfields (the MCP path forwards params verbatim, unlike the CLI which normalizes first);pm_list/pm_search/pm_update/pm_statsexpose the new options. Contracts golden regenerated.Security (Dependabot #39/#40/#41)
viteoverride7.3.2→7.3.5(server.fs.deny bypass on Windows / launch-editor NTLMv2 disclosure — dev scope).@opentelemetry/core>=2.8.0override (W3C Baggage unbounded memory allocation — runtime scope, via Sentry).Testing
--field-utilizationall verified.Tracking
Closes #241, #242, #236. Orchestrated via plan
pm-php4; each sub-agent recorded its work on the corresponding pm item. This PR also regeneratesCHANGELOG.md, clearing pre-existing drift onmain.🤖 Generated with Claude Code