Skip to content

feat(core): Add progressCallback parameter to runDefaultAction#1420

Merged
yamadashy merged 4 commits intomainfrom
feat/add-progress-callback-to-default-action
Apr 6, 2026
Merged

feat(core): Add progressCallback parameter to runDefaultAction#1420
yamadashy merged 4 commits intomainfrom
feat/add-progress-callback-to-default-action

Conversation

@yamadashy
Copy link
Copy Markdown
Owner

@yamadashy yamadashy commented Apr 6, 2026

Add an optional progressCallback parameter to runDefaultAction so callers can receive detailed progress messages from pack() (e.g., "Searching for files...", "Collecting files...", "Generating output...").

The callback is invoked alongside the existing spinner updates, so CLI behavior is unchanged. This enables programmatic consumers like the website server to stream fine-grained progress to clients.

Checklist

  • Run npm run test
  • Run npm run lint

Open with Devin

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

⚡ Performance Benchmark

Latest commit:cb00a7c test(core): Add test coverage for progressCallback in runDefaultAction
Status:✅ Benchmark complete!
Ubuntu:1.46s (±0.04s) → 1.46s (±0.03s) · -0.00s (-0.1%)
macOS:1.38s (±0.17s) → 1.38s (±0.20s) · +0.01s (+0.5%)
Windows:2.39s (±0.07s) → 2.33s (±0.38s) · -0.06s (-2.7%)
Details
  • Packing the repomix repository with node bin/repomix.cjs
  • Warmup: 2 runs (discarded), interleaved execution
  • Measurement: 20 runs / 30 on macOS (median ± IQR)
  • Workflow run
History

de2fce0 test(core): Add test coverage for progressCallback in runDefaultAction

Ubuntu:1.41s (±0.03s) → 1.40s (±0.03s) · -0.01s (-0.6%)
macOS:1.02s (±0.12s) → 1.09s (±0.21s) · +0.07s (+6.7%)
Windows:1.79s (±0.07s) → 1.80s (±0.05s) · +0.01s (+0.6%)

e2d2447 fix(core): Address PR review feedback

Ubuntu:1.47s (±0.03s) → 1.47s (±0.04s) · +0.00s (+0.2%)
macOS:0.86s (±0.03s) → 0.87s (±0.09s) · +0.01s (+0.7%)
Windows:2.26s (±0.48s) → 2.24s (±0.48s) · -0.02s (-1.0%)

eb256fb fix(core): Isolate progressCallback failures from pack flow

Ubuntu:1.46s (±0.03s) → 1.46s (±0.02s) · +0.00s (+0.0%)
macOS:1.48s (±0.17s) → 1.47s (±0.19s) · -0.01s (-0.6%)
Windows:1.80s (±0.03s) → 1.80s (±0.03s) · -0.00s (-0.2%)

68fdc17 feat(core): Add progressCallback parameter to runDefaultAction

Ubuntu:1.51s (±0.04s) → 1.49s (±0.05s) · -0.02s (-1.3%)
macOS:1.00s (±0.21s) → 0.99s (±0.33s) · -0.02s (-1.7%)
Windows:2.31s (±0.06s) → 2.29s (±0.06s) · -0.02s (-0.9%)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 6, 2026

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.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 82762e40-8c4f-4017-a73f-b830f69caa34

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

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The runDefaultAction function now accepts an optional progressCallback parameter of type RepomixProgressCallback. A new handleProgress function was introduced to update the CLI spinner and forward progress messages to the callback when provided, replacing the previous inline progress handler.

Changes

Cohort / File(s) Summary
CLI Progress Callback Integration
src/cli/actions/defaultAction.ts
Added optional progressCallback parameter to runDefaultAction function signature. Introduced handleProgress function that updates spinner and forwards progress messages to callback, then passed it to pack invocation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • PR #223 — Adds progressCallback support to pack and processFiles functions, directly complementing this change that threads the callback through the CLI layer.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding an optional progressCallback parameter to the runDefaultAction function.
Description check ✅ Passed The description covers the purpose, implementation approach, and impact of the change. Required checklist items are marked as completed. The description fully aligns with the template structure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/add-progress-callback-to-default-action

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.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Apr 6, 2026

Deploying repomix with  Cloudflare Pages  Cloudflare Pages

Latest commit: cb00a7c
Status: ✅  Deploy successful!
Preview URL: https://865dff56.repomix.pages.dev
Branch Preview URL: https://feat-add-progress-callback-t.repomix.pages.dev

View logs

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.32%. Comparing base (7b9e3d4) to head (cb00a7c).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1420      +/-   ##
==========================================
+ Coverage   87.26%   87.32%   +0.05%     
==========================================
  Files         117      117              
  Lines        4421     4426       +5     
  Branches     1021     1022       +1     
==========================================
+ Hits         3858     3865       +7     
+ Misses        563      561       -2     

☔ 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
Copy Markdown
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 updates the runDefaultAction function in src/cli/actions/defaultAction.ts to accept an optional progressCallback. The implementation refactors the progress handling during the pack operation to ensure that both the CLI spinner and the new callback are updated simultaneously. I have no feedback to provide.

coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 6, 2026

Code Review

Clean, minimal PR that threads an optional progressCallback through runDefaultAction for programmatic consumers. No breaking changes — all existing callers continue to work. Two items worth considering:

1. Type mismatch: void vs async handling

RepomixProgressCallback is typed as (message: string) => void, but the implementation defensively handles async callbacks with Promise.resolve(...).catch(...). Consider updating the type to (message: string) => void | Promise<void> so callers know async callbacks are supported.

2. Redundant outer try-catch

The outer try-catch around Promise.resolve(progressCallback(message)).catch(...) is unnecessary — Promise.resolve() never throws synchronously, and .catch() handles rejection. Simplifying to just the Promise chain (or just a plain try-catch if the callback is meant to be sync-only) would be cleaner.

Suggested simplification

If async callbacks should be supported:

const handleProgress: RepomixProgressCallback = (message) => {
  spinner.update(message);
  if (progressCallback) {
    Promise.resolve(progressCallback(message)).catch((error) => {
      logger.trace('progressCallback error:', error);
    });
  }
};

If callbacks are sync-only (matching the current type):

const handleProgress: RepomixProgressCallback = (message) => {
  spinner.update(message);
  if (progressCallback) {
    try {
      progressCallback(message);
    } catch (error) {
      logger.trace('progressCallback error:', error);
    }
  }
};
Additional notes from review agents
  • No breaking changes: All callers (cliRun.ts, remoteAction.ts, processZipFile.ts, tests) are backward compatible
  • No security issues: Callback only receives string messages, errors are isolated
  • No performance concerns: Benchmark shows no regression (-0.9% to -1.7%)
  • Test coverage: Codecov reports 50% patch coverage (2 uncovered error-handling lines). Adding a test for the callback invocation and error isolation paths would be worthwhile
  • Conventions: Commit messages follow Conventional Commits properly. The file was already over the 250-line guideline before this PR

Overall this is a solid, focused change. The two items above are suggestions, not blockers.

🤖 Generated with Claude Code

@yamadashy
Copy link
Copy Markdown
Owner Author

Addressed both suggestions in e2d2447b:

  1. Updated RepomixProgressCallback type to void | Promise<void> to explicitly indicate async callback support
  2. Removed redundant outer try-catch — Promise.resolve() handles both sync and async returns

Thanks for the thorough review! 🤖

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 6, 2026

Code Review (Follow-up)

Previous feedback was addressed in e2d2447 — type updated to void | Promise<void> and redundant try-catch removed. Implementation looks clean. One remaining item:

Test coverage for handleProgress

Codecov flags 25% patch coverage (6 uncovered lines). All existing tests pass undefined for progressCallback, and the pack mock never invokes its progress argument, so the handleProgress closure body is never exercised.

Three narrow test cases would close the gap:

  1. Callback is invoked — configure the pack mock to call its 3rd argument, assert the provided progressCallback receives the same message
  2. Error isolation — pass a rejecting async callback, verify pack still completes and logger.trace is called
  3. Spinner still updates on callback failure — verify spinner.update is called even when progressCallback throws

This matters because runDefaultAction is part of the public API (exported from src/index.ts), and the error-isolation contract (Promise.resolve(...).catch(...)) has no regression guard.

Additional notes from review agents
  • Security: No concerns — callback only receives string progress messages, errors are isolated, no sensitive data exposed
  • Performance: Fire-and-forget is correct for this use case. Benchmarks confirm no regression. Promise overhead is negligible (~15KB peak for large repos)
  • Conventions: Commit messages follow Conventional Commits properly. The file was already over the 250-line guideline before this PR (pre-existing)
  • Design: progressCallback is not forwarded through runRemoteAction — if remote pack progress is needed later, it will require a similar threading. Not a blocker for this PR

Overall this is a solid, focused change. The test coverage gap is the only actionable item — not a merge blocker, but recommended before or shortly after merge.

🤖 Generated with Claude Code

yamadashy and others added 4 commits April 6, 2026 21:56
Allow callers to receive detailed progress messages from pack()
(e.g., "Searching for files...", "Collecting files...",
"Generating output...") by passing an optional progressCallback.

The callback is invoked alongside the existing spinner updates,
so CLI behavior is unchanged. This enables programmatic consumers
like the website server to stream fine-grained progress to clients.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wrap progressCallback invocation with try/catch and Promise.catch
to prevent unhandled rejections from crashing the process when an
async callback is passed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update RepomixProgressCallback type to void | Promise<void> to
  explicitly indicate async callback support
- Remove redundant outer try-catch since Promise.resolve() handles
  both sync and async returns

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add three test cases for handleProgress:
- Callback is invoked with progress messages from pack()
- Async callback rejection is isolated (pack completes, error logged)
- Spinner still updates when callback throws synchronously

Also restore the outer try-catch for sync errors — Promise.resolve()
only catches async rejections, not synchronous throws from the
callback invocation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yamadashy yamadashy force-pushed the feat/add-progress-callback-to-default-action branch from de2fce0 to cb00a7c Compare April 6, 2026 12:56
@yamadashy yamadashy merged commit 3bb57ca into main Apr 6, 2026
59 checks passed
@yamadashy yamadashy deleted the feat/add-progress-callback-to-default-action branch April 6, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant