Skip to content

feat: show elapsed time per step in runner output#17

Open
Kanneboinashivakumar wants to merge 2 commits into
faisalishfaq2005:mainfrom
Kanneboinashivakumar:feat/show-step-duration
Open

feat: show elapsed time per step in runner output#17
Kanneboinashivakumar wants to merge 2 commits into
faisalishfaq2005:mainfrom
Kanneboinashivakumar:feat/show-step-duration

Conversation

@Kanneboinashivakumar

Copy link
Copy Markdown

Closes #6

What problem does this solve?

Currently, the runner output only shows the cost of each step:

✓ done · $0.31

However, ClaudeResult already contains a durationMs field with the elapsed time. Users want visibility into how long each step takes, not just the cost.

Changes Made

This PR adds duration display to the runner output by:

  1. Ensuring durationMs is carried through StepRun (src/core/runner.ts)

    • Allows duration data to flow from Claude results to step reporting
  2. Propagating durationMs from ClaudeResult

    • Included in stepRun as durationMs: result.durationMs
  3. Updating onStepFinish reporter (src/commands/run.ts)

    • Formats milliseconds into seconds (e.g. 8.2s)
    • Displays duration alongside cost in CLI output
    • Gracefully handles missing duration values

📊 Before & After

Before:
✓ done · $0.31
✗ ERROR · $0.00

After:
✓ done · $0.31 · 8.2s
✗ ERROR · $0.00 · 0.2s

Testing

  • npm run build passes
  • Tested using:
    node dist/cli.js run test-and-fix --dry-run
  • CLI output correctly shows duration when available
  • No breaking changes introduced

Checklist

  • My code follows project style
  • npm run build passes
  • npm run typecheck passes
  • Tested locally
  • No breaking changes

@Kanneboinashivakumar

Copy link
Copy Markdown
Author

Hi @faisalishfaq2005, could you please take a look at this PR and approve the pending workflow when you get a chance so the checks can run? I've tested everything locally and it should be ready for review. Thank you!

@faisalishfaq2005

Copy link
Copy Markdown
Owner

Thanks for this , the core change in src/commands/run.ts and src/core/runner.ts is exactly right and does what the issue asked for; I checked it out and typecheck/build/tests all pass. Before merging though, could you trim the diff down to just those two files? The commit also pulls in
.loopflow/loops/*.yaml, .loopflow/memory/test-and-fix.md, and package-lock.json, which look like local artifacts from running loopflow init/loopflow run test-and-fix to test the feature (the memory file even records a failed run) rather than anything meant for this repo. While you're at it, the two new lines in runner.ts have trailing whitespace, and the new line in run.ts uses single quotes where the rest of the file uses double . Once it's just the feature change, this is good to go. Thanks for the contribution!

@Kanneboinashivakumar

Copy link
Copy Markdown
Author

Hi @faisalishfaq2005, thanks for the review!

I've addressed all the requested changes:

Removed the local .loopflow artifacts and reverted package-lock.json.
Fixed the trailing whitespace in src/core/runner.ts.
Updated the new code in src/commands/run.ts to use double quotes, matching the project's style.
Verified that the PR now contains only the intended feature changes (src/core/runner.ts and src/commands/run.ts).

Validation completed successfully:
npm run typecheck
npm run build
npm run test (33/33 tests passing)

Please take another look when you have a chance. Thanks again!

@Kanneboinashivakumar

Copy link
Copy Markdown
Author

Hi @faisalishfaq2005, just a quick follow-up on this PR whenever you get a chance.

I've addressed all the requested changes (artifact cleanup, style fixes, and tests passing), and the PR should now be aligned with the review feedback. It looks like the only remaining item is the workflow approval.

Whenever you're available to take another look or merge, I'd really appreciate it. Thanks again for your time reviewing this!

@Kanneboinashivakumar

Copy link
Copy Markdown
Author

Hi @faisalishfaq2005

Just a friendly follow-up on this PR whenever you have some time.

As requested, I removed the local artifacts, applied the style fixes, and verified that the PR now contains only the intended changes to src/core/runner.ts and src/commands/run.ts. Everything continues to pass locally.

Whenever you're available, I'd appreciate another look. Thanks again for your time and feedback!

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.

[feature]: Show elapsed time per step in the runner output

2 participants