deno-denops-std icon indicating copy to clipboard operation
deno-denops-std copied to clipboard

:+1: Add accumulate() function for automatic RPC batching with parallel execution support

Open Milly opened this issue 4 months ago β€’ 4 comments

Summary

  • Add accumulate() function to the batch module that automatically batches multiple RPC calls
  • Support parallel execution with Promise.all() while maintaining automatic batching
  • Add AccumulateCancelledError for proper error handling in parallel execution scenarios

Background

The existing batch() and collect() functions require sequential execution and manual batching. The new accumulate() function enables more natural async/await patterns with automatic batching during microtask processing.

This function was originally developed in a separate repository and has been published on jsr.io with proven usage in various Denops plugins. Given its maturity and utility, we're now integrating it into the standard library.

Example Usage

const results = await accumulate(denops, async (denops) => {
  const lines = await fn.getline(denops, 1, "$");
  return await Promise.all(lines.map(async (line, index) => {
    const keyword = await fn.matchstr(denops, line, "\\k\\+");
    const len = await fn.len(denops, keyword);
    return { lnum: index + 1, keyword, len };
  }));
});

In this example, the following 3 RPCs are called:

  1. RPC call to getline
  2. Multiple matchstr calls in one RPC
  3. Multiple len calls in one RPC

Testing

  • Comprehensive test coverage including parallel execution scenarios
  • Error handling and cancellation behavior tested
  • Integration with existing batch() and collect() functions verified

Breaking Changes

None. This is a purely additive change.

Related

  • Original repository: https://github.com/Milly/denops-batch-accumulate
  • jsr.io package: https://jsr.io/@milly/denops-batch-accumulate

Additional Context

  • A rename to gather() was suggested by AI but rejected due to:
    • Historical conflict: A function with this name existed in earlier denops-std versions
    • Breaking change: The function is already in use as accumulate() in some plugin libraries

Summary by CodeRabbit

  • New Features

    • Introduced accumulate to batch multiple RPC calls within Denops.
    • Added AccumulateCancelledError to signal cancelled batched operations.
    • Exposed new batch utilities via public module exports.
  • Tests

    • Added comprehensive test suite covering accumulations, parallel/sequential behavior, error paths, and API surface.
  • Chores

    • Updated project config to include additional async utilities.

Milly avatar Oct 21 '25 14:10 Milly

Walkthrough

Adds an accumulate batching helper for Denops RPCs, a cancellation error type, tests, public exports, and an import-map alias for std/async. Provides AccumulateHelper with guarded batched call methods and an exported accumulate(denops, executor) API.

Changes

Cohort / File(s) Summary
Core accumulate implementation
batch/accumulate.ts
New module implementing AccumulateHelper that enqueues RPC Calls, resolves them via deferred ticks using denops.batch, exposes guarded call, batch, cmd, eval, and dispatch methods, normalizes results/errors, and exports accumulate(denops, executor).
Cancellation error
batch/error.ts
New AccumulateCancelledError and AccumulateCancelledErrorOptions to represent cancellation of pending batched calls; carries optional calls payload.
Test suite
batch/accumulate_test.ts
New comprehensive tests covering basic resolutions, collections, nested accumulate/batch/collect behavior, parallel/sequential flows, many error paths (executor, denops.batch failures, cancellations), API surface guards, and stack/trace expectations.
Module exports and docs
batch/mod.ts
Re-exports new accumulate and error modules and updates example usage to import the accumulate utility.
Import map / config
deno.jsonc
Adds import alias @std/async (version ^1.0.15) to the import map for async utilities used by accumulate implementation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Accumulate as accumulate()
    participant Helper as AccumulateHelper
    participant Scheduler as nextTick
    participant DenopsBatch as denops.batch()

    User->>Accumulate: accumulate(denops, executor)
    Accumulate->>Helper: new Helper(denops)
    Accumulate->>User: run executor(helper)

    loop executor schedules batched calls
        User->>Helper: helper.call / .batch / .cmd / .eval / .dispatch
        Helper->>Helper: enqueue Call, return Promise
        Helper->>Scheduler: schedule `#resolvePendingCalls`
    end

    Scheduler->>Helper: `#resolvePendingCalls`
    Helper->>DenopsBatch: denops.batch(enqueued calls)
    DenopsBatch-->>Helper: results[] or BatchError

    alt success
        Helper->>Helper: map results, resolve Promises
        Helper-->>User: Promises resolved
    else batch error / cancellation
        Helper->>Helper: wrap/convert errors (BatchError | AccumulateCancelledError)
        Helper-->>User: Promises rejected
    end

    User->>Accumulate: executor returns
    Accumulate->>Helper: close() / cleanup
    Accumulate-->>User: final awaited result

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas needing extra attention:
    • Error wrapping/propagation logic in accumulate.ts (BatchError vs generic errors vs AccumulateCancelledError).
    • Promise resolution queueing and nextTick scheduling (#waitResolved / #resolvePendingCalls).
    • Tests asserting stack/trace contents and cancellation semantics in batch/accumulate_test.ts.
    • Public API guards preventing helper usage outside accumulate block.

Poem

🐰 I queued my hops in tidy rows,

Batched each call where soft wind blows.
If one fell down, I marked the restβ€”
Cancelled gently, laid to rest.
Hops resolved, the rabbit knows.

Pre-merge checks and finishing touches

βœ… Passed checks (3 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title accurately describes the main change: introducing an accumulate() function for automatic RPC batching with parallel execution support, which aligns with the primary objective and implementation of the PR.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 batch-accumulate

πŸ“œ Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between c4da17500b74c5e26330f8a983b431c9f3f3d4ab and 5d6645ec81edec38495e248d47f6560a3385b4c4.

πŸ“’ Files selected for processing (1)
  • batch/mod.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • batch/mod.ts
⏰ 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). (6)
  • GitHub Check: test (windows-latest, 2.x, v9.1.1646, v0.11.3)
  • GitHub Check: test (ubuntu-latest, 2.x, v9.1.1646, v0.11.3)
  • GitHub Check: test (ubuntu-latest, ~2.3, v9.1.1646, v0.11.3)
  • GitHub Check: test (macos-latest, 2.x, v9.1.1646, v0.11.3)
  • GitHub Check: test (windows-latest, ~2.3, v9.1.1646, v0.11.3)
  • GitHub Check: test (macos-latest, ~2.3, v9.1.1646, v0.11.3)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

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

coderabbitai[bot] avatar Oct 21 '25 14:10 coderabbitai[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 85.73%. Comparing base (d8bfd9d) to head (5d6645e). :warning: Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #297      +/-   ##
==========================================
+ Coverage   84.82%   85.73%   +0.91%     
==========================================
  Files          64       66       +2     
  Lines        2879     3064     +185     
  Branches      281      306      +25     
==========================================
+ Hits         2442     2627     +185     
  Misses        435      435              
  Partials        2        2              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Oct 21 '25 14:10 codecov[bot]

accumulate() is not a replacement for batch() and collect(), but rather a complementary function for different use cases.

Each serves a specific purpose:

  • batch() - Manual batching for side-effects only (commands, settings). Most efficient when return values aren't needed.
  • collect() - Simple parallel value collection with type-safe returns. Cleanest API for fetching multiple fixed values at once.
  • accumulate() - Automatic batching for complex async flows with loops and conditions. Writes natural async code while automatically optimizing RPC calls in the background.

They complement each other nicely - developers can choose based on their specific needs. I don't think we need to deprecate the existing functions. What do you think?

Milly avatar Nov 01 '25 18:11 Milly

They complement each other nicely - developers can choose based on their specific needs. I don't think we need to deprecate the existing functions. What do you think?

OK. Then, we should have clear documentation for that difference.

lambdalisue avatar Nov 05 '25 01:11 lambdalisue

They complement each other nicely - developers can choose based on their specific needs. I don't think we need to deprecate the existing functions. What do you think?

OK. Then, we should have clear documentation for that difference.

Wrote to module document (JSDoc in batch/mod.ts).

Milly avatar Nov 15 '25 05:11 Milly