Improve agent token efficiency for history and activity#35
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (33)
📝 WalkthroughWalkthroughThis PR implements compact-by-default history and activity CLI output with a ChangesCompact History Projection and Extension Validation
GC Cleanup Artifacts Extension
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Reviewer's GuideDefault history/activity outputs to a new compact projection while keeping full JSON Patch payloads opt-in via a --full flag, tighten dynamic extension loose option handling (validation, canonicalization, and nested command resolution), and extend gc to clean semantic vector cache artifacts, with contracts, completions, MCP runtime, docs, and tests aligned to the new behavior. Sequence diagram for dynamic extension loose option validation and canonicalizationsequenceDiagram
participant User
participant PmCli as pm_CLI
participant Main as main.extractCommandScopedOptions
participant Reg as ExtensionRegistrations
participant Flags as collectExtensionFlagDefinitionsForInvocation
participant Validate as validateLooseCommandOptionsWithFlagDefinitions
participant Coerce as coerceLooseCommandOptionsWithFlagDefinitions
User->>PmCli: invoke extension command
PmCli->>Reg: getActiveExtensionRegistrations
PmCli->>Flags: collectExtensionFlagDefinitionsForInvocation(commandPath, commandArgs)
Flags-->>PmCli: extensionFlagDefinitions (including nested commands)
PmCli->>Main: extractCommandScopedOptions(actionCommand, commandArgs, extensionFlagDefinitions)
Main->>Main: parseLooseCommandOptions(commandArgs)
Main->>Validate: validateLooseCommandOptionsWithFlagDefinitions(looseOptions, definitions, commandPath)
Validate-->>Main: throws PmCliError on unknown/disabled/missing required flags
Main->>Coerce: coerceLooseCommandOptionsWithFlagDefinitions(looseOptions, definitions)
Coerce->>Coerce: resolveCanonicalLooseOptionKey(definition)
Coerce->>Coerce: collectLooseOptionKeys(definition)
Coerce->>Coerce: coerceLooseOptionValue for canonical key
Coerce-->>Main: coercedOptions with canonical keys
Main-->>PmCli: scopedOptions including validated extension flags
PmCli-->>User: run extension with canonicalized, validated options
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 reduces the default verbosity of the pm activity and pm history CLI commands by introducing a compact output mode, while providing a --full flag for users requiring complete JSON Patch payloads. It also enhances extension command handling by adding support for short flag parsing, canonicalization to long keys, and strict validation against declared flag definitions (checking for unknown, disabled, or missing required flags). Additionally, the pm gc command is expanded to clean up vectorization status and LanceDB artifacts. Feedback from the review identifies a precedence issue in subcommand flag lookup and suggests improving the accuracy of error messages when reporting unknown short flags.
Comment on lines
+576
to
+591
| const exact = collectExtensionFlagDefinitionsForCommand(registrations, commandPath); | ||
| if (exact.length > 0) { | ||
| return exact; | ||
| } | ||
| const pathParts = [commandPath]; | ||
| for (const arg of commandArgs) { | ||
| if (arg.startsWith("-")) { | ||
| break; | ||
| } | ||
| pathParts.push(arg); | ||
| const nested = collectExtensionFlagDefinitionsForCommand(registrations, pathParts.join(" ")); | ||
| if (nested.length > 0) { | ||
| return nested; | ||
| } | ||
| } | ||
| return []; |
There was a problem hiding this comment.
The current precedence logic for extension flag lookup returns the first match found, starting with the base commandPath. This means that if a parent command (e.g., acme) has flags defined, it will block the lookup for more specific subcommand flags (e.g., acme sync) when using dynamic command dispatch. The logic should be updated to search from the most specific path to the least specific.
Suggested change
| const exact = collectExtensionFlagDefinitionsForCommand(registrations, commandPath); | |
| if (exact.length > 0) { | |
| return exact; | |
| } | |
| const pathParts = [commandPath]; | |
| for (const arg of commandArgs) { | |
| if (arg.startsWith("-")) { | |
| break; | |
| } | |
| pathParts.push(arg); | |
| const nested = collectExtensionFlagDefinitionsForCommand(registrations, pathParts.join(" ")); | |
| if (nested.length > 0) { | |
| return nested; | |
| } | |
| } | |
| return []; | |
| const pathParts = commandPath.split(" ").filter(Boolean); | |
| for (const arg of commandArgs) { | |
| if (arg.startsWith("-")) { | |
| break; | |
| } | |
| pathParts.push(arg); | |
| } | |
| for (let i = pathParts.length; i >= 1; i--) { | |
| const currentPath = pathParts.slice(0, i).join(" "); | |
| const flags = collectExtensionFlagDefinitionsForCommand(registrations, currentPath); | |
| if (flags.length > 0) { | |
| return flags; | |
| } | |
| } |
| for (const key of Object.keys(options)) { | ||
| if (!allowed.has(key)) { | ||
| const expected = labels.length > 0 ? ` Expected one of: ${[...new Set(labels)].join(", ")}.` : ""; | ||
| throw new PmCliError(`Unknown option '--${key}' for extension command '${commandPath}'.${expected}`, EXIT_CODE.USAGE); |
There was a problem hiding this comment.
The error message for unknown options hardcodes the -- prefix, which is misleading if the user provided a short flag (e.g., -d would be reported as --d). We should use a heuristic to determine the appropriate prefix based on the key length.
const label = key.length === 1 ? `-${key}` : `--${key}`;
throw new PmCliError(`Unknown option '${label}' for extension command '${commandPath}'.${expected}`, EXIT_CODE.USAGE);![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
normalizeActivityOptions, the returnedcompactflag is derived solely fromoptions.fulland always defaults totrue, which means any explicitoptions.compactvalue (e.g.,falsewhen called programmatically) is ignored; consider honoring an explicitcompactsetting when present and only defaulting totruewhen neithercompactnorfullis provided.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `normalizeActivityOptions`, the returned `compact` flag is derived solely from `options.full` and always defaults to `true`, which means any explicit `options.compact` value (e.g., `false` when called programmatically) is ignored; consider honoring an explicit `compact` setting when present and only defaulting to `true` when neither `compact` nor `full` is provided.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/cli/extension-command-options.ts`:
- Around line 258-266: The loop over collectLooseOptionKeys(definition)
currently deletes alias entries even when the canonical key already exists,
which can drop user-provided alias values; update the logic in that loop (the
block referencing collectLooseOptionKeys(definition), canonical, and coerced) so
you only assign coerced[canonical] = coerced[key] and delete coerced[key] when
the canonical key does not already exist, and if coerced already has the
canonical key simply skip both the assignment and the deletion so alias values
are preserved (i.e., do not delete coerced[key] when Object.hasOwn(coerced,
canonical) is true).
In `@src/cli/main.ts`:
- Around line 576-592: The code returns exact flag definitions immediately
(exact variable) which prevents finding more specific nested flags from
commandArgs; modify the logic in the function that calls
collectExtensionFlagDefinitionsForCommand so that when exact.length > 0 you
still iterate commandArgs to look for nested matches (respecting the existing
arg.startsWith("-") break) and return nested if found, otherwise fall back to
exact; reference the exact and nested variables, commandPath and commandArgs,
and the collectExtensionFlagDefinitionsForCommand call to locate and implement
this change.
🪄 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: c8889a17-a323-4ca7-b06d-f011114bd408
📒 Files selected for processing (33)
.agents/pm/features/pm-rnpb.toon.agents/pm/history/pm-3pbs.jsonl.agents/pm/history/pm-rnpb.jsonl.agents/pm/issues/pm-3pbs.toondocs/ARCHITECTURE.mddocs/COMMANDS.mdsrc/cli/commands/completion.tssrc/cli/commands/gc.tssrc/cli/commands/history.tssrc/cli/extension-command-options.tssrc/cli/help-content.tssrc/cli/main.tssrc/cli/register-list-query.tssrc/cli/registration-helpers.tssrc/mcp/server.tssrc/sdk/cli-contracts.tstests/integration/cli.integration.spec.tstests/integration/mcp-dynamic-package-actions.spec.tstests/integration/release-readiness-runtime.spec.tstests/unit/beads-command.spec.tstests/unit/claim-command.spec.tstests/unit/close-command.spec.tstests/unit/contracts-command.spec.tstests/unit/create-command.spec.tstests/unit/delete-command.spec.tstests/unit/gc-command.spec.tstests/unit/get-append-command.spec.tstests/unit/history-activity-command.spec.tstests/unit/main-loose-options.spec.tstests/unit/restore-command.spec.tstests/unit/test-command.spec.tstests/unit/todos-extension.spec.tstests/unit/update-command.spec.ts
unbraind
force-pushed
the
audit/token-output-activity-history-2026-05-20
branch
from
50d8064 to
134eac5
Compare
- default history/activity CLI output to compact projections with --full for audit payloads - align completions, contracts, MCP activity handling, and docs with compact/full projections - clean semantic search gc artifacts and reject unsupported dynamic extension flags - record full manual audit findings and close pm-3pbs with 100% coverage evidence Verification: - pnpm build - PM_RUN_TESTS_SKIP_BUILD=1 node scripts/run-tests.mjs test -- tests/unit/main-loose-options.spec.ts tests/integration/cli.integration.spec.ts tests/unit/contracts-command.spec.ts tests/unit/history-activity-command.spec.ts tests/unit/gc-command.spec.ts tests/integration/mcp-dynamic-package-actions.spec.ts --reporter=dot - node scripts/run-tests.mjs coverage - pnpm quality:static - node scripts/check-secrets.mjs
unbraind
force-pushed
the
audit/token-output-activity-history-2026-05-20
branch
from
134eac5 to
9288cc8
Compare
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.
Summary
Verification
Notes
Summary by Sourcery
Default history and activity outputs to compact, token-efficient projections while adding explicit full-output options, tightening extension flag handling, and extending garbage collection to cover new semantic search artifacts.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: