fix(step): only auto-batch when rendered command exceeds ARG_MAX by jdx · Pull Request #901 · jdx/hk

jdx · 2026-05-01T15:51:55Z

Summary

Auto-batching previously split jobs based on the size of the file-list expansion alone, even when the run command never interpolated {{files}}. On Windows where ARG_MAX falls back to 128KB, a hook step like:

local vscodeCommitHint = new Step {
  exclusive = true
  check = "echo If you see this message in a pop-up, the pre-commit steps failed."
}

…running on a large repo (~20K files) would be split into ~29 jobs and the message would print 29 times. The check command doesn't reference {{files}} at all, so there's no reason to batch.

What changed

Move auto-batching out of build_step_jobs and into a new Step::auto_batch_jobs invoked from execution time, where the full tera context is available.
The new implementation renders the actual run command with each candidate file subset and only splits jobs whose rendered command exceeds the safe ARG_MAX limit.
Falls back to the old byte estimation if rendering fails (template references a variable not yet in context), preserving the previous behavior as a safety net.
auto_batch_jobs also now preserves check_first and workspace_indicator across split jobs (the old code dropped them with a TODO note).

Test plan

Existing test/auto_batch_large_files.bats tests still pass (the echo {{files}} cases still get split).
New regression test: step with check = "echo hello world" + 1000 long-path files runs as 1 job, not many.
Manual repro of the original bug shows the static-message step now produces a single job (201 files – ... – echo ...) instead of many.

🤖 Generated with Claude Code

Note

Medium Risk
Changes when and how step jobs are split, which can affect execution concurrency and command invocation for large repos; includes a fallback estimator and new regression test to reduce risk.

Overview
Auto-batching is changed to render the actual step run command (with the full tera context) and only split jobs when that rendered command would exceed the safe ARG_MAX limit, avoiding unnecessary batching for steps that don’t interpolate {{files}}.

The batching logic is moved from job construction into execution time, adds a render-based binary search with a size-estimation fallback, and preserves per-job state (e.g. check_first and workspace_indicator) when creating split jobs. A new Bats regression test ensures static commands stay single-job even with huge file sets.

^{Reviewed by Cursor Bugbot for commit 861818b. Bugbot is set up for automated code reviews on this repo. Configure here.}

Auto-batching previously split jobs based on the size of the file-list expansion alone — even when the run command never interpolated `{{files}}`. On Windows (ARG_MAX falls back to 128KB), a hook step like `check = "echo static-message"` running on a large repo (~20K files) would be split into ~29 jobs and the message would print 29 times. Move auto-batching out of `build_step_jobs` and into a new `auto_batch_jobs` invoked from execution time, where the full tera context is available. The new implementation renders the actual run command for each job and only splits jobs whose rendered command would exceed the safe ARG_MAX limit, falling back to the previous byte estimation if rendering fails. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

gemini-code-assist

Code Review

This pull request refines the auto-batching logic for shell commands by calculating the actual size of the rendered command rather than relying solely on file list estimation. This change prevents unnecessary job splitting for commands that do not interpolate {{files}}. The batching logic is moved to the execution phase to utilize the full template context. A review comment suggests returning a zero size for empty commands to avoid triggering the fallback estimation logic unnecessarily.

gemini-code-assist · 2026-05-01T15:54:09Z

+        if run.trim().is_empty() {
+            return None;
+        }


Returning None for empty or whitespace-only commands triggers the fallback to estimate_files_string_size. If the file list is large, this will cause the job to be unnecessarily batched, which is exactly what this PR aims to avoid for commands that don't reference {{files}}. Returning Some(0) would correctly indicate that the command fits within the limit.

Suggested change

if run.trim().is_empty() {

return None;

}

if run.trim().is_empty() {

return Some(0);

}

greptile-apps · 2026-05-01T15:54:51Z

Greptile Summary

This PR fixes a bug where steps with static run commands (no {{files}} interpolation) were unnecessarily split into many batches solely because the file list was large. Auto-batching is moved from build_step_jobs to execution time, where tera rendering is used to measure the actual rendered command length before deciding to split.

Confidence Score: 5/5

Safe to merge — the fix is well-targeted, the fallback preserves previous safety-net behavior, and a regression test covers the reported bug.

No P0 or P1 issues found. The render-based sizing strategy is sound, the binary search logic is correct (including edge cases where even 1 file exceeds the limit), and previously dropped fields (check_first, workspace_indicator, skip_reason) are now correctly preserved on split jobs.

No files require special attention.

Important Files Changed

