Unify the batch and incremental request pipelines

[christianhelle]

Summary

Collapses the two duplicate per-request orchestrations into one loop behind a reporter seam. The batch engine (CLI) and the incremental engine (GUI/TUI) previously reimplemented the same sequence — dependency checks, condition evaluation, variable/function substitution, pre/post delays, execution and assertions. Now there is a single orchestration with two adapters.

Design — reporter seam (two adapters)

• run_requests (processor/incremental_loop.rs) is the single orchestration. It calls a RequestReporter at each decision point and controls fail-fast via the method return value. It owns context tracking and delays, and returns the accumulated request contexts.
• CallbackReporter wraps the existing FnMut(idx, total, RequestProcessingResult) -> bool callback and reproduces today's events verbatim — GUI/TUI, the async path, and incremental tests are untouched.
• BatchReporter (processor/executor.rs) logs each outcome (reusing the output helpers) and aggregates pass/fail/skip counts for the CLI.

Each adapter preserves its own outcome semantics (e.g. the batch path counts a condition-evaluation error as a skip; the UI path emits Failed). The old process_single_request / process_single_file inline loop (~168 lines of duplicated orchestration) is deleted.

Behaviour preserved

• Data (heavily tested): pass/fail/skip counts, result_contexts, .success, assertion results.
• CLI logging: file header, per-request result lines, redaction, TLS warning, fail-fast detail, dependency-skip message, file/overall summaries.
• Capture flag verbose || fail_fast is threaded so the failing request always has body/headers on fail-fast.

Verification

• cargo test --workspace ✅ (incl. 136 processor tests)
• cargo clippy --workspace --all-targets -- -D warnings ✅
• End-to-end CLI smoke tests against a local server: normal, verbose (request → result → response), fail-fast (stops + suppresses summary), dependency-skip (exact message) — all match prior behaviour.

Net -63 lines; GUI/TUI/CLI source untouched.

Known minor reduction

CLI verbose mode no longer prints the per-condition evaluation detail (previously output::log_condition_evaluation_verbose). The capability remains available as public API in conditions::evaluate_conditions_verbose; it is simply no longer called by the batch path, because the unified loop keeps presentation out of the orchestration. Easy to restore later by threading contexts into the reporter if desired.

Summary by CodeRabbit

• New Features

  • Improved request processing with more consistent progress reporting during batch runs.
  • Added clearer messages for skipped requests and condition-related errors.

• Bug Fixes

  • Fixed batch execution so fail-fast now stops processing sooner and more reliably.
  • Improved handling of request substitution and execution errors for clearer output.
  • Corrected request count tracking so summaries reflect actual successes, failures, and skips.

[coderabbitai]

Warning

Review limit reached

@christianhelle, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 15ece27d-0239-42b1-a254-be5e24c21f81

📥 Commits

Reviewing files that changed from the base of the PR and between 6906e10 and a9f7fee.

📒 Files selected for processing (2)

• src/core/src/processor/executor.rs
• src/core/src/processor/executor_tests.rs

📝 Walkthrough

Walkthrough

Introduces a RequestReporter trait in incremental_loop.rs and extracts run_requests from process_requests_incremental. A CallbackReporter adapts the existing callback API. In executor.rs, process_single_file is rewired to use run_requests with a new BatchReporter that centralizes logging, counters, and fail-fast halting. output.rs drops the total counter and the verbose condition evaluator, replacing it with two simpler logging helpers.

Changes

RequestReporter abstraction and batch execution rewiring

Layer / File(s)	Summary
RequestReporter trait and CallbackReporter adapter src/core/src/processor/incremental_loop.rs	Defines the RequestReporter trait covering all loop decision points (dependency/condition/substitution/execution). Extracts run_requests from the old loop body; process_requests_incremental now wraps its callback via CallbackReporter and delegates to run_requests. Substitution error messages are unified to "Substitution error: {}".
output.rs counter and condition logger cleanup src/core/src/processor/output.rs	Removes total field and increment_total from RequestCounters. Deletes log_condition_evaluation_verbose and replaces it with log_conditions_not_met and log_condition_error.
BatchReporter adapter and process_single_file rewiring src/core/src/processor/executor.rs	Adds BatchReporter implementing RequestReporter with per-request logging, skip/success/failure counting, and fail-fast halting via an halted flag. Replaces the old sequential loop in process_single_file with block_on(run_requests(...)), extracting counters and halted from the reporter afterward.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• christianhelle/httprunner#99: Modifies executor.rs around process_http_files/request execution, introducing the executor-injection API that this PR builds on top of.
• christianhelle/httprunner#277: Introduces processor/output.rs including RequestCounters.total and log_condition_evaluation_verbose, both of which this PR removes or replaces.
• christianhelle/httprunner#279: Foundational refactor of incremental_loop.rs that introduced the unified loop and RequestProcessingResult types that the new RequestReporter trait builds upon.

Suggested labels

Core

🐇 A reporter was born with callbacks galore,
Batch and incremental now share the same core.
The total field vanished, the verbose log gone,
run_requests returns contexts and carries them on.
Fail-fast halts cleanly with a false in the night —
One trait to rule loops, and the rabbit says: right! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main refactor: unifying the batch and incremental request pipelines.
Description check	✅ Passed	The description covers summary, design, preserved behavior, and testing/verification, with only optional template fields like issues and checklist omitted.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch prd/2-unify-pipelines

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 72.86245% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.32%. Comparing base (1236577) to head (a9f7fee).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/core/src/processor/executor.rs	73.61%	38 Missing ⚠️
src/core/src/processor/incremental_loop.rs	72.00%	35 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #282      +/-   ##
==========================================
- Coverage   92.81%   92.32%   -0.50%     
==========================================
  Files          68       68              
  Lines        8602     8544      -58     
==========================================
- Hits         7984     7888      -96     
- Misses        618      656      +38     

☔ View full report in Codecov by Harness.
📢 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.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/src/processor/executor.rs`:
- Around line 155-164: The substitution failure log in `substitution_error`
ignores the `_request` argument, so failed batch entries only show a generic
internal error. Update this method to include request context from `HttpRequest`
in the log message, such as the request name, method, and URL, so users can
identify which request failed when `record_failure()` is called.
- Around line 117-151: Skipped-request logging in executor should avoid emitting
raw URLs when secrets are not included. Update `dependency_skipped`,
`conditions_skipped`, and `condition_error` in `executor.rs` to sanitize the
request before passing it to logging, using the same secret-redaction behavior
used elsewhere in the request output path. Keep the existing skip/error counters
and messages, but ensure `output::log_conditions_not_met` and
`output::log_condition_error` receive a sanitized `HttpRequest` or equivalent
redacted URL representation when `include_secrets` is false.
- Around line 143-152: Treat condition evaluation errors as failures when
running in batch mode. Update CallbackReporter::condition_error in executor.rs
so it no longer unconditionally records a skip and returns true; instead, detect
batch mode and mark the request as failed (and honor fail_fast by stopping
further processing as needed) while keeping the existing logging via
output::log_condition_error. Make sure the callback’s failure path aligns with
the Failed status emitted by CallbackReporter and does not inflate success
totals for condition errors.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 22ff87fe-7a38-4897-a409-4c2b005141b4

📥 Commits

Reviewing files that changed from the base of the PR and between bb6671c and 6906e10.

📒 Files selected for processing (3)

• src/core/src/processor/executor.rs
• src/core/src/processor/incremental_loop.rs
• src/core/src/processor/output.rs