Skip to content

Conversation

@MinZheCheah
Copy link

@MinZheCheah MinZheCheah commented Oct 1, 2025

Fixes #65
Fixed pdd sync to show stdout/stderr messages at the bottom. Previously the progress animation was hiding all command output and interactive prompts.

Implementation
Added output capture in sync_orchestration.py:
capture_output() context manager to intercept stdout/stderr
call_with_capture() helper to wrap operations

Wrapped major operations:
auto_deps_main, code_generator_main, context_generator_main, crash_main, cmd_test_main, fix_main
Display captured output after each operation completes

@gltanaka gltanaka requested a review from Copilot October 1, 2025 07:26
@gltanaka
Copy link
Contributor

gltanaka commented Oct 1, 2025

Do all the existing tests still work?

@gltanaka
Copy link
Contributor

gltanaka commented Oct 1, 2025

Should we have new tests?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the pdd sync command to display stdout/stderr output from operations that were previously hidden by progress animations. The change allows users to see command output and interactive prompts that were being suppressed.

  • Added output capture functionality with capture_output() context manager and call_with_capture() helper
  • Wrapped major operations (auto_deps, code_generator, context_generator, crash, cmd_test, fix) to capture their output
  • Display captured output after each operation completes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

sys.stdout = old_stdout
sys.stderr = old_stderr

def call_with_capture(func, quiet=False, *args, **kwargs):
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quiet parameter should be the first positional parameter after func to follow Python conventions for boolean flags in function signatures.

Copilot uses AI. Check for mistakes.
prompt_file=str(pdd_files['prompt']),
result, operation_output = call_with_capture(
auto_deps_main,
quiet,
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quiet parameter is being passed as a positional argument but the function signature expects it as a keyword argument. This will cause quiet to be passed as the first argument to auto_deps_main instead of controlling the capture behavior.

Suggested change
quiet,
quiet=quiet,

Copilot uses AI. Check for mistakes.
prompt_file=str(pdd_files['prompt']),
result, operation_output = call_with_capture(
code_generator_main,
quiet,
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as with auto_deps_main - quiet should be passed as a keyword argument: call_with_capture(code_generator_main, quiet=quiet, ctx, ...)

Suggested change
quiet,
quiet=quiet,

Copilot uses AI. Check for mistakes.
code_file=str(pdd_files['code']),
result, operation_output = call_with_capture(
context_generator_main,
quiet,
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as with previous operations - quiet should be passed as a keyword argument: call_with_capture(context_generator_main, quiet=quiet, ctx, ...)

Suggested change
quiet,
quiet=quiet,

Copilot uses AI. Check for mistakes.
program_file=str(pdd_files['example']),
result, operation_output = call_with_capture(
crash_main,
quiet,
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as with previous operations - quiet should be passed as a keyword argument: call_with_capture(crash_main, quiet=quiet, ctx, ...)

Suggested change
quiet,
quiet=quiet,

Copilot uses AI. Check for mistakes.
code_file=str(pdd_files['code']),
result, operation_output = call_with_capture(
cmd_test_main,
quiet,
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as with previous operations - quiet should be passed as a keyword argument: call_with_capture(cmd_test_main, quiet=quiet, ctx, ...)

Suggested change
quiet,
quiet=quiet,

Copilot uses AI. Check for mistakes.
unit_test_file=str(pdd_files['test']),
result, operation_output = call_with_capture(
fix_main,
quiet,
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as with previous operations - quiet should be passed as a keyword argument: call_with_capture(fix_main, quiet=quiet, ctx, ...)

Suggested change
quiet,
quiet=quiet,

Copilot uses AI. Check for mistakes.
@MinZheCheah
Copy link
Author

Do all the existing tests still work?

Yes tested on my branch

@MinZheCheah
Copy link
Author

Should we have new tests?

Only if an issue with pdd sync occurs

@gltanaka
Copy link
Contributor

Did you want to try to resolve the copilot issues?

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.

Right now pdd sync hides all the messages and interactive prompts. Modify this so that it shows the stdout/error messages at the bottom

2 participants