Skip to content

Implement progress reporting for in-flight commands - #115

Open
leandropineda wants to merge 2 commits into
mainfrom
feat/command-progress-reporting
Open

Implement progress reporting for in-flight commands#115
leandropineda wants to merge 2 commits into
mainfrom
feat/command-progress-reporting

Conversation

@leandropineda

@leandropineda leandropineda commented Sep 2, 2026

Copy link
Copy Markdown
Member

Why

progress_function has been a stub since it was introduced:

# TODO: Implement progress reporting function
def progress_function(output, error):
    return 1

So a handler had no way to say a command was still running. A command that takes a while is silent until it finishes, leaving no way to tell slow progress apart from a stalled command.

What

  • CUSTOM_COMMAND_STATUS_RUNNING = "running"
  • report_command_progress(), mirroring report_command_result but with no return code — the command has not finished, so there is no outcome yet
  • progress_function publishes that, and no-ops when there is no execution_id to correlate against

No proto change: execution_status is already a free string, and stdout/stderr are already fields.

Opt-in

Progress is emitted only when a handler calls progress_function(), never automatically on dispatch. Some consumers act on the first status update they see, so a handler that does not opt in keeps publishing a single final status, exactly as before.

progress_function(output, error) keeps its signature, with both arguments now optional, so existing callers are unaffected.

Demo

The demo only had commands that answer instantly, so there was no way to try this. It now handles two custom commands, both taking an optional seconds argument (default 20):

Filename Outcome
slow_success reports progress, then succeeds
slow_failure reports progress, then fails with details
slow_success seconds 30

Each reports once before starting, then every 5 seconds, then the real result — including the failure path with details, which the demo did not exercise before. They run on a thread rather than blocking the callback, since a handler that sleeps in place holds up everything else on that session.

Screencast.2026-09-03.17.33.50.mp4

Tests

123 pass (119 existing + 4 new): progress publishes running with no return code; a handler calling both emits running then finished under one execution id; a handler that does not opt in emits only the final status; no execution_id publishes nothing.

test_metrics.py and test_video.py already fail to collect on main for missing optional deps, unrelated to this change.

progress_function was a stub returning 1, so a handler had no way to say a
command was still running. A command that takes a while was therefore
silent until it finished, leaving no way to tell slow progress apart from a
stalled command.

It now publishes a status update carrying "running", alongside the existing
final result. No proto change: execution_status is already a free string.

Reporting is opt-in, emitted only when a handler calls progress_function
rather than automatically on dispatch. Some consumers act on the first
status update they see, so a handler that does not opt in keeps reporting a
single final status, exactly as before.

progress_function keeps its signature, with both arguments now optional.
@leandropineda
leandropineda force-pushed the feat/command-progress-reporting branch from ab3078a to 5effea6 Compare September 2, 2026 20:08
The demo only had commands that answer instantly, so there was no way to
try progress reporting or to see how a slow command behaves.

Adds slow_success and slow_failure, both taking an optional seconds
argument. Each reports once before starting, then periodically, then the
real result - including the failure path with details, which the demo did
not exercise at all before.

They run on a thread rather than blocking the callback, since a handler
that sleeps in place holds up everything else on that session.
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