Filename	Overview
src/step/batching.rs	Core batching logic refactored: render-based sizing replaces pure file-list estimation; now preserves check_first, skip_reason, and workspace_indicator on split jobs.
src/step/execution.rs	auto_batch_jobs moved to after build_step_jobs so the full tera context is available; semaphore assignment unchanged.
src/step/job_builder.rs	Removed inline auto-batch call and updated doc comment to reflect deferred batching.
src/step_job.rs	Added workspace_indicator() getter to expose previously private field for batching code.
test/auto_batch_large_files.bats	Added regression test verifying a static-command step with 1000 long-path files produces exactly 1 job.

Sequence Diagram

sequenceDiagram
    participant E as execution.rs
    participant JB as job_builder.rs
    participant B as batching.rs
    participant T as tera

    E->>JB: build_step_jobs(files, run_type)
    JB-->>E: jobs (workspace/batch split, check_first set)
    E->>B: auto_batch_jobs(jobs, base_tctx)
    loop for each job
        B->>T: render_run_command_size(job, files, tctx)
        alt rendering succeeds
            T-->>B: rendered_len
        else rendering fails
            B->>B: estimate_files_string_size(files) fallback
        end
        alt rendered_len within safe_limit
            B-->>B: keep job unchanged
        else rendered_len exceeds safe_limit
            B->>B: binary search optimal batch_size
            B-->>B: emit sub-jobs preserving check_first and workspace_indicator
        end
    end
    B-->>E: batched jobs
    E->>E: assign semaphore to first job

_{Reviews (1): Last reviewed commit: "fix(step): only auto-batch when rendered..." | Re-trigger Greptile}

@plx

### 🚀 Features - **(builtins)** add `buildifier` format and lint built-ins by [@plx](https://github.com/plx) in [#896](#896) ### 🐛 Bug Fixes - **(step)** only auto-batch when rendered command exceeds ARG_MAX by [@jdx](https://github.com/jdx) in [#901](#901) ### 📚 Documentation - thank Namespace for GitHub Actions runner support by [@jdx](https://github.com/jdx) in [#895](#895) ### 🔍 Other Changes - **(ci)** use !cancelled() instead of always() for final job by [@jdx](https://github.com/jdx) in [#906](#906) - **(docs)** remove shrill.en.dev analytics script by [@jdx](https://github.com/jdx) in [#903](#903) - remove rust-cache from release jobs by [@jdx](https://github.com/jdx) in [#893](#893) - invert CLAUDE.md/AGENTS.md so AGENTS.md is canonical by [@jdx](https://github.com/jdx) in [#905](#905) - set dev profile debug to 1 by [@jdx](https://github.com/jdx) in [#907](#907) ### 📦️ Dependency Updates - update anthropics/claude-code-action digest to fefa07e by [@renovate[bot]](https://github.com/renovate[bot]) in [#897](#897) - update jdx/mise-action digest to 1648a78 by [@renovate[bot]](https://github.com/renovate[bot]) in [#898](#898) - update apple-actions/import-codesign-certs action to v7 by [@renovate[bot]](https://github.com/renovate[bot]) in [#900](#900) - update autofix-ci/action action to v1.3.4 by [@renovate[bot]](https://github.com/renovate[bot]) in [#899](#899) - lock file maintenance by [@renovate[bot]](https://github.com/renovate[bot]) in [#908](#908) ### New Contributors - @plx made their first contribution in [#896](#896)  --- > [!NOTE] > **Low Risk** > Low risk release bookkeeping: primarily version string updates across manifests and docs with no functional code changes in this diff. > > **Overview** > Updates the project for the `v1.45.0` release by bumping the crate/CLI version (`Cargo.toml`, `Cargo.lock`, `hk.usage.kdl`, generated CLI docs) and adding the `1.45.0` entry to `CHANGELOG.md`. > > Refreshes documentation and example configs to reference the new versioned Pkl package URLs (`docs/*.md`, `docs/public/*.pkl`, `hk-example.pkl`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cfe2da5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>  Co-authored-by: mise-en-dev <[email protected]>

gemini-code-assist Bot reviewed May 1, 2026

View reviewed changes

jdx merged commit b219cb3 into main May 1, 2026
24 checks passed

jdx deleted the claude/gracious-swanson-758b7e branch May 1, 2026 17:40

jdx mentioned this pull request May 1, 2026

chore: release v1.45.0 #894

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(step): only auto-batch when rendered command exceeds ARG_MAX#901

fix(step): only auto-batch when rendered command exceeds ARG_MAX#901
jdx merged 1 commit intomainfrom
claude/gracious-swanson-758b7e

jdx commented May 1, 2026 •

edited by cursor Bot

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 1, 2026

Uh oh!

greptile-apps Bot commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented May 1, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 1, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented May 1, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented May 1, 2026 •

edited by cursor Bot

Loading