Skip to content

refactor(core): Consolidate worker pool arguments into WorkerOptions interface#816

Merged
yamadashy merged 5 commits intomainfrom
feat/worker
Aug 31, 2025
Merged

refactor(core): Consolidate worker pool arguments into WorkerOptions interface#816
yamadashy merged 5 commits intomainfrom
feat/worker

Conversation

@yamadashy
Copy link
Owner

Consolidates the arguments for createWorkerPool and initTaskRunner functions into a single WorkerOptions interface for improved type safety and usability.

Changes

  • Added WorkerOptions interface with numOfTasks, workerPath, and optional runtime properties
  • Refactored createWorkerPool to accept a single WorkerOptions object instead of separate parameters
  • Refactored initTaskRunner to use the same WorkerOptions interface
  • Updated all call sites across the codebase to use the new interface:
    • File collection and processing workers
    • Metrics calculation workers (git diff, git log, output, selective file metrics)
    • Security check workers
    • Globby execution workers
  • Updated tests to reflect the new API

Benefits

  • Improved type safety: Named parameters prevent argument order mistakes
  • Better readability: Clear parameter names make code more self-documenting
  • Enhanced maintainability: Consistent interface across all worker pool usage
  • Future extensibility: Easy to add new options without breaking existing code

Checklist

  • Run npm run test
  • Run npm run lint

Add WorkerRuntime type and configurable runtime parameter to createWorkerPool and initTaskRunner functions. This allows choosing between 'worker_threads' and 'child_process' runtimes based on performance requirements.

- Add WorkerRuntime type definition for type safety
- Add optional runtime parameter to createWorkerPool with child_process default
- Add optional runtime parameter to initTaskRunner with child_process default
- Configure fileCollectWorker to use worker_threads for better performance
- Update all test files to use WorkerRuntime type
- Add comprehensive tests for runtime parameter functionality
- Maintain backward compatibility with existing code

The fileCollectWorker now benefits from worker_threads faster startup and shared memory, while other workers continue using child_process for stability.
…interface

- Add WorkerOptions interface to combine numOfTasks, workerPath, and optional runtime
- Update createWorkerPool and initTaskRunner functions to accept WorkerOptions object
- Refactor all usage sites across file processing, metrics, and security modules
- Update corresponding test cases to use new interface

This improves type safety and makes the API more maintainable by avoiding parameter order mistakes.
Copilot AI review requested due to automatic review settings August 31, 2025 07:28
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 31, 2025

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.

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Refactors the concurrency API from positional parameters to an options object with optional runtime selection. Updates all callers and tests accordingly. Adds cleanupWorkerPool and Bun-specific destroy handling. Adjusts a benchmark’s default iteration count. File collection explicitly requests worker_threads runtime; other call sites keep defaults.

Changes

Cohort / File(s) Summary of change
Benchmark defaults
benchmarks/memory/src/memory-test.ts
Increased non-full default iterations 50→100; full remains 200. No API changes.
Concurrency API implementation
src/shared/processConcurrency.ts
Switched createWorkerPool/initTaskRunner to options object { numOfTasks, workerPath, runtime? }. Added WorkerRuntime, WorkerOptions, runtime propagation to Tinypool (default 'child_process'), workerData with logLevel, init timing logs, Bun-aware cleanupWorkerPool.
File collection (explicit runtime)
src/core/file/fileCollect.ts
Replaced initTaskRunner positional args with options and set runtime: 'worker_threads'.
Core file processing
src/core/file/fileProcess.ts
Migrated initTaskRunner to options object; same values as before.
Globbing execution
src/core/file/globbyExecute.ts
Migrated initTaskRunner to options object; numOfTasks: 1, same worker path.
Metrics: git diff/log
src/core/metrics/calculateGitDiffMetrics.ts, src/core/metrics/calculateGitLogMetrics.ts
Switched initTaskRunner to options object; semantics unchanged (numOfTasks: 1).
Metrics: output/selective files
src/core/metrics/calculateOutputMetrics.ts, src/core/metrics/calculateSelectiveFileMetrics.ts
Replaced positional args with options { numOfTasks, workerPath }; behavior unchanged.
Security check
src/core/security/securityCheck.ts
Updated initTaskRunner to options form; computes numOfTasks as before.
Tests: file collection
tests/core/file/fileCollect.test.ts
Added WorkerRuntime type, extended mock to accept/record runtime, verify 'worker_threads' is passed. Mock now returns cleanup.
Tests: core file/metrics/security
tests/core/file/fileProcess.test.ts, tests/core/metrics/calculateOutputMetrics.test.ts, tests/core/metrics/calculateSelectiveFileMetrics.test.ts, tests/core/security/securityCheck.test.ts
Imported WorkerRuntime, updated mocks to accept optional runtime (and return cleanup where applicable); no behavioral changes to assertions except runtime typing support.
Tests: integration packager
tests/integration-tests/packager.test.ts
Extended mock init to accept optional runtime; type-only import of WorkerRuntime.
Tests: concurrency layer
tests/shared/processConcurrency.test.ts
Updated for new options API; added assertions for runtime propagation and defaults into Tinypool.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller as Caller (e.g., fileCollect)
  participant PC as processConcurrency.initTaskRunner
  participant WP as createWorkerPool
  participant TP as Tinypool
  participant W as Worker(s)

  rect rgb(240,245,255)
    note over Caller,PC: Initialization
    Caller->>PC: initTaskRunner({ numOfTasks, workerPath, runtime? })
    PC->>WP: createWorkerPool({ numOfTasks, workerPath, runtime? })
    WP->>TP: new Tinypool({ filename: workerPath, min/maxThreads, runtime, workerData })
    TP-->>WP: pool instance
    WP-->>PC: pool
    PC-->>Caller: { run(tasks), cleanup() }
  end

  rect rgb(242,255,242)
    note over Caller,W: Execution
    Caller->>PC: run(tasks)
    PC->>TP: queue tasks
    TP->>W: execute
    W-->>TP: results
    TP-->>PC: aggregated results
    PC-->>Caller: results
  end

  rect rgb(255,248,240)
    note over Caller,TP: Cleanup
    Caller->>PC: cleanup()
    PC->>TP: destroy() (skipped on Bun)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

enhancement

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/worker

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@claude
Copy link
Contributor

claude bot commented Aug 31, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the core worker pool management by consolidating arguments for worker creation and task initialization into a single, type-safe WorkerOptions interface. This change aims to improve the overall structure, readability, and maintainability of the codebase by providing a consistent and extensible way to configure worker processes.

Highlights

  • New WorkerOptions Interface: A dedicated WorkerOptions interface has been introduced to encapsulate worker configuration parameters, improving type safety and clarity.
  • Unified Worker Pool API: The createWorkerPool and initTaskRunner functions now accept a single WorkerOptions object instead of separate parameters, streamlining their API.
  • Widespread Adoption: All existing call sites across the codebase that create or initialize worker pools have been updated to utilize the new WorkerOptions interface.
  • Enhanced Maintainability: The consistent interface across all worker pool usage makes the code more self-documenting and easier to maintain and extend in the future.
  • Test Updates: Corresponding tests have been modified to reflect the new API structure and ensure proper functionality with the WorkerOptions interface.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the worker pool creation API by consolidating function parameters into a single WorkerOptions interface. This improves type safety and makes the code more maintainable.

  • Introduces a WorkerOptions interface with numOfTasks, workerPath, and optional runtime properties
  • Updates createWorkerPool and initTaskRunner functions to accept the new interface instead of separate parameters
  • Updates all call sites across the codebase to use the new object-based API

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/shared/processConcurrency.ts Adds WorkerOptions interface and refactors worker pool functions to use it
src/core/file/fileCollect.ts Updates to use new API and sets runtime to 'worker_threads'
src/core/file/fileProcess.ts Updates function call to use new object-based API
src/core/file/globbyExecute.ts Updates function call to use new object-based API
src/core/metrics/*.ts Updates all metrics calculation functions to use new API
src/core/security/securityCheck.ts Updates function call to use new object-based API
tests/shared/processConcurrency.test.ts Adds comprehensive tests for new API and runtime parameter
tests/core/**/*.test.ts Updates mock functions to handle new API signature
tests/integration-tests/packager.test.ts Updates mock function signature
benchmarks/memory/src/memory-test.ts Minor change to default iteration count

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request does a great job of refactoring the worker pool creation by consolidating arguments into a WorkerOptions interface. This significantly improves type safety and readability. The core implementation in src/shared/processConcurrency.ts and the updates to the call sites are well-executed.

My main feedback is regarding the test files. While new tests have been added to cover the changes, the mock implementations for initTaskRunner in several test files have not been updated to match the new function signature (i.e., accepting a single options object). This is a critical oversight that could lead to tests passing incorrectly. I've left specific comments and suggestions in each affected test file to address this. Once these are fixed, the PR will be in excellent shape.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 31, 2025

Deploying repomix with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4d9b52c
Status: ✅  Deploy successful!
Preview URL: https://48b01f33.repomix.pages.dev
Branch Preview URL: https://feat-worker.repomix.pages.dev

View logs

@codecov
Copy link

codecov bot commented Aug 31, 2025

Codecov Report

❌ Patch coverage is 68.42105% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.59%. Comparing base (3e67817) to head (4d9b52c).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/core/file/globbyExecute.ts 0.00% 4 Missing ⚠️
src/core/metrics/calculateGitDiffMetrics.ts 0.00% 4 Missing ⚠️
src/core/metrics/calculateGitLogMetrics.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #816   +/-   ##
=======================================
  Coverage   87.59%   87.59%           
=======================================
  Files         113      113           
  Lines        6602     6604    +2     
  Branches     1372     1372           
=======================================
+ Hits         5783     5785    +2     
  Misses        819      819           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

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

⚠️ Outside diff range comments (4)
tests/core/file/fileProcess.test.ts (1)

23-32: Fix mockInitTaskRunner signature to accept WorkerOptions.

The production code now calls initTaskRunner(optionsObject). Update the mock to match; otherwise TS typing will be off under strict settings.

-const mockInitTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
-  return {
-    run: async (task: T) => {
-      return (await fileProcessWorker(task as FileProcessTask)) as R;
-    },
-    cleanup: async () => {
-      // Mock cleanup - no-op for tests
-    },
-  };
-};
+const mockInitTaskRunner = <T, R>(_options: WorkerOptions): TaskRunner<T, R> => ({
+  run: async (task: T) => (await fileProcessWorker(task as FileProcessTask)) as R,
+  cleanup: async () => {
+    // Mock cleanup - no-op for tests
+  },
+});
tests/core/security/securityCheck.test.ts (2)

43-52: Fix mockInitTaskRunner signature to accept WorkerOptions (object).

Tests currently model the old positional signature, which will break type checking and behavior.

