Repository navigation
feat(package): scaffold search capability starters - #333
Conversation
Add pm package/extension init --capability search so package authors get a runnable starter command, search provider, vector-store adapter, and SDK testing coverage. Normalize generated scaffold command paths from hyphenated directory names to space-separated command words so installed starter commands can be invoked manually after package install. Update docs, completions, CHANGELOG, and PM evidence for pm-pwai.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
@codex review |
|
Warning Review limit reached
More reviews will be available in 35 minutes and 43 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 To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds ChangesSearch Capability Scaffold + Command Name Normalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ 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 adds a new "search" capability to the package and extension scaffolding commands (pm package init and pm extension init), enabling authors to scaffold a deterministic in-memory search provider and vector-store adapter with runnable SDK tests. Additionally, it normalizes generated command paths to use space-separated words instead of hyphens to prevent dynamic dispatch failures in Commander. Feedback on the changes highlights a redundant optional chaining guard on document.metadata in the generated search provider query logic, which should be simplified for consistency.
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.
| " const needle = context.query.toLowerCase();", | ||
| " const hits = context.documents", | ||
| " .filter((document) => {", | ||
| " const title = String(document.metadata?.title ?? \"\").toLowerCase();", |
There was a problem hiding this comment.
Avoid adding redundant defensive checks (such as optional chaining guards) for fields that are typed as non-optional and guaranteed to be populated by the system. Since document.metadata is guaranteed to be populated (and is accessed unguarded as document.metadata.id in the subsequent .map call), the optional chaining operator ?. on metadata is redundant and introduces inconsistency.
const title = String(document.metadata.title ?? "").toLowerCase();References
- Avoid adding redundant defensive checks (such as null/undefined guards or type checks) for fields that are typed as non-optional and guaranteed to be populated by the system. Doing so introduces unreachable branches and inconsistency with the rest of the codebase where the field is consumed unguarded.
There was a problem hiding this comment.
Code Review
This pull request introduces the search capability to pm package init and pm extension init, allowing developers to scaffold starter packages with a registered search provider and vector-store adapter. It also normalizes generated command names to use space-separated words instead of hyphens to ensure compatibility with Commander's dynamic dispatch. The feedback suggests removing a redundant optional chaining check on document.metadata when retrieving the document title, as metadata is guaranteed to be populated.
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.
| " const needle = context.query.toLowerCase();", | ||
| " const hits = context.documents", | ||
| " .filter((document) => {", | ||
| " const title = String(document.metadata?.title ?? \"\").toLowerCase();", |
There was a problem hiding this comment.
Avoid adding redundant optional chaining (?.) on document.metadata when accessing title, as metadata is typed as non-optional and guaranteed to be populated by the system. This is consistent with how document.metadata.id is accessed unguarded later in the same function.
const title = String(document.metadata.title ?? "").toLowerCase();References
- Avoid adding redundant defensive checks (such as null/undefined guards or type checks) for fields that are typed as non-optional and guaranteed to be populated by the system. Doing so introduces unreachable branches and inconsistency with the rest of the codebase where the field is consumed unguarded.
Address Gemini review feedback by removing redundant optional chaining from the generated search-provider title read. Record post-review verification on pm-pwai, including focused tests, build, static quality, and full 100/100/100/100 coverage.
|
@codex review Re-triggering after follow-up commit ba54807 addressed Gemini feedback. |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new --capability search option to the pm package init and pm extension init commands, enabling developers to scaffold starter packages with an in-memory search provider, a vector-store adapter, and runnable SDK tests. Additionally, it normalizes generated command names by replacing hyphens with spaces to prevent dynamic dispatch failures in Commander, and updates shell completions, help content, and documentation. The review feedback suggests refactoring nested conditional blocks and ternary operators in scaffold.ts to simplify the code, as well as removing redundant defensive checks on the guaranteed title metadata field.
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.
| if (capability !== "hooks") { | ||
| return commandLines; | ||
| if (capability !== "search") { | ||
| return commandLines; | ||
| } | ||
| return [ | ||
| ...commandLines, | ||
| "", | ||
| " // Search providers let packages customize how pm ranks and retrieves", | ||
| " // project context. This starter is deterministic and dependency-free:", | ||
| " // replace the scoring with your domain retrieval, embedding, or rerank", | ||
| " // logic as the package grows.", | ||
| " api.registerSearchProvider({", | ||
| ` name: ${JSON.stringify(searchProviderName)},`, | ||
| " query: async (context) => {", | ||
| " const needle = context.query.toLowerCase();", | ||
| " const hits = context.documents", | ||
| " .filter((document) => {", | ||
| " const title = String(document.metadata.title ?? \"\").toLowerCase();", | ||
| " return title.includes(needle);", | ||
| " })", | ||
| " .map((document) => ({", | ||
| " id: document.metadata.id,", | ||
| " score: 1,", | ||
| ' matched_fields: ["title"],', | ||
| " }));", | ||
| " return { hits };", | ||
| " },", | ||
| " embed: async (context) => [context.input.length],", | ||
| " });", | ||
| "", | ||
| " // Vector-store adapters let packages own semantic index storage. This", | ||
| " // starter returns a stable in-memory hit so generated tests can exercise", | ||
| " // the adapter without external services.", | ||
| " api.registerVectorStoreAdapter({", | ||
| ` name: ${JSON.stringify(vectorAdapterName)},`, | ||
| " query: async (context) => [{ id: \"starter-vector-hit\", score: context.limit }],", | ||
| " upsert: async (context) => ({ upserted: context.points.length }),", | ||
| " delete: async (context) => ({ deleted: context.ids.length }),", | ||
| " });", | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Refactor the nested if statements into a cleaner structure and remove the redundant defensive checks on document.metadata.title. Since title is a non-optional string guaranteed to be populated by the system, using String(document.metadata.title ?? "") is unnecessary and introduces unreachable branches.
if (capability === "commands") {
return commandLines;
}
if (capability === "search") {
return [
...commandLines,
"",
" // Search providers let packages customize how pm ranks and retrieves",
" // project context. This starter is deterministic and dependency-free:",
" // replace the scoring with your domain retrieval, embedding, or rerank",
" // logic as the package grows.",
" api.registerSearchProvider({",
" name: " + JSON.stringify(searchProviderName) + ",",
" query: async (context) => {",
" const needle = context.query.toLowerCase();",
" const hits = context.documents",
" .filter((document) => {",
" const title = document.metadata.title.toLowerCase();",
" return title.includes(needle);",
" })",
" .map((document) => ({",
" id: document.metadata.id,",
" score: 1,",
" matched_fields: [\"title\"],",
" }));",
" return { hits };",
" },",
" embed: async (context) => [context.input.length],",
" });",
"",
" // Vector-store adapters let packages own semantic index storage. This",
" // starter returns a stable in-memory hit so generated tests can exercise",
" // the adapter without external services.",
" api.registerVectorStoreAdapter({",
" name: " + JSON.stringify(vectorAdapterName) + ",",
" query: async (context) => [{ id: \"starter-vector-hit\", score: context.limit }],",
" upsert: async (context) => ({ upserted: context.points.length }),",
" delete: async (context) => ({ deleted: context.ids.length }),",
" });",
];
}References
- Avoid adding redundant defensive checks (such as null/undefined guards or type checks) for fields that are typed as non-optional and guaranteed to be populated by the system.
| const capabilitiesLiteral = hooksEnabled | ||
| ? '["commands", "hooks"]' | ||
| : searchEnabled | ||
| ? '["commands", "search"]' | ||
| : '["commands"]'; |
There was a problem hiding this comment.
Simplify the nested ternary operators by directly interpolating the capability variable when it is not 'commands'.
const capabilitiesLiteral = capability === "commands"
? '["commands"]'
: '["commands", "' + capability + '"]';| const capabilities = | ||
| capability === "hooks" ? ["commands", "hooks"] : capability === "search" ? ["commands", "search"] : ["commands"]; |
There was a problem hiding this comment.
Simplify the nested ternary operators by using a direct check on 'commands' and appending the capability.
| const capabilities = | |
| capability === "hooks" ? ["commands", "hooks"] : capability === "search" ? ["commands", "search"] : ["commands"]; | |
| const capabilities = | |
| capability === "commands" ? ["commands"] : ["commands", capability]; |
|
@codex review Re-triggering after follow-up commit e81788f addressed Gemini feedback by simplifying scaffold branching and generated metadata handling. |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces the search capability to the package and extension scaffolding system (pm package init --capability search). This scaffolds a starter package or extension containing a deterministic, in-memory search provider and vector-store adapter pair, along with runnable SDK tests. Additionally, the PR normalizes generated command names to use space-separated words instead of hyphenated names to prevent dynamic dispatch failures in Commander. Shell completions, help content, and documentation have been updated accordingly, and new unit tests have been added to verify the scaffolding output and completions. There are no review comments, so I have no feedback to provide.
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.
|
@codex review Re-triggering after follow-up commit a0b31d8 fixed the current-head Gates (smokes) failure by aligning dogfood-package-first with the normalized scaffold command path. |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new --capability search option to the pm package init and pm extension init commands, allowing users to scaffold search provider and vector-store adapter starter packages with runnable SDK tests. It also normalizes generated command names by replacing hyphens with spaces to prevent dynamic dispatch failures in Commander. Feedback on the changes points out a potential runtime crash in the scaffolded search provider if a document's metadata lacks a title property, suggesting the use of optional chaining and a fallback value.
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.
| " const needle = context.query.toLowerCase();", | ||
| " const hits = context.documents", | ||
| " .filter((document) => {", | ||
| " const title = document.metadata.title.toLowerCase();", |
There was a problem hiding this comment.
In the scaffolded search provider, document.metadata.title is accessed and .toLowerCase() is called directly on it. However, document metadata is dynamic, and some documents might not have a title property (or it might be null/undefined). This would cause a runtime TypeError: Cannot read properties of undefined (reading 'toLowerCase') and crash the search provider.
To make the scaffolded code robust and production-ready, we should safely access the title property using optional chaining and a fallback value.
const title = (document.metadata?.title ?? "").toLowerCase();![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/help-content.ts`:
- Around line 213-215: The example paths in the install commands are
inconsistent with their preceding init examples. In the first flow, the init
command uses "./my-search-package" but the install command uses "./my-package".
Similarly, in the second flow (lines 253-255), the paths do not align. Update
the install example paths to match the package names and paths established by
their corresponding preceding init examples so users can follow the examples
sequentially without confusion.
In `@tests/unit/extensions/extension-command.spec.ts`:
- Line 2196: There is a duplicated expect assertion on line 2196 in the test
file that matches the expect call checking scaffold.details files mapping.
Locate both instances of the identical expect((scaffold.details as { files?:
Array<{ path: string }> }).files?.map((file) => file.path)).toEqual([ assertion
and remove one of the duplicate lines to ensure the test parses correctly with a
single matched assertion pair.
🪄 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
Run ID: be399da6-a1d4-4409-bedd-67e1080949ee
📒 Files selected for processing (12)
.agents/pm/features/pm-pwai.toon.agents/pm/history/pm-pwai.jsonlCHANGELOG.mddocs/EXTENSIONS.mddocs/SDK.mdscripts/dogfood-package-first.mjssrc/cli/commands/completion.tssrc/cli/commands/extension/scaffold.tssrc/cli/help-content.tstests/unit/commands/completion-command.spec.tstests/unit/extensions/extension-command.spec.tstests/unit/scripts/dogfood-package-first.spec.ts
| command: "starter search ext ping", | ||
| }, | ||
| }); | ||
| expect((scaffold.details as { files?: Array<{ path: string }> }).files?.map((file) => file.path)).toEqual([ |
There was a problem hiding this comment.
Remove the duplicated assertion line (test parse break risk).
Line 2196 appears duplicated with the same expect(...).toEqual([ call. If both exist in the file, this leaves an unmatched call and breaks test parsing.
Proposed fix
- expect((scaffold.details as { files?: Array<{ path: string }> }).files?.map((file) => file.path)).toEqual([
expect((scaffold.details as { files?: Array<{ path: string }> }).files?.map((file) => file.path)).toEqual([🤖 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 `@tests/unit/extensions/extension-command.spec.ts` at line 2196, There is a
duplicated expect assertion on line 2196 in the test file that matches the
expect call checking scaffold.details files mapping. Locate both instances of
the identical expect((scaffold.details as { files?: Array<{ path: string }>
}).files?.map((file) => file.path)).toEqual([ assertion and remove one of the
duplicate lines to ensure the test parses correctly with a single matched
assertion pair.
|
@codex review Re-triggering after follow-up commit 3a2dffb addressed current-head review feedback: aligned help examples and made generated search title access defensive. CodeRabbit duplicate assertion note was verified as a false positive against the current file and covered by focused tests. |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces the --capability search option for pm package init and pm extension init, allowing developers to scaffold starter packages with an in-memory search provider and vector-store adapter along with runnable SDK tests. It also normalizes generated command paths from hyphenated names to space-separated words to ensure proper dynamic dispatch. Documentation, shell completions, and unit tests have been updated accordingly. Feedback on the changes suggests removing a redundant optional chaining guard on document.metadata in the generated search provider code to maintain consistency with other direct property accesses.
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.
| " const needle = context.query.toLowerCase();", | ||
| " const hits = context.documents", | ||
| " .filter((document) => {", | ||
| " const title = String(document.metadata?.title ?? \"\").toLowerCase();", |
There was a problem hiding this comment.
According to the general rules, we should avoid adding redundant defensive checks (such as optional chaining guards) for fields that are guaranteed to be populated by the system. Since document.metadata is consumed unguarded on line 177 (document.metadata.id), using optional chaining document.metadata?.title on line 173 is redundant and inconsistent. Please remove the optional chaining on metadata to keep the property access consistent and avoid unreachable branches.
const title = String(document.metadata.title ?? "").toLowerCase();References
- Avoid adding redundant defensive checks (such as null/undefined guards or type checks) for fields that are typed as non-optional and guaranteed to be populated by the system.
|
@codex review Re-triggering after follow-up commit 397389d removed optional chaining on the guaranteed metadata object while keeping a title fallback for dynamic document metadata. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
Summary
pm package/extension init --capability searchstarter generation with a command, search provider, vector-store adapter, and generated SDKnode:testcoveragepm-pwaiVerification
PM_RUN_TESTS_SKIP_BUILD=1 node scripts/run-tests.mjs test -- tests/unit/extensions/extension-command.spec.ts tests/unit/commands/completion-command.spec.ts tests/unit/commands/contracts-command.spec.ts tests/unit/sdk/sdk-index.spec.ts=> 4 files, 309 tests passedpnpm buildpnpm contracts:checkpnpm quality:staticpnpm security:scan/tmp/tmp.hP0BuKj8Gw:pm init,pm package init ./search-package --capability search, local SDK install,npm test,pm install, package doctor deep trace warning_count=0,pm search package ping --jsonnode scripts/run-tests.mjs coverage=> 242 files, 4,832 tests, 100/100/100/100pnpm changelog:pm:checkpm health --check-only --summary --jsonpm validate --check-resolution --check-history-drift=> history_drift ok; pre-existing resolution warnings remain on unrelated closed itemspnpm audit --audit-level low