Skip to content

Feature/welcome page ab test#298

Merged
wonderwhy-er merged 16 commits into
mainfrom
feature/welcome-page-ab-test
Dec 20, 2025
Merged

Feature/welcome page ab test#298
wonderwhy-er merged 16 commits into
mainfrom
feature/welcome-page-ab-test

Conversation

@wonderwhy-er

@wonderwhy-er wonderwhy-er commented Dec 19, 2025

Copy link
Copy Markdown
Owner

CodeAnt-AI Description

Add A/B welcome-page experiment, cross-platform opener, analytics, and manifest/build updates

What Changed

  • Implements a 50/50 A/B test that may open a welcome page for claude-ai users on first run, deciding treatment by the last hex digit of clientId and persisting the assignment so it is shown or skipped consistently
  • Adds a cross-platform browser opener and a welcome-page opener that launches the appropriate URL; welcome-page exposure is saved to config and included in analytics events
  • Records the welcome-page assignment in config so other parts of the app can read it, and surfaces that value in telemetry payloads
  • Adds user-facing tool annotations (titles and readOnly/destructive hints) for several tools to improve tool listings and onboarding clarity
  • Adds a privacy policy URL to the extension manifest and makes the MCP bundle's package.json copy dependencies directly from the project's package.json so the bundle's dependencies match the package

Impact

✅ Targeted welcome onboarding for Claude users
✅ Analytics records welcome page exposure
✅ MCP bundle uses actual package dependencies

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

  • New Features

    • Welcome-page onboarding for first-time users, gated by A/B testing and deterministic assignment.
    • Privacy policy link added; manifest version updated to 0.3.
  • Enhancements

    • Tool metadata enriched to surface UI hints.
    • Cross-platform browser launch to open the welcome page.
    • Build now mirrors project dependencies for bundled outputs.
    • A/B test assignment persistence and query APIs added.
  • Tests & Samples

    • New A/B test suite and several sample Markdown/PDF examples added.

✏️ Tip: You can customize this high-level summary in your review settings.

- 50/50 split based on clientId last hex digit
- Opens welcome page for claude-ai users in treatment group
- Tracks wasShownWelcomePage in config and analytics
- Cross-platform browser opening (macOS/Windows/Linux)
- manifest.template.json: Add privacy_policies URL
- create_directory: readOnlyHint=false, destructiveHint=false
- start_search: readOnlyHint=true
- stop_search: readOnlyHint=false, destructiveHint=false
- give_feedback_to_desktop_commander: openWorldHint=true
- get_prompts: readOnlyHint=true
@codeant-ai

codeant-ai Bot commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds deterministic A/B testing with persistence, first-run onboarding gated by A/B variant, cross‑platform browser opener to show a welcome page, telemetry inclusion of A/B assignments and onboarding flag, tool metadata annotations, manifest/version updates, and MCPB bundling now copies root dependencies.

Changes