-const mockInitTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
+const mockInitTaskRunner = <T, R>(_options: WorkerOptions) => {
   return {
     run: async (task: T) => {
       return (await securityCheckWorker(task as SecurityCheckTask)) as R;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };

82-91: Update error-path mock to the new WorkerOptions signature.

Align the mock to receive a single options object.

-const mockErrorTaskRunner = (_numOfTasks?: number, _workerPath?: string, _runtime?: WorkerRuntime) => {
+const mockErrorTaskRunner = (_options?: WorkerOptions) => {
   return {
     run: async () => {
       throw mockError;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };
tests/core/file/fileCollect.test.ts (1)

36-47: Adapt mockInitTaskRunner implementation to accept WorkerOptions and record runtime.

This will make the “worker_threads runtime” test pass and address the lint error.

-const mockInitTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string, runtime?: WorkerRuntime) => {
-  // Store runtime for verification in tests
-  (mockInitTaskRunner as MockInitTaskRunner).lastRuntime = runtime;
+const mockInitTaskRunner = <T, R>(options: WorkerOptions) => {
+  // Store runtime for verification in tests
+  (mockInitTaskRunner as MockInitTaskRunner).lastRuntime = options.runtime;
   return {
     run: async (task: T) => {
       return (await fileCollectWorker(task as FileCollectTask)) as R;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };
🧹 Nitpick comments (12)
src/shared/processConcurrency.ts (2)

34-61: Pool initialization: solid defaults; consider Readonly options and stricter validation.

  • Nice: min/max threads derived from workload; logging includes runtime; workerData carries logLevel.
  • Optional: mark WorkerOptions fields as readonly (and accept Readonly<WorkerOptions>) to discourage mutation after construction; validate workerPath non-empty.
-export interface WorkerOptions {
-  numOfTasks: number;
-  workerPath: string;
-  runtime?: WorkerRuntime;
-}
+export interface WorkerOptions {
+  readonly numOfTasks: number;
+  readonly workerPath: string;
+  readonly runtime?: WorkerRuntime;
+}

63-82: Bun guard looks fine; consider idempotent destroy.

Skipping destroy under Bun is pragmatic. To be extra safe across reuse, you could catch double-destroy by tracking a local flag or accepting that Tinypool handles it—non-blocking.

src/core/file/fileProcess.ts (1)

24-27: Switched to WorkerOptions correctly; consider hoisting worker URL.

This aligns with the new API. Minor: compute the worker URL once at module scope to avoid recreating it per call.

-  const taskRunner = deps.initTaskRunner<FileProcessTask, ProcessedFile>({
-    numOfTasks: rawFiles.length,
-    workerPath: new URL('./workers/fileProcessWorker.js', import.meta.url).href,
-  });
+  const workerPath = new URL('./workers/fileProcessWorker.js', import.meta.url).href;
+  const taskRunner = deps.initTaskRunner<FileProcessTask, ProcessedFile>({
+    numOfTasks: rawFiles.length,
+    workerPath,
+  });
tests/integration-tests/packager.test.ts (1)

34-43: Align all mockInitTaskRunner signatures to the new options-style API

Several tests still define mockInitTaskRunner (or variants) with positional parameters. To prevent type drift from the production API—which now takes a single options object—update each mock to accept an opts: { numOfTasks: number; workerPath: string; runtime?: WorkerRuntime } instead of (_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime).
Applies to:

  • tests/integration-tests/packager.test.ts
  • tests/core/security/securityCheck.test.ts
  • tests/core/file/fileProcess.test.ts
  • tests/core/file/fileCollect.test.ts
  • tests/core/metrics/calculateSelectiveFileMetrics.test.ts
  • tests/core/metrics/calculateOutputMetrics.test.ts
tests/core/metrics/calculateSelectiveFileMetrics.test.ts (1)

13-22: Update mock to accept a single options object.

Keep tests aligned with the refactored runner signature.

-const mockInitTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
+type InitOpts = { numOfTasks: number; workerPath: string; runtime?: WorkerRuntime };
+const mockInitTaskRunner = <T, R>(_opts: InitOpts) => {
   return {
     run: async (task: T) => {
       return (await fileMetricsWorker(task as FileMetricsTask)) as R;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };
tests/core/metrics/calculateOutputMetrics.test.ts (5)

10-19: Switch mock to options-style signature.

-const mockInitTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
+type InitOpts = { numOfTasks: number; workerPath: string; runtime?: WorkerRuntime };
+const mockInitTaskRunner = <T, R>(_opts: InitOpts) => {
   return {
     run: async (task: T) => {
       return (await outputMetricsWorker(task as OutputMetricsTask)) as R;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };

50-59: Unify erroring mock with options-style signature.

-const mockErrorTaskRunner = <T, _R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
+const mockErrorTaskRunner = <T, _R>(_opts: InitOpts) => {
   return {
     run: async (_task: T) => {
       throw mockError;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };

100-111: Parallel mock: adopt options object for consistency.

-const mockParallelTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
+const mockParallelTaskRunner = <T, R>(_opts: InitOpts) => {
   return {
     run: async (_task: T) => {
       chunksProcessed++;
       // Return a fixed token count for each chunk
       return 100 as R;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };

126-135: Parallel error mock: adopt options object.

-const mockErrorTaskRunner = <T, _R>(_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) => {
+const mockErrorTaskRunner = <T, _R>(_opts: InitOpts) => {
   return {
     run: async (_task: T) => {
       throw mockError;
     },
     cleanup: async () => {
       // Mock cleanup - no-op for tests
     },
   };
 };

151-162: Convert TaskRunner mocks to use an options object

  • Replace (_numOfTasks: number, _workerPath: string, _runtime?: WorkerRuntime) with (_opts: InitOpts) in all TaskRunner mocks (e.g. mockChunkTrackingTaskRunner, mockCollectFileInitTaskRunner, mockInitTaskRunner, mockParallelTaskRunner and their error variants).
src/core/file/fileCollect.ts (1)

26-30: Explicit worker_threads runtime: confirm intent and environment support

Hard-coding runtime: 'worker_threads' here while other call sites use the default ('child_process') creates an asymmetry. Please confirm this is intentional for file-collection performance, and that all target environments (CI, Node versions, any Bun usage) fully support worker_threads for this worker. If you want runtime selection to be user-tunable, consider threading it from config/CLI instead of inlining here.

tests/shared/processConcurrency.test.ts (1)

74-87: Stabilize expectation: avoid hard-coding numeric log level

workerData.logLevel: 2 is brittle and ties the test to the logger’s current numeric mapping. Prefer expect.any(Number) (or mock the logger) to keep the test resilient.

Apply within the existing assertion:

-      expect(Tinypool).toHaveBeenCalledWith({
+      expect(Tinypool).toHaveBeenCalledWith({
         filename: workerPath,
         runtime: 'child_process',
         minThreads: 1,
         maxThreads: 4, // Math.min(4, 500/100) = 4
         idleTimeout: 5000,
         workerData: {
-          logLevel: 2,
+          logLevel: expect.any(Number),
         },
       });
📜 Review details

Configuration used: Path: .coderabbit.yaml

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 3e67817 and 25d65df.

📒 Files selected for processing (17)
  • benchmarks/memory/src/memory-test.ts (1 hunks)
  • src/core/file/fileCollect.ts (1 hunks)
  • src/core/file/fileProcess.ts (1 hunks)
  • src/core/file/globbyExecute.ts (1 hunks)
  • src/core/metrics/calculateGitDiffMetrics.ts (1 hunks)
  • src/core/metrics/calculateGitLogMetrics.ts (1 hunks)
  • src/core/metrics/calculateOutputMetrics.ts (1 hunks)
  • src/core/metrics/calculateSelectiveFileMetrics.ts (1 hunks)
  • src/core/security/securityCheck.ts (1 hunks)
  • src/shared/processConcurrency.ts (3 hunks)
  • tests/core/file/fileCollect.test.ts (3 hunks)
  • tests/core/file/fileProcess.test.ts (2 hunks)
  • tests/core/metrics/calculateOutputMetrics.test.ts (5 hunks)
  • tests/core/metrics/calculateSelectiveFileMetrics.test.ts (1 hunks)
  • tests/core/security/securityCheck.test.ts (3 hunks)
  • tests/integration-tests/packager.test.ts (1 hunks)
  • tests/shared/processConcurrency.test.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
tests/core/metrics/calculateSelectiveFileMetrics.test.ts (1)
src/shared/processConcurrency.ts (1)
  • WorkerRuntime (5-5)
tests/integration-tests/packager.test.ts (1)
src/shared/processConcurrency.ts (1)
  • WorkerRuntime (5-5)
src/core/file/fileProcess.ts (2)
src/core/file/workers/fileProcessWorker.ts (2)
  • FileProcessTask (10-13)
  • FileProcessTask (15-21)
src/core/file/fileTypes.ts (1)
  • ProcessedFile (6-9)
src/core/metrics/calculateSelectiveFileMetrics.ts (1)
src/core/metrics/workers/types.ts (1)
  • FileMetrics (1-5)
tests/core/file/fileCollect.test.ts (3)
src/shared/processConcurrency.ts (1)
  • WorkerRuntime (5-5)
tests/testing/testUtils.ts (1)
  • createMockConfig (15-45)
src/core/file/fileCollect.ts (1)
  • collectFiles (17-82)
tests/core/security/securityCheck.test.ts (1)
src/shared/processConcurrency.ts (1)
  • WorkerRuntime (5-5)
src/core/metrics/calculateGitLogMetrics.ts (1)
src/core/metrics/workers/gitLogMetricsWorker.ts (2)
  • GitLogMetricsTask (9-12)
  • GitLogMetricsTask (14-37)
tests/shared/processConcurrency.test.ts (1)
src/shared/processConcurrency.ts (2)
  • createWorkerPool (34-61)
  • initTaskRunner (89-95)
src/core/security/securityCheck.ts (1)
src/core/security/workers/securityCheckWorker.ts (3)
  • SecurityCheckTask (13-17)
  • SecurityCheckTask (25-42)
  • SuspiciousFileResult (19-23)
tests/core/file/fileProcess.test.ts (1)
src/shared/processConcurrency.ts (1)
  • WorkerRuntime (5-5)
src/core/file/fileCollect.ts (1)
src/core/file/workers/fileCollectWorker.ts (3)
  • FileCollectTask (10-14)
  • FileCollectTask (26-51)
  • FileCollectResult (21-24)
tests/core/metrics/calculateOutputMetrics.test.ts (1)
src/shared/processConcurrency.ts (1)
  • WorkerRuntime (5-5)
src/core/metrics/calculateGitDiffMetrics.ts (1)
src/core/metrics/workers/gitDiffMetricsWorker.ts (2)
  • GitDiffMetricsTask (9-13)
  • GitDiffMetricsTask (15-37)
src/core/metrics/calculateOutputMetrics.ts (1)
src/core/metrics/workers/outputMetricsWorker.ts (2)
  • OutputMetricsTask (9-13)
  • OutputMetricsTask (15-26)
src/shared/processConcurrency.ts (1)
src/shared/logger.ts (1)
  • logger (89-89)
🪛 GitHub Check: Test with Bun (ubuntu-latest, latest)
tests/core/file/fileCollect.test.ts

[failure] 228-228: tests/core/file/fileCollect.test.ts > fileCollect > should use worker_threads runtime when calling initTaskRunner
AssertionError: expected undefined to be 'worker_threads' // Object.is equality

  • Expected:
    "worker_threads"
  • Received:
    undefined

❯ tests/core/file/fileCollect.test.ts:228:68

🪛 GitHub Check: Lint TypeScript
tests/core/file/fileCollect.test.ts

[failure] 225-225:
Type '<T, R>(_numOfTasks: number, _workerPath: string, runtime?: WorkerRuntime | undefined) => { run: (task: T) => Promise; cleanup: () => Promise; }' is not assignable to type '<T, R>(options: WorkerOptions) => TaskRunner<T, R>'.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build and run (windows-latest, 24.x)
🔇 Additional comments (15)
src/shared/processConcurrency.ts (2)

89-95: API consolidation via options object: LGTM.

The generic TaskRunner shape with explicit cleanup is clear and consistent with call sites.


2-2: Tinypool runtime option supported in v1.1.1. Verified that the extracted dist/index.d.ts declares runtime?: 'worker_threads' | 'child_process' and the implementation in dist/index.js switches on that option at runtime.

src/core/file/globbyExecute.ts (1)

16-19: API migration to options object: LGTM.

Defaults to child_process runtime, which is appropriate here for memory isolation. Cleanup retained.

src/core/metrics/calculateGitDiffMetrics.ts (1)

26-29: Good migration to options object; confirm workerPath contract.

API switch to { numOfTasks, workerPath } looks correct and cleanup is handled. Please confirm workerPath accepts a file URL (.href) in all runtimes, or convert to a filesystem path earlier if required by the pool implementation.

tests/integration-tests/packager.test.ts (1)

27-27: Type-only import is fine.

Importing WorkerRuntime for test typing is OK.

tests/core/metrics/calculateSelectiveFileMetrics.test.ts (1)

6-6: Type import OK.

Using WorkerRuntime for typing is consistent with the new API.

src/core/metrics/calculateGitLogMetrics.ts (1)

31-34: Refactor looks correct; ensure URL handling matches pool expectations.

Options object usage and cleanup are correct. As with diff metrics, verify that workerPath as a file URL is supported across runtimes.

tests/core/metrics/calculateOutputMetrics.test.ts (1)

6-6: Type import OK.

WorkerRuntime type import is appropriate for test doubles.

src/core/metrics/calculateOutputMetrics.ts (1)

19-22: Good migration to WorkerOptions.

The options-object refactor is correct and consistent with the new API. No functional regressions spotted here.

src/core/metrics/calculateSelectiveFileMetrics.ts (1)

26-29: Looks good — consistent with the new API.

Passing { numOfTasks, workerPath } via WorkerOptions matches the refactor plan and maintains behavior.

tests/core/file/fileCollect.test.ts (1)

214-229: Test intent is correct; it will pass once the mock is updated.

After switching the mock to accept WorkerOptions and record options.runtime, lastRuntime should equal 'worker_threads'.

src/core/security/securityCheck.ts (1)

58-61: Good consistency with WorkerOptions.

The refactor to pass { numOfTasks, workerPath } via options is correct and preserves behavior.

tests/shared/processConcurrency.test.ts (3)

91-106: Good coverage of runtime propagation to Tinypool

Asserting the exact runtime: 'worker_threads' in the Tinypool config validates the new options path. LGTM.


123-124: Options-object API usage looks correct

initTaskRunner({ numOfTasks, workerPath }) matches the new signature and default-runtime behavior. LGTM.


131-142: Runtime plumbed through initTaskRunner correctly

Test accurately verifies that runtime is forwarded. LGTM.

@yamadashy yamadashy merged commit 8f7a7d9 into main Aug 31, 2025
58 checks passed
@yamadashy yamadashy deleted the feat/worker branch August 31, 2025 08:12
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.

2 participants