Skip to content

Adding prompt tool with first 9 prompts for onboarding#233

Merged
wonderwhy-er merged 18 commits into
mainfrom
onboarding
Sep 10, 2025
Merged

Adding prompt tool with first 9 prompts for onboarding#233
wonderwhy-er merged 18 commits into
mainfrom
onboarding

Conversation

@wonderwhy-er

@wonderwhy-er wonderwhy-er commented Sep 2, 2025

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Added guided onboarding with a browsable catalog of 9 starter prompts, category listing, and prompt retrieval.
    • Smart, state-aware onboarding messages and reminders for first-time users.
    • Onboarding data bundled into builds for immediate availability.
  • Documentation

    • Expanded Getting Started guidance in README and FAQ.
    • Added “What’s next?” onboarding hints on the docs site.
  • Chores

    • Ignored local planning/ directory in version control.
  • Tests

    • Removed legacy binary file detection test suite.

@coderabbitai

coderabbitai Bot commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Adds an onboarding prompts feature: new prompts data, a prompts tool and schema, server integration and telemetry for prompt actions, onboarding state and usage tracking, build changes to include data in dist, documentation updates, .gitignore update, and removal of a binary-file detection test.

Changes

Cohort / File(s) Summary
Build packaging
package.json
Build now creates dist/data and copies src/data/onboarding-prompts.json into distribution alongside existing copied assets and chmod steps.
Onboarding prompts data
src/data/onboarding-prompts.json
New JSON catalog (v1.0.0) with 9 onboarding prompts and associated metadata (id, title, description, prompt, categories, secondaryTag, icon, author, verified, votes, gaClicks).
Server integration
src/server.ts
Adds get_prompts tool to ListTools and CallToolRequest; dispatches to prompts helper, emits telemetry for get/list actions and errors, injects onboarding messages post-call when gated, and marks prompts used.
Prompts tool implementation
src/tools/prompts.ts
New module exposing loadPromptsData() and getPrompts(params) with in-memory caching; supports list_categories, list_prompts, and get_prompt; formats outputs and records prompt usage via usageTracker.
Schemas
src/tools/schemas.ts
Adds GetPromptsArgsSchema (zod schema with action: 'list_categories'
Usage tracking & onboarding
src/utils/usageTracker.ts
Adds OnboardingState interface, onboarding persistence and gating methods (getOnboardingState, saveOnboardingState, shouldShowOnboarding, getOnboardingMessage, markOnboardingShown, markOnboardingPromptsUsed, markPromptUsed, resetOnboardingState) and exports singleton usageTracker.
Documentation
README.md, FAQ.md, docs/index.html
Adds Getting Started / onboarding content and “What’s next?” blocks; some content duplicated across files.
VCS ignore config
.gitignore
Adds ignore rule for planning/ directory.
Tests
test/test-binary-file-detection.js
Removes the binary-file detection test suite (file deleted).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Client
  participant Server
  participant Prompts as "tools/prompts.ts"
  participant Usage as "usageTracker"
  participant Data as "src/data/onboarding-prompts.json"

  User->>Client: Request prompts (list/get)
  Client->>Server: CallToolRequest(name:get_prompts, args)
  Server->>Prompts: getPrompts(args)
  Prompts->>Data: loadPromptsData()
  Data-->>Prompts: PromptsData
  alt action == get_prompt
    Prompts->>Usage: markPromptUsed(promptId, category)
    Prompts-->>Server: Prompt response
    Server->>Server: emit server_get_prompt telemetry
  else list_categories / list_prompts
    Prompts-->>Server: List/Category response
    Server->>Server: emit list/category telemetry
  end
  Server-->>Client: Tool result
  opt After successful call
    Server->>Usage: shouldShowOnboarding()
    alt true
      Server->>Usage: getOnboardingMessage()
      Server->>Usage: markOnboardingShown(variant)
      Server-->>Client: Inject onboarding message into LLM response
    end
    opt onboarding prompts used
      Server->>Usage: markOnboardingPromptsUsed()
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

size:L

Suggested reviewers

  • serg33v

Poem

A hop, a nibble, prompts in tow,
Nine tiny carrots all in a row.
I tuck them in /data, tidy and bright,
The server hums back—guidance takes flight.
Thump-thump, onboarding—welcome delight! 🥕

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3a2d0c and be0aedc.

📒 Files selected for processing (2)
  • src/server.ts (5 hunks)
  • src/tools/prompts.ts (1 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch onboarding

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (5)
package.json (1)

30-30: Build step LGTM; consider copying all data JSONs.

If more prompt catalogs are added, copy them generically to avoid future script edits.

-    "build": "tsc && shx cp setup-claude-server.js uninstall-claude-server.js track-installation.js dist/ && shx chmod +x dist/*.js && shx mkdir -p dist/data && shx cp src/data/onboarding-prompts.json dist/data/",
+    "build": "tsc && shx cp setup-claude-server.js uninstall-claude-server.js track-installation.js dist/ && shx chmod +x dist/*.js && shx mkdir -p dist/data && shx cp src/data/*.json dist/data/"
src/utils/usageTracker.ts (1)

356-363: Replace console log with structured analytics event.

Leverage existing capture() so usage is observable and queryable.

   async markPromptUsed(promptId: string, category: string): Promise<void> {
-    // This could be expanded later to track detailed prompt usage
-    // For now, we'll just rely on the capture analytics
-    console.log(`[PROMPT USAGE] User retrieved prompt: ${promptId} (category: ${category})`);
+    await capture('prompt_used', {
+      prompt_id: promptId,
+      category,
+    });
   }
src/tools/schemas.ts (1)

124-131: Make args validation stricter with a discriminated union.

Enforces promptId only for get_prompt at the schema level.

-export const ListSearchesArgsSchema = z.object({});
+export const ListSearchesArgsSchema = z.object({});
 
-// Prompts tool schema
-export const GetPromptsArgsSchema = z.object({
-  action: z.enum(['list_categories', 'list_prompts', 'get_prompt']),
-  category: z.string().optional(),
-  promptId: z.string().optional(),
-});
+// Prompts tool schema
+export const GetPromptsArgsSchema = z.discriminatedUnion('action', [
+  z.object({ action: z.literal('list_categories') }),
+  z.object({
+    action: z.literal('list_prompts'),
+    category: z.string().optional(),
+  }),
+  z.object({
+    action: z.literal('get_prompt'),
+    promptId: z.string(),
+  }),
+]);
src/server.ts (1)

681-710: Consider exposing prompts via MCP prompts capability too.

Keeping get_prompts is fine; additionally wiring real prompts into ListPromptsRequest could let clients render them natively.

src/tools/prompts.ts (1)

321-335: Tone nit: don’t promise auto-execution.

If the tool doesn’t actually auto-run, soften the claim to avoid user confusion.

-  response += `## Ready to Use This Prompt\nThe prompt below is ready to use. I'll start executing it right away:\n\n`;
+  response += `## Ready to Use This Prompt\nThe prompt below is ready to use. I can execute it on your confirmation:\n\n`;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e0c91f5 and 6014f2d.

📒 Files selected for processing (6)
  • package.json (1 hunks)
  • src/data/onboarding-prompts.json (1 hunks)
  • src/server.ts (3 hunks)
  • src/tools/prompts.ts (1 hunks)
  • src/tools/schemas.ts (1 hunks)
  • src/utils/usageTracker.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/tools/prompts.ts (3)
src/types.ts (1)
  • ServerResult (51-55)
src/utils/capture.ts (1)
  • capture (277-284)
src/utils/usageTracker.ts (1)
  • usageTracker (367-367)
src/server.ts (2)
src/tools/schemas.ts (1)
  • GetPromptsArgsSchema (127-131)
src/tools/prompts.ts (1)
  • getPrompts (65-129)
🔇 Additional comments (7)
src/data/onboarding-prompts.json (1)

1-114: No issues detected—prompt IDs are unique and all prompts include the required fields.

src/server.ts (3)

46-47: Import looks good.


51-51: Prompts handler import LGTM.


777-788: Execution path LGTM with consistent error handling.

src/tools/prompts.ts (3)

154-158: Confirm renderer interprets Markdown for type "text".

Responses are Markdown-formatted but use { type: "text" }. Verify the client renders Markdown for this type; otherwise switch to the appropriate content type.

Also applies to: 188-193, 229-233


260-276: LGTM on categories presentation.

Clear summary with counts and guidance.


48-49: Data file packaging verified. The build script in package.json copies src/data/onboarding-prompts.json into dist/data/, matching the runtime path (dist/data/onboarding-prompts.json); no changes needed.

Comment thread src/tools/prompts.ts
Comment thread src/tools/prompts.ts
Comment on lines +70 to +78
if (!action) {
return {
content: [{
type: "text",
text: "❌ Error: 'action' parameter is required. Use 'list_categories', 'list_prompts', or 'get_prompt'"
}],
isError: true
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate action value before emitting analytics.

Guard against arbitrary action strings to keep analytics clean and return early on invalid input.

     if (!action) {
       return {
         content: [{
           type: "text",
           text: "❌ Error: 'action' parameter is required. Use 'list_categories', 'list_prompts', or 'get_prompt'"
         }],
         isError: true
       };
     }
+
+    const validActions = new Set(['list_categories','list_prompts','get_prompt']);
+    if (!validActions.has(action as any)) {
+      return {
+        content: [{ type: "text", text: "❌ Error: Invalid action. Use 'list_categories', 'list_prompts', or 'get_prompt'" }],
+        isError: true
+      };
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!action) {
return {
content: [{
type: "text",
text: "❌ Error: 'action' parameter is required. Use 'list_categories', 'list_prompts', or 'get_prompt'"
}],
isError: true
};
}
if (!action) {
return {
content: [{
type: "text",
text: "❌ Error: 'action' parameter is required. Use 'list_categories', 'list_prompts', or 'get_prompt'"
}],
isError: true
};
}
const validActions = new Set(['list_categories', 'list_prompts', 'get_prompt']);
if (!validActions.has(action as any)) {
return {
content: [{
type: "text",
text: "❌ Error: Invalid action. Use 'list_categories', 'list_prompts', or 'get_prompt'"
}],
isError: true
};
}
🤖 Prompt for AI Agents
In src/tools/prompts.ts around lines 70-78, the current code only checks for
missing action but allows any string and may emit analytics for invalid values;
add a whitelist check against allowed actions
['list_categories','list_prompts','get_prompt'] immediately after the existing
null/undefined check and return the same error response (isError: true with the
message) if the action is not one of those values, ensuring no analytics are
emitted for invalid actions.

Comment thread src/tools/prompts.ts Outdated
Comment on lines +80 to +86
// Track analytics for tool usage
await capture(`prompts_tool_${action}`, {
category: category,
prompt_id: promptId,
has_category_filter: !!category
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Don’t let telemetry or usage tracking block or fail the tool.

Make analytics fire-and-forget and swallow errors, especially in the catch block.

-    await capture(`prompts_tool_${action}`, {
+    void capture(`prompts_tool_${action}`, {
       category: category,
       prompt_id: promptId,
       has_category_filter: !!category
-    });
+    }).catch(() => {});
-    await capture('prompts_tool_error', {
+    void capture('prompts_tool_error', {
       error_message: error instanceof Error ? error.message : String(error),
       action: params?.action
-    });
+    }).catch(() => {});
-  await capture('prompt_retrieved', {
+  void capture('prompt_retrieved', {
     prompt_id: promptId,
     prompt_title: prompt.title,
     category: prompt.categories[0] || 'uncategorized',
     author: prompt.author,
     verified: prompt.verified
-  });
+  }).catch(() => {});
 
-  await usageTracker.markPromptUsed(promptId, prompt.categories[0] || 'uncategorized');
+  void usageTracker.markPromptUsed(promptId, prompt.categories[0] || 'uncategorized').catch(() => {});

Also applies to: 115-120, 214-225

🤖 Prompt for AI Agents
In src/tools/prompts.ts around lines 80-86 (and also update the similar calls at
115-120 and 214-225): the telemetry capture calls currently use await and can
block or propagate failures; change them to fire-and-forget by removing the
await and appending a rejection handler (e.g., call capture(...).catch(()=>{})
or use void capture(...).catch(()=>{})) so errors are swallowed and telemetry
cannot block or fail the tool.

Comment thread src/tools/prompts.ts
Comment on lines +164 to +184
async function listPrompts(category?: string): Promise<ServerResult> {
const data = await loadPromptsData();

let filteredPrompts = data.prompts;

// Filter by category if specified
if (category) {
filteredPrompts = data.prompts.filter(prompt =>
prompt.categories.includes(category)
);

if (filteredPrompts.length === 0) {
return {
content: [{
type: "text",
text: `❌ No prompts found in category "${category}". Use action='list_categories' to see available categories.`
}],
isError: true
};
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Make category filtering tolerant (trim + case-insensitive).

Prevents “No prompts found” due to whitespace/case mismatches.

-  let filteredPrompts = data.prompts;
+  let filteredPrompts = data.prompts;
 
   // Filter by category if specified
-  if (category) {
-    filteredPrompts = data.prompts.filter(prompt => 
-      prompt.categories.includes(category)
-    );
+  const normalizedCategory = category?.trim().toLowerCase();
+  if (normalizedCategory) {
+    filteredPrompts = data.prompts.filter(prompt =>
+      prompt.categories.some(c => c.toLowerCase() === normalizedCategory)
+    );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/tools/prompts.ts around lines 164 to 184, the category filter is strict
and can miss matches due to whitespace or casing; modify it to normalize both
the incoming category and each prompt's categories by trimming and lowercasing
before comparing (e.g., compute const normalizedCategory =
category.trim().toLowerCase() then filter prompts where prompt.categories.some(c
=> c.trim().toLowerCase() === normalizedCategory)); keep the early return
behavior when no matches but ensure you only call .trim()/.toLowerCase() after
confirming category is non-empty.

Comment thread src/tools/prompts.ts
- Add OnboardingState interface to usageTracker
- Implement shouldShowOnboarding() logic (triggers after 3-10 successful calls)
- Add onboarding message injection to server.ts (before feedback prompts)
- Track onboarding analytics: message_shown, message_accepted
- Auto-detect onboarding acceptance when user calls get_prompts
- Use Variant A: Welcome & Guidance message for first-time users
- Complete analytics pipeline for cohort analysis and A/B testing
…for accuracy

- Changed onboarding_message_accepted to user_asked_for_prompts
- Renamed onboardingAccepted to onboardingUsedPrompts in state
- Updated method names to be more accurate
- User may call get_prompts for reasons other than responding to onboarding invitation
- get_prompts calls are already captured by standard tool call analytics
- prompts_tool_* events provide specific action tracking
- Simplified to just update state for onboarding flow control
- Eliminates duplicate/redundant analytics data
- Removed redundant prompts_tool_* events from prompts.ts
- Added get_prompts parameters (action, category, prompt_id) to server_call_tool event
- Added detailed prompt_retrieved event for specific prompt usage
- Cleaner analytics with all prompts data in standard tool call events
- Exported loadPromptsData function for server analytics use
- Changed onboarding_message_shown -> server_onboarding_shown
- Changed prompt_retrieved -> server_prompt_retrieved
- Follows established naming convention:
  - package_* for npm/installation events
  - server_* for server-side operations
  - Consistent with server_call_tool, server_request_error patterns

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

102-105: Auto-Updates text is self-contradictory

“✅ Auto-Updates: Yes - requires manual updates” is unclear. If Option 2 (bash) truly auto-updates on restart, drop “requires manual updates”; if not, flip to “No”.

Apply one of these diffs:

-**✅ Auto-Updates:** Yes - requires manual updates  
+**✅ Auto-Updates:** Yes  

or

-**✅ Auto-Updates:** Yes - requires manual updates  
+**❌ Auto-Updates:** No - re-run the installer to update  
♻️ Duplicate comments (2)
src/tools/prompts.ts (2)

170-184: Make category filtering tolerant (trim + case-insensitive)

Prevents “No prompts found” due to whitespace/case mismatches.

-  if (category) {
-    filteredPrompts = data.prompts.filter(prompt => 
-      prompt.categories.includes(category)
-    );
+  const normalizedCategory = category?.trim().toLowerCase();
+  if (normalizedCategory) {
+    filteredPrompts = data.prompts.filter(prompt =>
+      prompt.categories.some(c => c.toLowerCase() === normalizedCategory)
+    );

56-66: Harden JSON parsing with shape validation (truthy check is ineffective)

Validate the parsed structure before caching to prevent malformed data from slipping through.

   try {
     const dataPath = path.join(__dirname, '..', 'data', 'onboarding-prompts.json');
     const fileContent = await fs.readFile(dataPath, 'utf-8');
-    cachedPromptsData = JSON.parse(fileContent);
-    
-    if (!cachedPromptsData) {
-      throw new Error('Failed to parse prompts data');
-    }
+    const parsed = JSON.parse(fileContent);
+    if (!parsed || typeof parsed !== 'object' || !Array.isArray((parsed as any).prompts)) {
+      throw new Error('Invalid prompts data: expected { prompts: Prompt[] }');
+    }
+    cachedPromptsData = parsed as PromptsData;
🧹 Nitpick comments (13)
FAQ.md (1)

176-193: Onboarding guidance is concise and aligned

Reads well and matches the described flow (auto + manual + examples). Consider adding a one-liner hint that these are powered by the new get_prompts tool to connect docs to UX, but optional.

README.md (1)

147-147: Typo: duplicated heading markers

### ### Option 5### Option 5.

-### ### Option 5: Checkout locally ❌ **Manual Updates** **Requires Node.js** ❌ **Manual Updates** **Requires Node.js**
+### Option 5: Checkout locally ❌ **Manual Updates** **Requires Node.js** ❌ **Manual Updates** **Requires Node.js**
test_onboarding.md (2)

37-43: Markdown lint: add a language to this fenced block

Label the debug output block to satisfy MD040 and improve rendering.

-```
+```text
 [ONBOARDING DEBUG] Should show onboarding: true
 ...

---

`64-81`: **Production/test delay toggling should be config-driven**

Instead of editing code to switch delays, consider reading delays from config or env (e.g., ONBOARDING_DELAY_MS list) so tests can override without code changes.

</blockquote></details>
<details>
<summary>.serena/project.yml (1)</summary><blockquote>

`25-25`: **Trailing space in comment**

YAML linters flag this. Remove trailing spaces on Line 25.


```diff
-# To make sure you have the latest list of tools, and to view their descriptions, 
+# To make sure you have the latest list of tools, and to view their descriptions,
src/data/onboarding-prompts.json (1)

1-4: Add a top-level “lastUpdated” field (optional)

A timestamp helps consumers handle cache invalidation/versioning without bumping semantic version for content-only tweaks.

src/server.ts (2)

747-747: Make telemetry fire-and-forget to avoid latency and unhandled rejections

Current capture calls can block the request path or generate unhandled promise rejections. Prefer non-blocking calls with explicit rejection swallowing.

Apply diffs (representative spots; repeat the pattern for other capture(...) calls in this file):

-        capture_call_tool('server_call_tool', telemetryData);
+        void capture_call_tool('server_call_tool', telemetryData).catch(() => {});
-                                await capture('server_prompt_retrieved', {
+                                void capture('server_prompt_retrieved', {
                                     prompt_id: prompt.id,
                                     prompt_title: prompt.title,
                                     category: prompt.categories[0] || 'uncategorized',
                                     author: prompt.author,
                                     verified: prompt.verified
-                                });
+                                }).catch(() => {});
-        capture('server_request_error', {
+        void capture('server_request_error', {
             error: errorMessage
-        });
+        }).catch(() => {});

Also applies to: 761-762, 772-773, 784-785, 805-812, 825-826, 927-927, 1030-1032


792-817: Avoid extra disk read; return prompt metadata from tool instead

The handler lazily imports and reads prompts data again just to emit analytics. Push prompt metadata through the tool response (e.g., set result._meta = { id, title, categories[0], author, verified }) during the get_prompt path and use that here. This removes redundant I/O and keeps concerns separated.

src/tools/prompts.ts (4)

214-216: Don’t block on analytics bookkeeping

Prompt usage tracking shouldn’t delay the user-visible response. Fire-and-forget and swallow errors.

-  await usageTracker.markPromptUsed(promptId, prompt.categories[0] || 'uncategorized');
+  void usageTracker.markPromptUsed(promptId, prompt.categories[0] || 'uncategorized').catch(() => {});

292-301: Sort within groups for stable, meaningful listing

Surface verified prompts first, then by votes, then alphabetically. Improves UX consistency.

   preferredOrder.forEach(tag => {
     if (groupedPrompts.has(tag)) {
-      const tagPrompts = groupedPrompts.get(tag)!;
+      const tagPrompts = groupedPrompts.get(tag)!.slice().sort((a, b) => {
+        if (a.verified !== b.verified) return a.verified ? -1 : 1;
+        if (b.votes !== a.votes) return b.votes - a.votes;
+        return a.title.localeCompare(b.title);
+      });
       response += `${tag}:\n`;
       tagPrompts.forEach(prompt => {
         response += `${promptNumber}. ${prompt.title}\n`;

306-317: Avoid leaking internal IDs in rendered text; prefer _meta mapping

The HTML comment trick may still surface in some clients. Consider returning the number→ID map in ServerResult._meta (e.g., _meta: { promptMap }) and omit the comment entirely from the user-facing text.


74-119: Optional: validate params with the shared Zod schema

Since this tool can be called programmatically too, parse params with GetPromptsArgsSchema to guarantee consistent validation regardless of the caller. Keep existing friendly error messages for missing/invalid action.

If you want, I can wire in GetPromptsArgsSchema.safeParse and preserve your current error strings.

src/utils/usageTracker.ts (1)

365-372: Defensive read of onboarding state

Persisted state can be partial or stale. Coalesce each field to defaults to avoid undefined math/logic later.

-    const stored = await configManager.getValue('onboardingState');
-    return stored || {
-      promptsUsed: false,
-      attemptsShown: 0,
-      lastShownAt: 0
-    };
+    const stored = await configManager.getValue('onboardingState');
+    return {
+      promptsUsed: !!stored?.promptsUsed,
+      attemptsShown: Number.isFinite(stored?.attemptsShown) ? stored.attemptsShown : 0,
+      lastShownAt: Number.isFinite(stored?.lastShownAt) ? stored.lastShownAt : 0
+    };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6014f2d and 9326c6c.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • .gitignore (1 hunks)
  • .serena/.gitignore (1 hunks)
  • .serena/memories/code_style_conventions.md (1 hunks)
  • .serena/memories/project_overview.md (1 hunks)
  • .serena/memories/suggested_commands.md (1 hunks)
  • .serena/memories/task_completion_checklist.md (1 hunks)
  • .serena/project.yml (1 hunks)
  • FAQ.md (2 hunks)
  • README.md (2 hunks)
  • config_backup_before_onboarding_test.json (1 hunks)
  • docs/index.html (1 hunks)
  • src/data/onboarding-prompts.json (1 hunks)
  • src/server.ts (5 hunks)
  • src/tools/prompts.ts (1 hunks)
  • src/utils/usageTracker.ts (2 hunks)
  • test/test-binary-file-detection.js (0 hunks)
  • test_onboarding.md (1 hunks)
💤 Files with no reviewable changes (1)
  • test/test-binary-file-detection.js
✅ Files skipped from review due to trivial changes (5)
  • .gitignore
  • .serena/memories/project_overview.md
  • .serena/memories/code_style_conventions.md
  • .serena/.gitignore
  • .serena/memories/task_completion_checklist.md
🧰 Additional context used
🧬 Code graph analysis (5)
.serena/memories/suggested_commands.md (1)
setup-claude-server.js (1)
  • setup (614-854)
src/utils/usageTracker.ts (1)
src/config-manager.ts (1)
  • configManager (219-219)
config_backup_before_onboarding_test.json (2)
src/config-manager.ts (1)
  • ConfigManager (29-216)
test/test-negative-offset-readfile.js (1)
  • setup (28-48)
src/server.ts (4)
src/tools/schemas.ts (1)
  • GetPromptsArgsSchema (127-131)
src/tools/prompts.ts (2)
  • getPrompts (74-129)
  • loadPromptsData (50-69)
src/utils/capture.ts (1)
  • capture (277-284)
src/utils/usageTracker.ts (2)
  • usageTracker (488-488)
  • shouldShowOnboarding (384-414)
src/tools/prompts.ts (2)
src/types.ts (1)
  • ServerResult (51-55)
src/utils/usageTracker.ts (1)
  • usageTracker (488-488)
🪛 YAMLlint (1.37.1)
.serena/project.yml

[error] 25-25: trailing spaces

(trailing-spaces)

🪛 markdownlint-cli2 (0.17.2)
test_onboarding.md

36-36: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (6)
docs/index.html (1)

1615-1618: Nice onboarding nudge—LGTM

Clear CTA that ties into the new prompts flow. No issues.

FAQ.md (1)

25-25: TOC entry added—LGTM

Anchor matches the section title below.

README.md (2)

25-25: TOC: Getting Started—LGTM


382-403: Getting Started matches the onboarding feature—LGTM

Good examples and user phrasing align with the prompts catalog.

.serena/memories/suggested_commands.md (1)

1-56: Developer quick-reference—LGTM

Handy and scoped to docs. No runtime impact.

src/data/onboarding-prompts.json (1)

1-123: Packaging and loader paths are correctly configured
The build script creates dist/data and copies src/data/onboarding-prompts.json there, and loadPromptsData uses path.join(__dirname, '..', 'data', 'onboarding-prompts.json')—which points to dist/data/onboarding-prompts.json at runtime. No further action required.

Comment thread config_backup_before_onboarding_test.json Outdated
Comment thread src/data/onboarding-prompts.json Outdated
Comment thread src/tools/prompts.ts Outdated
- Replace /Users/fiberta/ paths with ~/ placeholders
- Add (replace with your path) instructions for clarity
- Improve cross-platform compatibility in onboarding prompts
- Addresses coderabbitai[bot] review feedback
- Uncomment the cache check in loadPromptsData()
- Improves performance by avoiding redundant file reads
- Cache is properly cleared via clearCache() function when needed
@wonderwhy-er wonderwhy-er merged commit e4984c4 into main Sep 10, 2025
1 of 2 checks passed
@wonderwhy-er wonderwhy-er deleted the onboarding branch September 10, 2025 08:49
@coderabbitai coderabbitai Bot mentioned this pull request Nov 14, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant