Add timing metrics to response regeneration pipeline

[orestis-z]

Summary

• Instruments the response regeneration script with performance timing using time.perf_counter()
• Adds live throughput to tqdm postfix (rps = vLLM requests/s, tps = generated tokens/s) alongside existing ok/err/trunc counters
• Tracks per-request latency, queue wait time, and token counts; logs a final summary with aggregate stats
• Uses plain logging + tqdm (not speculators.metrics), per the approach agreed in #717

Companion to the offline data generation timing PR (#804). Both address INFERENG-8704.

Test plan

• Existing unit tests pass (tests/unit/scripts/test_response_regeneration.py — 43 tests)
• Updated _NullProgress stub and stats dict in tests to match new signature
• ruff check and ruff format pass
• Manual run against a vLLM endpoint to verify tqdm postfix and final summary output

🤖 Generated with Claude Code

[coderabbitai]

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: 1d981bc5-3cb4-4ab6-8d3c-7f60fbf1c504

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

Changes

The response regeneration pipeline now tracks request latency, queue wait, token usage, throughput, and conversation outcomes, displays live RPS/TPS progress metrics, and logs an aggregate summary after completion. Tests were updated for the expanded statistics structure.

Response regeneration metrics

Layer / File(s)	Summary
Metrics state and queue timestamps scripts/response_regeneration/script.py	Shared statistics now include timing, request, token, and queue-wait fields; queued work records its enqueue timestamp.
Worker instrumentation and progress metrics scripts/response_regeneration/script.py, tests/unit/scripts/test_response_regeneration.py	Workers accumulate request latency, queue wait, and token usage, while progress output reports calculated RPS and TPS; tests cover the expanded stats structure.
Aggregated summary logging scripts/response_regeneration/script.py	Completion logging reports elapsed time, conversation outcomes, throughput, and average queue wait after workers finish.

Possibly related PRs

• vllm-project/speculators#729: Directly overlaps the response regeneration worker’s latency and usage statistics logic.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the main change: adding timing metrics to the response regeneration pipeline.
Description check	✅ Passed	The description is directly related to the changes and accurately describes the timing instrumentation work.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

tests/unit/scripts/test_response_regeneration.py (1)

477-502: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Include enqueued_at in the test item to exercise queue wait logic.

The worker function now pops enqueued_at from the queue item to calculate total_queue_wait_s. Since the mock _TWO_TURN_ITEM does not include this key, the wait-time calculation path is not exercised during tests. As per path instructions, check that new code paths introduced in the PR are covered. Consider injecting a timestamp before enqueuing.

🧪 Proposed fix to cover queue wait logic

     async def scenario(out_fh, err_fh):
         queue: asyncio.Queue = asyncio.Queue()
-        await queue.put(_TWO_TURN_ITEM)
+        item = dict(_TWO_TURN_ITEM)
+        item["enqueued_at"] = time.perf_counter()
+        await queue.put(item)
         await queue.put(None)

🤖 Prompt for 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.

In `@tests/unit/scripts/test_response_regeneration.py` around lines 477 - 502,
Update the test scenario using _TWO_TURN_ITEM to enqueue an item containing an
enqueued_at timestamp, preserving the existing item fields and setting the
timestamp before queue.put. Ensure worker exercises its queue-wait calculation
and validates the resulting total_queue_wait_s path.

Source: Path instructions

🤖 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 `@scripts/response_regeneration/script.py`:
- Around line 786-796: Update the stats parameter annotation in the worker
function to accept the float-valued fields initialized in the stats dictionary,
using dict[str, Any] or dict[str, int | float] while preserving the existing
worker behavior.

In `@tests/unit/scripts/test_response_regeneration.py`:
- Around line 521-523: Extend the assertions for the worker statistics in the
response regeneration test to cover the newly introduced metric fields,
including requests and total_request_s, using the expected values for this
scenario. Keep the existing ok, errors, and truncated assertions unchanged.

---

Outside diff comments:
In `@tests/unit/scripts/test_response_regeneration.py`:
- Around line 477-502: Update the test scenario using _TWO_TURN_ITEM to enqueue
an item containing an enqueued_at timestamp, preserving the existing item fields
and setting the timestamp before queue.put. Ensure worker exercises its
queue-wait calculation and validates the resulting total_queue_wait_s path.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d53e770d-51df-47c4-8011-c99034f29ba2

📥 Commits

Reviewing files that changed from the base of the PR and between c2d7a6c and be5588f.

📒 Files selected for processing (2)

• scripts/response_regeneration/script.py
• tests/unit/scripts/test_response_regeneration.py

[speculatorsbot]

LGTM. Clean, well-scoped instrumentation that matches the RFC #717 consensus (plain logging + tqdm). Agree with @WindChimeRan's three outstanding items (logging config, queue-wait denominator, dead prompt_tokens). Recommend approving once those are addressed.

🤖 Generated with Claude Code using the /pr-review skill