Cohort / File(s) Change Summary
Manifest & Build
manifest.template.json, scripts/build-mcpb.cjs
Bumped manifest_version to "0.3" and added privacy_policies; build script now copies dependencies from root package.json into MCPB bundle instead of using a hardcoded block.
Config manager
src/config-manager.ts
Added private _isFirstRun flag, set on default-config creation vs load, and public isFirstRun(): boolean; comment updated to allow abTest_* keys.
A/B test utility
src/utils/ab-test.ts
New deterministic A/B testing: reads experiments from featureFlagManager, computes stable variant using clientId+hash, persists abTest_<name> in config, caches assignments, exports hasFeature() and getABTestAssignments().
Server / Onboarding & Tools
src/server.ts
Imports handleWelcomePageOnboarding and invokes it during client init for claude-ai when allowed; adds annotations (title, readOnlyHint, destructiveHint, openWorldHint) to multiple public tools.
Telemetry / Capture
src/utils/capture.ts
Reads sawOnboardingPage from configManager and attaches saw_onboarding_page to clientContext before sanitization; merges A/B assignments into event properties (errors ignored).
Browser utilities & Onboarding
src/utils/open-browser.ts, src/utils/welcome-onboarding.ts
Added cross-platform openBrowser(url) and openWelcomePage() using safe exec/spawn; handleWelcomePageOnboarding() checks isFirstRun() + A/B assignment, opens welcome page, sets sawOnboardingPage in config, and logs outcomes.
Tests & Samples
test/ab-test.test.js, test/samples/*
New unit-style test for A/B logic with deterministic assignments and persistence checks; added several sample Markdown/PDF fixtures under test/samples/.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant ConfigMgr as ConfigManager
    participant ABTest as ab-test
    participant Browser as open-browser
    participant Capture as capture.ts

    Client->>Server: initialize(clientType)
    Server->>ConfigMgr: isFirstRun()
    ConfigMgr-->>Server: true/false

    Server->>ABTest: hasFeature("showOnboardingPage")
    ABTest->>ConfigMgr: get abTest_<experiment>
    ConfigMgr-->>ABTest: assignment / none
    ABTest-->>Server: inTreatment? (true/false)

    alt inTreatment and isFirstRun
        Server->>Browser: openWelcomePage()
        Browser-->>Server: resolved / rejected
        Server->>ConfigMgr: setValue("sawOnboardingPage", true)
        ConfigMgr-->>Server: persisted
    end

    Note over Server,Capture: Later telemetry include A/B assignments & saw_onboarding_page
    Server->>Capture: captureEvent(event)
    Capture->>ABTest: getABTestAssignments()
    ABTest->>ConfigMgr: read abTest_* keys
    ConfigMgr-->>ABTest: assignments
    ABTest-->>Capture: assignments
    Capture->>Capture: merge into event properties and sanitize
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus areas:
    • src/utils/ab-test.ts: hashing, deterministic assignment, caching, persistence correctness.
    • src/config-manager.ts: _isFirstRun lifecycle and interactions during init/load.
    • src/utils/open-browser.ts: platform spawn/exec safety and error handling.
    • src/utils/welcome-onboarding.ts and src/server.ts: onboarding gating and non-blocking invocation.
    • src/utils/capture.ts: merging A/B assignments into telemetry without leaking PII.

Possibly related PRs

Suggested reviewers

  • serg33v

"I hopped from burrow with a twitching nose,
I hashed the paths where feature-flag wind blows.
I nudged a welcome through the browser gate,
Saved a little flag, and checked the slate.
Hooray — a tiny rabbit's code parade! 🐇"

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Feature/welcome page ab test' directly relates to the main feature implemented: an A/B test for the welcome page onboarding flow for Claude users, which is the primary objective of this PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/welcome-page-ab-test

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.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Dec 19, 2025
Comment thread src/server.ts Outdated
Comment on lines +145 to +146
const clientId = await configManager.getValue('clientId') || '';
const lastChar = clientId.slice(-1);

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.

Suggestion: The A/B test assignment for showing the welcome page falls back to an empty string when no client ID is stored, which always hashes to bucket 0 and therefore always assigns users without a client ID to the "show welcome page" group instead of the intended 50/50 split; use a random hex string when the client ID is missing to maintain an unbiased 50/50 assignment. [logic error]

Severity Level: Minor ⚠️

Suggested change
const clientId = await configManager.getValue('clientId') || '';
const lastChar = clientId.slice(-1);
let clientId = await configManager.getValue('clientId');
if (typeof clientId !== 'string' || clientId.length === 0) {
// If no clientId exists yet, use a random hex string so the split remains 50/50
clientId = Math.random().toString(16);
}
const lastChar = clientId.slice(-1);
const num = parseInt(lastChar, 16);
Why it matters? ⭐

The current code falls back to an empty string when clientId is missing; slice(-1) on '' yields '' and parseInt('',16) returns NaN, which the code then coerces to 0 via || 0. That makes every missing clientId land in the "even" bucket (shouldShow = true), biasing the A/B split. The proposed change (use a random hex when clientId is absent) fixes a real logic bug that would misassign new/anonymous users and restores the intended ~50/50 split. It's a small, targeted behavioral fix that directly improves correctness.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/server.ts
**Line:** 145:146
**Comment:**
	*Logic Error: The A/B test assignment for showing the welcome page falls back to an empty string when no client ID is stored, which always hashes to bucket 0 and therefore always assigns users without a client ID to the "show welcome page" group instead of the intended 50/50 split; use a random hex string when the client ID is missing to maintain an unbiased 50/50 assignment.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Comment thread src/utils/open-browser.ts Outdated
Comment on lines +1 to +26
import { exec } from 'child_process';
import os from 'os';
import { logToStderr } from './logger.js';

/**
* Open a URL in the default browser (cross-platform)
*/
export async function openBrowser(url: string): Promise<void> {
const platform = os.platform();

let command: string;

switch (platform) {
case 'darwin': // macOS
command = `open "${url}"`;
break;
case 'win32': // Windows
command = `start "" "${url}"`;
break;
default: // Linux and others
command = `xdg-open "${url}"`;
break;
}

return new Promise((resolve, reject) => {
exec(command, (error) => {

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.

Suggestion: The URL is interpolated directly into a shell command string passed to exec, so if the URL ever contains double quotes or shell metacharacters it can break the command or be exploited for command injection; instead, the command should be executed without going through a shell by passing the program and arguments separately (e.g., via execFile). [security]

Severity Level: Critical 🚨

Suggested change
import { exec } from 'child_process';
import os from 'os';
import { logToStderr } from './logger.js';
/**
* Open a URL in the default browser (cross-platform)
*/
export async function openBrowser(url: string): Promise<void> {
const platform = os.platform();
let command: string;
switch (platform) {
case 'darwin': // macOS
command = `open "${url}"`;
break;
case 'win32': // Windows
command = `start "" "${url}"`;
break;
default: // Linux and others
command = `xdg-open "${url}"`;
break;
}
return new Promise((resolve, reject) => {
exec(command, (error) => {
import { execFile } from 'child_process';
import os from 'os';
import { logToStderr } from './logger.js';
/**
* Open a URL in the default browser (cross-platform)
*/
export async function openBrowser(url: string): Promise<void> {
const platform = os.platform();
let command: string;
let args: string[];
switch (platform) {
case 'darwin': // macOS
command = 'open';
args = [url];
break;
case 'win32': // Windows
command = 'cmd';
args = ['/c', 'start', '', url];
break;
default: // Linux and others
command = 'xdg-open';
args = [url];
break;
}
return new Promise((resolve, reject) => {
execFile(command, args, (error) => {
Why it matters? ⭐

The suggestion addresses a real security and robustness issue: building a shell command string with an interpolated URL can break if the URL contains quotes or shell metacharacters and can open a command-injection vector. Switching to execFile (or spawn with args) runs the program directly without invoking a shell and preserves the URL as a single argument, which fixes both correctness and security concerns. The improved code directly maps to the existing platform-specific behavior (open/xdg-open or cmd /c start) and is a safe, minimal change.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/utils/open-browser.ts
**Line:** 1:26
**Comment:**
	*Security: The URL is interpolated directly into a shell command string passed to `exec`, so if the URL ever contains double quotes or shell metacharacters it can break the command or be exploited for command injection; instead, the command should be executed without going through a shell by passing the program and arguments separately (e.g., via `execFile`).

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

@codeant-ai

codeant-ai Bot commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Command injection
    The code builds a shell command by interpolating the url directly into a string and passes it to exec. If url can be influenced (even indirectly), this permits shell command injection or breaking the intended command. Prefer APIs that avoid a shell or validate/encode the URL strictly.

  • Unescaped / malformed URLs
    The code wraps url in double quotes but doesn't escape double-quotes or other special characters inside url. URLs containing quotes, newlines or shell-sensitive characters can break the command or fail. Also the implementation assumes xdg-open/open/start exist; consider fallbacks and robust error messages.

  • Telemetry / Privacy
    Adding the A/B assignment into analytics increases fingerprinting surface. Verify this is allowed by privacy policy and that the value sent cannot be linked to PII. Ensure the key/value are intentionally non-identifying and respect user telemetry opt-out (telemetryEnabled is checked earlier, but confirm behavior across code paths).

  • Config read/write error handling
    The code awaits configManager.getValue and configManager.setValue without localized try/catch. If config manager I/O throws, the entire initialization flow will be affected (caught by outer try, but better to handle config errors gracefully and log them without blocking other init tasks). Also confirm configManager.setValue API exists and has expected semantics.

  • Local/workspace/file deps
    Copying dependency entries verbatim from the main package.json may include local, workspace or file: specifiers (e.g. workspace:*, file:, link:). These specifiers won't resolve correctly when installing inside the bundle and will cause npm install to fail. Filter or normalize such entries before writing the bundle package.json.

@codeant-ai

codeant-ai Bot commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

CodeAnt AI finished reviewing your PR.

@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: 4

🧹 Nitpick comments (1)
src/utils/open-browser.ts (1)

45-46: TODO comment should be addressed before merging.

The comment indicates this URL path (/welcome-v6/) is temporary for testing. Either remove the TODO if this is the intended production path, or track this as a follow-up issue.

Would you like me to open an issue to track changing the URL back to /welcome/ after testing?

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93f4114 and c36bdda.

📒 Files selected for processing (6)
  • manifest.template.json (2 hunks)
  • scripts/build-mcpb.cjs (2 hunks)
  • src/config-manager.ts (4 hunks)
  • src/server.ts (7 hunks)
  • src/utils/capture.ts (2 hunks)
  • src/utils/open-browser.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/utils/open-browser.ts (1)
src/utils/logger.ts (1)
  • logToStderr (71-82)
src/utils/capture.ts (1)
src/config-manager.ts (1)
  • configManager (238-238)
src/server.ts (2)
src/config-manager.ts (1)
  • configManager (238-238)
src/utils/open-browser.ts (1)
  • openWelcomePage (42-49)
🔇 Additional comments (6)
scripts/build-mcpb.cjs (1)

109-119: Good improvement for dependency consistency.

Sourcing dependencies directly from package.json eliminates the risk of drift between the npm package and MCPB bundle. The added comments clarify the intent well.

src/utils/capture.ts (2)

102-113: A/B test context integration is well implemented.

The silent error handling and undefined check are appropriate - this ensures analytics failures don't break the application and correctly distinguishes between users who haven't been assigned vs those assigned to control group (false).


220-225: Property spread order is correct.

The welcomePageContext is correctly spread before sanitizedProperties, ensuring any user-provided properties can override if needed while still including the A/B test data.

src/config-manager.ts (1)

18-18: Config field addition looks good.

The wasShownWelcomePage field appropriately tracks A/B test assignment persistently across sessions.

src/server.ts (2)

137-139: TODO comment indicates testing mode - ensure this is intentional for merge.

The isFirstRun() check is commented out, meaning all claude-ai users will enter the A/B test flow on every initialization (until wasShownWelcomePage is set). Verify this is the intended behavior before merging, or uncomment the check.


457-461: Tool annotations are a good addition.

The new annotations (title, readOnlyHint, destructiveHint, openWorldHint) provide useful UI metadata for tool discovery and presentation. The hint values appear consistent with each tool's behavior.

Also applies to: 601-604, 650-654, 1071-1075, 1103-1106

Comment thread manifest.template.json
{
"$schema": "../../dist/mcpb-manifest.schema.json",
"manifest_version": "0.1",
"manifest_version": "0.3",

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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "manifest.template.json" -type f

Repository: wonderwhy-er/DesktopCommanderMCP

Length of output: 100


🏁 Script executed:

cat -n ./manifest.template.json

Repository: wonderwhy-er/DesktopCommanderMCP

Length of output: 8079


🏁 Script executed:

#!/bin/bash
# Verify the privacy policy URL is accessible
curl -sI "https://legal.desktopcommander.app/privacy_desktop_commander_mcp" 2>&1 | head -10

Repository: wonderwhy-er/DesktopCommanderMCP

Length of output: 354


Fix privacy policy URL before release.

The manifest version bump to 0.3 and privacy_policies addition align with the new features. However, the privacy policy URL https://legal.desktopcommander.app/privacy_desktop_commander_mcp returns a 404 error and is not accessible. Ensure the correct URL is provided or the endpoint is deployed before shipping this release.

🤖 Prompt for AI Agents
In manifest.template.json around line 3, the privacy_policies URL added for the
manifest version 0.3 points to
https://legal.desktopcommander.app/privacy_desktop_commander_mcp which returns
404; update the manifest to use the correct, reachable privacy policy URL (or
remove the entry temporarily) before release. Locate the privacy_policies field
in the template, replace the broken URL with the canonical deployed URL for
Desktop Commander’s privacy policy (or a stable placeholder like the
company/legal landing page) and verify the link returns 200; if the endpoint
isn’t yet deployed, revert the privacy_policies addition until the policy is
available to avoid shipping a broken link.

Comment thread src/config-manager.ts
Comment thread src/server.ts Outdated
Comment thread src/utils/open-browser.ts Outdated
Use execFile with args array instead of exec with string interpolation
- New src/utils/ab-test.ts with getABTestVariant() and isInTreatment()
- Deterministic assignment based on clientId hash
- Config keys use 'abTest_' prefix for clarity
- Simplifies server.ts A/B test logic significantly
noOnboardingPage / sawOnboardingPage instead of control/treatment
@codeant-ai

codeant-ai Bot commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@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: 1

♻️ Duplicate comments (1)
src/utils/ab-test.ts (1)

17-33: A/B test assignment is biased when clientId is missing.

When clientId is undefined or empty (line 26), the fallback to '' causes all users without a clientId to hash identically. Since the hash computation (line 27) would then only hash the test name, all these users will be deterministically assigned to the same variant, breaking the intended 50/50 split.

For example:

  • User without clientId: simpleHash('' + 'welcomePage') → always returns the same hash
  • That hash % 2 → always 0 or 1
  • Result: All new users get assigned to the same bucket instead of ~50/50
🔎 Proposed fix to ensure unbiased assignment
   // Assign based on clientId hash
-  const clientId = await configManager.getValue('clientId') || '';
+  let clientId = await configManager.getValue('clientId');
+  if (!clientId) {
+    // Generate clientId if missing to ensure unbiased assignment
+    const { randomUUID } = await import('crypto');
+    clientId = randomUUID();
+    await configManager.setValue('clientId', clientId);
+  }
   const hash = simpleHash(clientId + test.name);
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c36bdda and 6e8a2bd.

📒 Files selected for processing (5)
  • src/config-manager.ts (4 hunks)
  • src/server.ts (7 hunks)
  • src/utils/ab-test.ts (1 hunks)
  • src/utils/capture.ts (2 hunks)
  • src/utils/open-browser.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/utils/capture.ts
  • src/config-manager.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/utils/open-browser.ts (1)
src/utils/logger.ts (1)
  • logToStderr (71-82)
src/utils/ab-test.ts (1)
src/config-manager.ts (1)
  • configManager (237-237)
src/server.ts (2)
src/utils/ab-test.ts (1)
  • isInTreatment (50-59)
src/utils/open-browser.ts (1)
  • openWelcomePage (43-46)
🔇 Additional comments (5)
src/utils/open-browser.ts (1)

43-46: LGTM!

The command injection concerns from past reviews have been properly addressed by using execFile and spawn with the URL passed as a separate argument rather than interpolated into a shell command string.

src/server.ts (2)

136-151: TODO comment indicates incomplete implementation.

The comment on line 137 notes that a configManager.isFirstRun() check should be added after testing. This suggests the onboarding flow is still in a testing phase and may trigger welcome pages for users who have already seen them.

Consider either:

  1. Implementing the isFirstRun() check before merging, or
  2. Confirming this is intentional for A/B testing purposes and the TODO will be addressed in a follow-up PR.

444-448: LGTM! Tool metadata annotations are appropriate.

The added annotations (title, readOnlyHint, destructiveHint, openWorldHint) provide useful UI hints without changing any tool logic. All hint values are correctly aligned with their respective tool behaviors.

Also applies to: 588-591, 637-641, 1058-1062, 1090-1093

src/utils/ab-test.ts (2)

38-45: Hash function implementation is sound.

The simpleHash function provides deterministic assignment using standard string hashing techniques. Implementation is correct for the A/B testing use case.


50-59: Convenience wrapper is correctly implemented.

The isInTreatment function appropriately wraps getABTestVariant for the welcome page onboarding use case. However, it inherits the assignment bias issue from getABTestVariant when clientId is missing.

Comment thread src/utils/open-browser.ts
Comment on lines +28 to +31
// Windows 'start' is a shell builtin, use spawn with shell but pass URL as separate arg
spawn('cmd', ['/c', 'start', '', url], { shell: false }).on('close', (code) => {
code === 0 ? resolve() : reject(new Error(`Exit code ${code}`));
});

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 | 🟡 Minor

Windows code path bypasses callback and misses success logging.

The Windows implementation uses .on('close') instead of the callback, so the success log at line 18 ("Opened browser to: ${url}") never executes for Windows users. This creates inconsistent logging across platforms.

🔎 Proposed fix to unify error handling
     case 'win32':
-      // Windows 'start' is a shell builtin, use spawn with shell but pass URL as separate arg
-      spawn('cmd', ['/c', 'start', '', url], { shell: false }).on('close', (code) => {
-        code === 0 ? resolve() : reject(new Error(`Exit code ${code}`));
-      });
+      // Windows 'start' is a shell builtin, requires cmd.exe
+      spawn('cmd', ['/c', 'start', '', url], { shell: false }).on('close', (code) => {
+        callback(code === 0 ? null : new Error(`Exit code ${code}`));
+      });
       break;
🤖 Prompt for AI Agents
In src/utils/open-browser.ts around lines 28 to 31, the Windows spawn branch
uses .on('close') but never triggers the same success logging used by other
platforms, causing missing "Opened browser to: ${url}" output; update the
Windows branch so that when the child exits with code 0 you call the same
success path — log processLogger.info(`Opened browser to: ${url}`) and then
resolve(), and on non-zero exit reject with an Error(`Exit code ${code}`),
ensuring consistent logging and resolution behavior across platforms.

- hasFeature('showOnboardingPage') - clean API for checking features
- A/B experiments define which variants enable which features
- Separate 'sawOnboardingPage' config tracks if action was taken
- getABTestAssignments() for analytics - auto-includes all experiments
- Extensible: add new experiments/features in one place
- Experiments defined in production.json under 'experiments' key
- Variant names are used directly as feature names
- hasFeature('showOnboardingPage') checks if user has that variant
- No hardcoded experiments - fully server-controlled
Use persisted config value, not recalculate from hash

@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: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e8a2bd and f756e5d.

📒 Files selected for processing (3)
  • src/server.ts (7 hunks)
  • src/utils/ab-test.ts (1 hunks)
  • src/utils/capture.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/utils/capture.ts
  • src/utils/ab-test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/server.ts (3)
src/utils/ab-test.ts (1)
  • hasFeature (72-82)
src/config-manager.ts (1)
  • configManager (237-237)
src/utils/open-browser.ts (1)
  • openWelcomePage (43-46)
🔇 Additional comments (2)
src/server.ts (2)

59-61: LGTM - New imports support onboarding functionality.

The imports for welcome page opening, A/B testing, and config management are correctly added to support the new onboarding flow.


444-448: LGTM - Tool annotations are semantically correct.

The added annotations accurately describe each tool's behavior:

  • Read-only hints correctly identify non-mutating operations
  • Destructive hints appropriately mark operations that modify state
  • Open-world hint correctly identifies external network access

These annotations will improve tool listings and provide better UX hints.

Also applies to: 588-591, 637-641, 1058-1062, 1090-1093

Comment thread src/server.ts
- New src/utils/welcome-onboarding.ts handles all logic
- Only triggers on first run (config just created)
- Then checks A/B test assignment
- server.ts reduced by 13 lines
- Handle undefined/null experiments config gracefully
- Handle malformed experiment objects (null, wrong type)
- Add test/ab-test.test.js with 10 test cases
- Ensures missing remote flags don't crash the app

@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: 2

🧹 Nitpick comments (3)
test/samples/02_sample_invoice.pdf.md (3)

3-3: Fix heading hierarchy increments.

Heading levels should increment by one level at a time. Lines 3 and 27 jump from h2 directly to h4 (expected h3).

🔎 Proposed fixes
-#### 1 
+### 1 
-#### # : INV-000007 
+### # : INV-000007 

Also applies to: 27-27


23-23: Wrap bare URL in angle brackets or link syntax.

Line 23 contains a bare email address. Use angle brackets <email@domain.com> or link syntax [email@domain.com](mailto:email@domain.com) for proper markdown formatting.


35-35: Align heading to line start.

The heading on line 35 is indented and should start at the beginning of the line.

🔎 Proposed fix
-### Shanti Spa Ayurveda 
+### Shanti Spa Ayurveda 
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2af97d9 and 9dcb1c5.

📒 Files selected for processing (6)
  • src/utils/ab-test.ts (1 hunks)
  • test/ab-test.test.js (1 hunks)
  • test/samples/01_sample_simple.pdf.md (1 hunks)
  • test/samples/02_sample_invoice.pdf.md (1 hunks)
  • test/samples/03_sample_compex.pdf.md (1 hunks)
  • test/samples/URL Sample.md (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/ab-test.test.js (2)
scripts/analyze-fuzzy-logs.js (1)
  • key (108-108)
src/utils/ab-test.ts (1)
  • hasFeature (72-83)
src/utils/ab-test.ts (3)
test/ab-test.test.js (13)
  • variantCache (42-42)
  • experiments (45-45)
  • experiments (72-72)
  • experiment (46-46)
  • configKey (53-53)
  • existing (54-54)
  • clientId (61-61)
  • hash (34-34)
  • hash (62-62)
  • variantIndex (63-63)
  • variant (64-64)
  • variant (77-77)
  • i (35-35)
src/utils/feature-flags.ts (1)
  • featureFlagManager (178-178)
src/config-manager.ts (1)
  • configManager (237-237)
🪛 GitHub Actions: Codespell
test/samples/URL Sample.md

[error] 5-5: codespell: varius ==> various

🪛 GitHub Check: Check for spelling errors
test/samples/URL Sample.md

[failure] 5-5:
varius ==> various


[failure] 5-5:
varius ==> various


[failure] 5-5:
ridiculus ==> ridiculous


[failure] 5-5:
varius ==> various


[failure] 5-5:
varius ==> various


[failure] 5-5:
varius ==> various

🪛 LanguageTool
test/samples/01_sample_simple.pdf.md

[uncategorized] ~3-~3: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...World This is a test PDF created from markdown. ## Features Simple text Bold text...

(MARKDOWN_NNP)

test/samples/03_sample_compex.pdf.md

[grammar] ~15-~15: Ensure spelling is correct
Context: ...s, impermanent gain ∗The authors are greteful to 0xSami_, Andrea Bugin, Andrea Prampo...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[style] ~24-~24: Consider a more concise word here.
Context: ...e for a percentage of the trading fees, in order to facilitate trades. While transactions o...

(IN_ORDER_TO_PREMIUM)


[style] ~28-~28: Consider a more concise word here.
Context: ...quidity pool with crypto assets it owns in order to facilitate trading on the platform and ...

(IN_ORDER_TO_PREMIUM)


[style] ~32-~32: Consider a more concise word here.
Context: ...okens to be locked for a period of time in order to access additional rewards (Liquidity Mi...

(IN_ORDER_TO_PREMIUM)


[style] ~36-~36: ‘in brief’ might be wordy. Consider a shorter alternative.
Context: ...tant product AMM Here we will present in brief what is a constant product AMM for a mo...

(EN_WORDINESS_PREMIUM_IN_BRIEF)


[style] ~50-~50: ‘in an instant’ might be wordy. Consider a shorter alternative.
Context: ...e of this type of pools is that knowing in an instant t 0 the following data: - the constan...

(EN_WORDINESS_PREMIUM_IN_AN_INSTANT)


[style] ~59-~59: Consider using the more formal “until”.
Context: ...no injection of new capital in the pool till time T , we can calculate the number of...

(TILL)


[style] ~141-~141: Consider using the more formal “until”.
Context: ...### √ αt − 1) What we have ignored till now are the fees that Traders have to p...

(TILL)


[style] ~165-~165: Consider using “who” when you are referring to a person instead of an object.
Context: ... DeFi jargon an HODLer indicates a user that holds its tokens without doing anything...

(THAT_WHO)


[style] ~180-~180: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...rn of a LP and that of an equal-weight (with respect to the starting time) HODLer. For a more i...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[grammar] ~180-~180: Use a hyphen to join words.
Context: ...the starting time) HODLer. For a more in depth analysis of it on UniSwap, the mos...

(QB_NEW_EN_HYPHEN)


[style] ~274-~274: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...to zero only if the price of token x is exactly the same as its starting price S 0 (r = 0). This...

(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)


[style] ~386-~386: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ative of the price of the position (Pt) with respect to the underlying price (St): #### ∆LP :...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[style] ~425-~425: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ative of the price of the position (Pt) with respect to the underlying price (St). It can also ...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[style] ~425-~425: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...seen as the partial derivative of Delta with respect to the underlying price: #### ΓLP := ...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[style] ~478-~478: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...derivative of the price of the position with respect to the volatility (σ): νLP := ∂Pt ∂σ ###...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[style] ~484-~484: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ative of the price of the position (Pt) with respect to time (t): #### ΘLP := ∂Pt ∂t = ...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[style] ~494-~494: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...ative of the price of the position (Pt) with respect to the risk-free rate (rf ): ρLP := ...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[style] ~504-~504: Consider a more concise word here.
Context: ...older to additional vega and rho risks. In order to calculate the fair price of the locked ...

(IN_ORDER_TO_PREMIUM)


[grammar] ~521-~521: Use a hyphen to join words.
Context: ...d" process relative to token x with risk free rate rx (we can take for example th...

(QB_NEW_EN_HYPHEN)


[grammar] ~521-~521: Use a hyphen to join words.
Context: ...d" process relative to token y with risk free rate ry (analogously we can take th...

(QB_NEW_EN_HYPHEN)


[style] ~549-~549: The adverb ‘also’ is commonly used to connect clauses and isn’t usually used at the end of a phrase or before a conjunction. Consider replacing it with a more formal alternative.
Context: ...at we can use the market price dynamics also for the pool price dynamics. From here ...

(ALSO_AS_WELL)


[grammar] ~931-~931: Use a hyphen to join words.
Context: ... Even if we have a model with two risk free rates we note that the price depend...

(QB_NEW_EN_HYPHEN)


[style] ~931-~931: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...rf so we can study only the sensibility with respect to rf. ρLP := ∂Pt ∂rf #### = −V 0 #...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[grammar] ~1346-~1346: Ensure spelling is correct
Context: ... IG Delta 1% IG 4.2.3 Gamma #### ΓIG = ∂^2 Pt ∂S^2 t #### = #### V 0 ...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[style] ~1677-~1677: This phrase is redundant. Consider writing “eliminates”.
Context: ...cked liquidity, buying Impermanent Gain completely eliminates Gamma and Vega risks as well as signifi...

(COMPLETELY_ANNIHILATE)

🪛 markdownlint-cli2 (0.18.1)
test/samples/02_sample_invoice.pdf.md

3-3: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


23-23: Bare URL used

(MD034, no-bare-urls)


27-27: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4

(MD001, heading-increment)


35-35: Headings must start at the beginning of the line

(MD023, heading-start-left)

test/samples/03_sample_compex.pdf.md

71-71: Multiple headings with the same content

(MD024, no-duplicate-heading)


83-83: Multiple headings with the same content

(MD024, no-duplicate-heading)


85-85: Multiple headings with the same content

(MD024, no-duplicate-heading)


89-89: Multiple headings with the same content

(MD024, no-duplicate-heading)


91-91: Multiple headings with the same content

(MD024, no-duplicate-heading)


97-97: Multiple headings with the same content

(MD024, no-duplicate-heading)


99-99: Multiple headings with the same content

(MD024, no-duplicate-heading)


105-105: Multiple headings with the same content

(MD024, no-duplicate-heading)


107-107: Multiple headings with the same content

(MD024, no-duplicate-heading)


113-113: Multiple headings with the same content

(MD024, no-duplicate-heading)


121-121: Multiple headings with the same content

(MD024, no-duplicate-heading)


125-125: Multiple headings with the same content

(MD024, no-duplicate-heading)


131-131: Multiple headings with the same content

(MD024, no-duplicate-heading)


137-137: Multiple headings with the same content

(MD024, no-duplicate-heading)


149-149: Multiple headings with the same content

(MD024, no-duplicate-heading)


153-153: Multiple headings with the same content

(MD024, no-duplicate-heading)


159-159: Multiple headings with the same content

(MD024, no-duplicate-heading)


184-184: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


192-192: Multiple headings with the same content

(MD024, no-duplicate-heading)


194-194: Multiple headings with the same content

(MD024, no-duplicate-heading)


204-204: Multiple headings with the same content

(MD024, no-duplicate-heading)


208-208: Multiple headings with the same content

(MD024, no-duplicate-heading)


216-216: Multiple headings with the same content

(MD024, no-duplicate-heading)


218-218: Multiple headings with the same content

(MD024, no-duplicate-heading)


240-240: Multiple headings with the same content

(MD024, no-duplicate-heading)


242-242: Multiple headings with the same content

(MD024, no-duplicate-heading)


246-246: Multiple headings with the same content

(MD024, no-duplicate-heading)


266-266: Multiple headings with the same content

(MD024, no-duplicate-heading)


293-293: Multiple headings with the same content

(MD024, no-duplicate-heading)


303-303: Multiple headings with the same content

(MD024, no-duplicate-heading)


305-305: Multiple headings with the same content

(MD024, no-duplicate-heading)


307-307: Multiple headings with the same content

(MD024, no-duplicate-heading)


311-311: Multiple headings with the same content

(MD024, no-duplicate-heading)


313-313: Multiple headings with the same content

(MD024, no-duplicate-heading)


319-319: Multiple headings with the same content

(MD024, no-duplicate-heading)


323-323: Multiple headings with the same content

(MD024, no-duplicate-heading)


335-335: Multiple headings with the same content

(MD024, no-duplicate-heading)


339-339: Multiple headings with the same content

(MD024, no-duplicate-heading)


341-341: Multiple headings with the same content

(MD024, no-duplicate-heading)


347-347: Multiple headings with the same content

(MD024, no-duplicate-heading)


349-349: Multiple headings with the same content

(MD024, no-duplicate-heading)


351-351: Multiple headings with the same content

(MD024, no-duplicate-heading)


353-353: Multiple headings with the same content

(MD024, no-duplicate-heading)


366-366: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


374-374: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


413-413: Multiple headings with the same content

(MD024, no-duplicate-heading)


415-415: Multiple headings with the same content

(MD024, no-duplicate-heading)


417-417: Multiple headings with the same content

(MD024, no-duplicate-heading)


419-419: Multiple headings with the same content

(MD024, no-duplicate-heading)


431-431: Multiple headings with the same content

(MD024, no-duplicate-heading)


439-439: Multiple headings with the same content

(MD024, no-duplicate-heading)


443-443: Multiple headings with the same content

(MD024, no-duplicate-heading)


466-466: Multiple headings with the same content

(MD024, no-duplicate-heading)


468-468: Multiple headings with the same content

(MD024, no-duplicate-heading)


470-470: Multiple headings with the same content

(MD024, no-duplicate-heading)


472-472: Multiple headings with the same content

(MD024, no-duplicate-heading)


500-500: Multiple headings with the same content

(MD024, no-duplicate-heading)


561-561: Multiple headings with the same content

(MD024, no-duplicate-heading)


571-571: Multiple headings with the same content

(MD024, no-duplicate-heading)


573-573: Multiple headings with the same content

(MD024, no-duplicate-heading)


575-575: Multiple headings with the same content

(MD024, no-duplicate-heading)


579-579: Multiple headings with the same content

(MD024, no-duplicate-heading)


587-587: Multiple headings with the same content

(MD024, no-duplicate-heading)


589-589: Multiple headings with the same content

(MD024, no-duplicate-heading)


593-593: Multiple headings with the same content

(MD024, no-duplicate-heading)


597-597: Multiple headings with the same content

(MD024, no-duplicate-heading)


599-599: Multiple headings with the same content

(MD024, no-duplicate-heading)


601-601: Multiple headings with the same content

(MD024, no-duplicate-heading)


605-605: Multiple headings with the same content

(MD024, no-duplicate-heading)


611-611: Multiple headings with the same content

(MD024, no-duplicate-heading)


625-625: Multiple headings with the same content

(MD024, no-duplicate-heading)


629-629: Multiple headings with the same content

(MD024, no-duplicate-heading)


639-639: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


641-641: Multiple headings with the same content

(MD024, no-duplicate-heading)


709-709: Multiple headings with the same content

(MD024, no-duplicate-heading)


711-711: Multiple headings with the same content

(MD024, no-duplicate-heading)


713-713: Multiple headings with the same content

(MD024, no-duplicate-heading)


717-717: Multiple headings with the same content

(MD024, no-duplicate-heading)


721-721: Multiple headings with the same content

(MD024, no-duplicate-heading)


725-725: Multiple headings with the same content

(MD024, no-duplicate-heading)


729-729: Multiple headings with the same content

(MD024, no-duplicate-heading)


737-737: Multiple headings with the same content

(MD024, no-duplicate-heading)


741-741: Multiple headings with the same content

(MD024, no-duplicate-heading)


749-749: Multiple headings with the same content

(MD024, no-duplicate-heading)


753-753: Multiple headings with the same content

(MD024, no-duplicate-heading)


757-757: Multiple headings with the same content

(MD024, no-duplicate-heading)


761-761: Multiple headings with the same content

(MD024, no-duplicate-heading)


767-767: Multiple headings with the same content

(MD024, no-duplicate-heading)


769-769: Multiple headings with the same content

(MD024, no-duplicate-heading)


771-771: Multiple headings with the same content

(MD024, no-duplicate-heading)


777-777: Multiple headings with the same content

(MD024, no-duplicate-heading)


781-781: Multiple headings with the same content

(MD024, no-duplicate-heading)


785-785: Multiple headings with the same content

(MD024, no-duplicate-heading)


789-789: Multiple headings with the same content

(MD024, no-duplicate-heading)


822-822: Multiple headings with the same content

(MD024, no-duplicate-heading)


828-828: Multiple headings with the same content

(MD024, no-duplicate-heading)


832-832: Multiple headings with the same content

(MD024, no-duplicate-heading)


836-836: Multiple headings with the same content

(MD024, no-duplicate-heading)


840-840: Multiple headings with the same content

(MD024, no-duplicate-heading)


876-876: Multiple headings with the same content

(MD024, no-duplicate-heading)


880-880: Multiple headings with the same content

(MD024, no-duplicate-heading)


888-888: Multiple headings with the same content

(MD024, no-duplicate-heading)


892-892: Multiple headings with the same content

(MD024, no-duplicate-heading)


896-896: Multiple headings with the same content

(MD024, no-duplicate-heading)


900-900: Multiple headings with the same content

(MD024, no-duplicate-heading)


902-902: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


904-904: Multiple headings with the same content

(MD024, no-duplicate-heading)


937-937: Multiple headings with the same content

(MD024, no-duplicate-heading)


941-941: Multiple headings with the same content

(MD024, no-duplicate-heading)


947-947: Multiple headings with the same content

(MD024, no-duplicate-heading)


951-951: Multiple headings with the same content

(MD024, no-duplicate-heading)


955-955: Multiple headings with the same content

(MD024, no-duplicate-heading)


959-959: Multiple headings with the same content

(MD024, no-duplicate-heading)


961-961: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


963-963: Multiple headings with the same content

(MD024, no-duplicate-heading)


1002-1002: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


1070-1070: Multiple headings with the same content

(MD024, no-duplicate-heading)


1078-1078: Multiple headings with the same content

(MD024, no-duplicate-heading)


1080-1080: Multiple headings with the same content

(MD024, no-duplicate-heading)


1082-1082: Multiple headings with the same content

(MD024, no-duplicate-heading)


1084-1084: Multiple headings with the same content

(MD024, no-duplicate-heading)


1086-1086: Multiple headings with the same content

(MD024, no-duplicate-heading)


1088-1088: Multiple headings with the same content

(MD024, no-duplicate-heading)


1092-1092: Multiple headings with the same content

(MD024, no-duplicate-heading)


1094-1094: Multiple headings with the same content

(MD024, no-duplicate-heading)


1096-1096: Multiple headings with the same content

(MD024, no-duplicate-heading)


1100-1100: Multiple headings with the same content

(MD024, no-duplicate-heading)


1110-1110: Multiple headings with the same content

(MD024, no-duplicate-heading)


1112-1112: Multiple headings with the same content

(MD024, no-duplicate-heading)


1116-1116: Multiple headings with the same content

(MD024, no-duplicate-heading)


1122-1122: Multiple headings with the same content

(MD024, no-duplicate-heading)


1124-1124: Multiple headings with the same content

(MD024, no-duplicate-heading)


1128-1128: Multiple headings with the same content

(MD024, no-duplicate-heading)


1140-1140: Multiple headings with the same content

(MD024, no-duplicate-heading)


1142-1142: Multiple headings with the same content

(MD024, no-duplicate-heading)


1144-1144: Multiple headings with the same content

(MD024, no-duplicate-heading)


1150-1150: Multiple headings with the same content

(MD024, no-duplicate-heading)


1152-1152: Multiple headings with the same content

(MD024, no-duplicate-heading)


1158-1158: Multiple headings with the same content

(MD024, no-duplicate-heading)


1160-1160: Multiple headings with the same content

(MD024, no-duplicate-heading)


1162-1162: Multiple headings with the same content

(MD024, no-duplicate-heading)


1166-1166: Multiple headings with the same content

(MD024, no-duplicate-heading)


1170-1170: Multiple headings with the same content

(MD024, no-duplicate-heading)


1221-1221: Multiple headings with the same content

(MD024, no-duplicate-heading)


1233-1233: Multiple headings with the same content

(MD024, no-duplicate-heading)


1235-1235: Multiple headings with the same content

(MD024, no-duplicate-heading)


1282-1282: Multiple headings with the same content

(MD024, no-duplicate-heading)


1286-1286: Multiple headings with the same content

(MD024, no-duplicate-heading)


1288-1288: Multiple headings with the same content

(MD024, no-duplicate-heading)


1290-1290: Multiple headings with the same content

(MD024, no-duplicate-heading)


1292-1292: Multiple headings with the same content

(MD024, no-duplicate-heading)


1298-1298: Multiple headings with the same content

(MD024, no-duplicate-heading)


1300-1300: Multiple headings with the same content

(MD024, no-duplicate-heading)


1302-1302: Multiple headings with the same content

(MD024, no-duplicate-heading)


1306-1306: Multiple headings with the same content

(MD024, no-duplicate-heading)


1310-1310: Multiple headings with the same content

(MD024, no-duplicate-heading)


1314-1314: Multiple headings with the same content

(MD024, no-duplicate-heading)


1356-1356: Multiple headings with the same content

(MD024, no-duplicate-heading)


1362-1362: Multiple headings with the same content

(MD024, no-duplicate-heading)


1364-1364: Multiple headings with the same content

(MD024, no-duplicate-heading)


1366-1366: Multiple headings with the same content

(MD024, no-duplicate-heading)


1370-1370: Multiple headings with the same content

(MD024, no-duplicate-heading)


1374-1374: Multiple headings with the same content

(MD024, no-duplicate-heading)


1384-1384: Multiple headings with the same content

(MD024, no-duplicate-heading)


1386-1386: Multiple headings with the same content

(MD024, no-duplicate-heading)


1388-1388: Multiple headings with the same content

(MD024, no-duplicate-heading)


1394-1394: Multiple headings with the same content

(MD024, no-duplicate-heading)


1396-1396: Multiple headings with the same content

(MD024, no-duplicate-heading)


1398-1398: Multiple headings with the same content

(MD024, no-duplicate-heading)


1402-1402: Multiple headings with the same content

(MD024, no-duplicate-heading)


1406-1406: Multiple headings with the same content

(MD024, no-duplicate-heading)


1410-1410: Multiple headings with the same content

(MD024, no-duplicate-heading)


1445-1445: Multiple headings with the same content

(MD024, no-duplicate-heading)


1451-1451: Multiple headings with the same content

(MD024, no-duplicate-heading)


1453-1453: Multiple headings with the same content

(MD024, no-duplicate-heading)


1455-1455: Multiple headings with the same content

(MD024, no-duplicate-heading)


1459-1459: Multiple headings with the same content

(MD024, no-duplicate-heading)


1463-1463: Multiple headings with the same content

(MD024, no-duplicate-heading)


1467-1467: Multiple headings with the same content

(MD024, no-duplicate-heading)


1473-1473: Multiple headings with the same content

(MD024, no-duplicate-heading)


1479-1479: Multiple headings with the same content

(MD024, no-duplicate-heading)


1483-1483: Multiple headings with the same content

(MD024, no-duplicate-heading)


1487-1487: Multiple headings with the same content

(MD024, no-duplicate-heading)


1491-1491: Multiple headings with the same content

(MD024, no-duplicate-heading)


1495-1495: Multiple headings with the same content

(MD024, no-duplicate-heading)


1497-1497: Multiple headings with the same content

(MD024, no-duplicate-heading)


1499-1499: Multiple headings with the same content

(MD024, no-duplicate-heading)


1503-1503: Multiple headings with the same content

(MD024, no-duplicate-heading)


1507-1507: Multiple headings with the same content

(MD024, no-duplicate-heading)


1511-1511: Multiple headings with the same content

(MD024, no-duplicate-heading)


1548-1548: Multiple headings with the same content

(MD024, no-duplicate-heading)


1554-1554: Multiple headings with the same content

(MD024, no-duplicate-heading)


1556-1556: Multiple headings with the same content

(MD024, no-duplicate-heading)


1558-1558: Multiple headings with the same content

(MD024, no-duplicate-heading)


1562-1562: Multiple headings with the same content

(MD024, no-duplicate-heading)


1566-1566: Multiple headings with the same content

(MD024, no-duplicate-heading)


1570-1570: Multiple headings with the same content

(MD024, no-duplicate-heading)


1572-1572: Multiple headings with the same content

(MD024, no-duplicate-heading)


1582-1582: Multiple headings with the same content

(MD024, no-duplicate-heading)


1584-1584: Multiple headings with the same content

(MD024, no-duplicate-heading)


1586-1586: Multiple headings with the same content

(MD024, no-duplicate-heading)


1590-1590: Multiple headings with the same content

(MD024, no-duplicate-heading)


1594-1594: Multiple headings with the same content

(MD024, no-duplicate-heading)


1598-1598: Multiple headings with the same content

(MD024, no-duplicate-heading)


1602-1602: Multiple headings with the same content

(MD024, no-duplicate-heading)


1651-1651: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


1663-1663: Multiple headings with the same content

(MD024, no-duplicate-heading)


1665-1665: Multiple headings with the same content

(MD024, no-duplicate-heading)


1667-1667: Multiple headings with the same content

(MD024, no-duplicate-heading)


1669-1669: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


1671-1671: Multiple headings with the same content

(MD024, no-duplicate-heading)


1690-1690: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


1720-1720: Multiple headings with the same content

(MD024, no-duplicate-heading)


1734-1734: Multiple headings with the same content

(MD024, no-duplicate-heading)


1744-1744: Multiple headings with the same content

(MD024, no-duplicate-heading)


1754-1754: Multiple headings with the same content

(MD024, no-duplicate-heading)


1758-1758: Multiple headings with the same content

(MD024, no-duplicate-heading)


1760-1760: Multiple headings with the same content

(MD024, no-duplicate-heading)


1762-1762: Multiple headings with the same content

(MD024, no-duplicate-heading)


1764-1764: Multiple headings with the same content

(MD024, no-duplicate-heading)


1768-1768: Multiple headings with the same content

(MD024, no-duplicate-heading)


1772-1772: Multiple headings with the same content

(MD024, no-duplicate-heading)


1774-1774: Multiple headings with the same content

(MD024, no-duplicate-heading)


1776-1776: Multiple headings with the same content

(MD024, no-duplicate-heading)


1780-1780: Multiple headings with the same content

(MD024, no-duplicate-heading)


1784-1784: Multiple headings with the same content

(MD024, no-duplicate-heading)


1788-1788: Multiple headings with the same content

(MD024, no-duplicate-heading)


1792-1792: Multiple headings with the same content

(MD024, no-duplicate-heading)


1796-1796: Multiple headings with the same content

(MD024, no-duplicate-heading)


1800-1800: Multiple headings with the same content

(MD024, no-duplicate-heading)


1810-1810: Multiple headings with the same content

(MD024, no-duplicate-heading)


1814-1814: Multiple headings with the same content

(MD024, no-duplicate-heading)


1820-1820: Multiple headings with the same content

(MD024, no-duplicate-heading)


1822-1822: Multiple headings with the same content

(MD024, no-duplicate-heading)


1824-1824: Multiple headings with the same content

(MD024, no-duplicate-heading)


1828-1828: Multiple headings with the same content

(MD024, no-duplicate-heading)


1836-1836: Multiple headings with the same content

(MD024, no-duplicate-heading)


1840-1840: Multiple headings with the same content

(MD024, no-duplicate-heading)


1848-1848: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


1878-1878: Multiple headings with the same content

(MD024, no-duplicate-heading)


1884-1884: Multiple headings with the same content

(MD024, no-duplicate-heading)


1888-1888: Multiple headings with the same content

(MD024, no-duplicate-heading)


1892-1892: Multiple headings with the same content

(MD024, no-duplicate-heading)


1900-1900: Multiple headings with the same content

(MD024, no-duplicate-heading)


1904-1904: Multiple headings with the same content

(MD024, no-duplicate-heading)


1908-1908: Multiple headings with the same content

(MD024, no-duplicate-heading)


1910-1910: Multiple headings with the same content

(MD024, no-duplicate-heading)


1912-1912: Multiple headings with the same content

(MD024, no-duplicate-heading)


1916-1916: Multiple headings with the same content

(MD024, no-duplicate-heading)


1924-1924: Multiple headings with the same content

(MD024, no-duplicate-heading)


1928-1928: Multiple headings with the same content

(MD024, no-duplicate-heading)


1936-1936: Multiple headings with the same content

(MD024, no-duplicate-heading)


1940-1940: Multiple headings with the same content

(MD024, no-duplicate-heading)


1946-1946: Multiple headings with the same content

(MD024, no-duplicate-heading)


1957-1957: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


1981-1981: Multiple headings with the same content

(MD024, no-duplicate-heading)


1987-1987: Multiple headings with the same content

(MD024, no-duplicate-heading)


1997-1997: Multiple headings with the same content

(MD024, no-duplicate-heading)


2003-2003: Multiple headings with the same content

(MD024, no-duplicate-heading)


2009-2009: Multiple headings with the same content

(MD024, no-duplicate-heading)


2013-2013: Multiple headings with the same content

(MD024, no-duplicate-heading)


2027-2027: Multiple headings with the same content

(MD024, no-duplicate-heading)


2033-2033: Multiple headings with the same content

(MD024, no-duplicate-heading)


2039-2039: Multiple headings with the same content

(MD024, no-duplicate-heading)


2047-2047: Multiple headings with the same content

(MD024, no-duplicate-heading)


2053-2053: Multiple headings with the same content

(MD024, no-duplicate-heading)


2061-2061: Multiple headings with the same content

(MD024, no-duplicate-heading)


2067-2067: Multiple headings with the same content

(MD024, no-duplicate-heading)


2071-2071: Multiple headings with the same content

(MD024, no-duplicate-heading)


2075-2075: Multiple headings with the same content

(MD024, no-duplicate-heading)


2079-2079: Multiple headings with the same content

(MD024, no-duplicate-heading)


2083-2083: Multiple headings with the same content

(MD024, no-duplicate-heading)


2089-2089: Multiple headings with the same content

(MD024, no-duplicate-heading)


2093-2093: Multiple headings with the same content

(MD024, no-duplicate-heading)


2097-2097: Multiple headings with the same content

(MD024, no-duplicate-heading)


2101-2101: Multiple headings with the same content

(MD024, no-duplicate-heading)


2105-2105: Multiple headings with the same content

(MD024, no-duplicate-heading)


2109-2109: Multiple headings with the same content

(MD024, no-duplicate-heading)


2113-2113: Multiple headings with the same content

(MD024, no-duplicate-heading)


2117-2117: Multiple headings with the same content

(MD024, no-duplicate-heading)


2126-2126: Bare URL used

(MD034, no-bare-urls)


2128-2128: Bare URL used

(MD034, no-bare-urls)


2130-2130: Bare URL used

(MD034, no-bare-urls)


2132-2132: Bare URL used

(MD034, no-bare-urls)


2136-2136: Bare URL used

(MD034, no-bare-urls)


2138-2138: Bare URL used

(MD034, no-bare-urls)

🔇 Additional comments (8)
test/samples/02_sample_invoice.pdf.md (1)

1-37: This sample file appears misaligned with the PR objectives.

This invoice sample doesn't relate to the PR's focus on welcome-page A/B testing, onboarding, telemetry, and MCP bundling. Confirm whether this file was intended to be included in this PR, or if it was added accidentally.

test/samples/01_sample_simple.pdf.md (1)

1-12: LGTM!

Simple test sample file looks appropriate for testing PDF-to-markdown conversion.

test/ab-test.test.js (2)

1-5: Good test coverage for the A/B test system.

The test suite covers key edge cases: missing/undefined/empty experiments, malformed data, and deterministic assignment. The approach of recreating the logic with mock dependencies is pragmatic for testing ES modules.


185-204: Test correctly validates deterministic mutual exclusivity.

This test properly verifies that a user is assigned to exactly one variant of a two-variant experiment, and that the assignment is persisted.

src/utils/ab-test.ts (4)

39-67: Well-structured variant assignment with good defensive checks.

The implementation correctly:

  • Validates persisted assignments against current variant list (line 53)
  • Handles missing clientId gracefully
  • Caches results for session consistency

Note: If the remote experiment variants change and a user's persisted variant is no longer valid, they'll be re-assigned. This is reasonable behavior but worth documenting for experiment integrity.


72-83: LGTM!

The function correctly handles edge cases and uses optional chaining for null safety. The assumption that variant names are unique across experiments is reasonable for this use case.


88-100: LGTM!

The function correctly reads only persisted assignments without triggering new ones, which is appropriate for analytics reporting.


102-109: LGTM!

Standard djb2-style hash function appropriate for A/B test bucketing. The Math.abs() correctly handles the signed 32-bit integer from the bitwise operations.

Comment thread test/samples/03_sample_compex.pdf.md Outdated
Comment on lines +1 to +7
# arXiv:2302.11942v3 [q-fin.MF] 10 Mar 2023

## Liquidity Providers Greeks and Impermanent Gain

### Niccolò Bardoscia, Alessandro Nodari ∗

### 23 February 2023

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 | 🟡 Minor

Filename typo: "compex" should be "complex".

The filename 03_sample_compex.pdf.md contains a typo. Consider renaming to 03_sample_complex.pdf.md for clarity and discoverability.

🤖 Prompt for AI Agents
In test/samples/03_sample_compex.pdf.md around lines 1 to 7, the filename
contains a typo ("compex" should be "complex"); rename the file to
test/samples/03_sample_complex.pdf.md and update any references (tests,
fixtures, README, CI config) that mention the old filename to the new name to
ensure everything continues to resolve correctly.

Comment thread test/samples/URL Sample.md Outdated
@codeant-ai

codeant-ai Bot commented Dec 20, 2025

Copy link
Copy Markdown
Contributor

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@wonderwhy-er wonderwhy-er merged commit b2eefbe into main Dec 20, 2025
2 checks passed
@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

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant