Sync progress flushes by tony · Pull Request #492 · vcs-python/libvcs

tony · 2025-06-20T11:46:12Z

Summary by Sourcery

Document TTY limitation for git progress output and introduce tests for the run module

Enhancements:

Add comment explaining why git progress outputs newlines instead of carriage returns when stderr is not a TTY

Tests:

Introduce tests for the internal run module to validate process polling and progress callbacks

sourcery-ai · 2025-06-20T11:46:17Z

Reviewer's Guide

This PR inserts explanatory comments in run.py detailing why Git’s progress output uses new lines when stderr isn’t a TTY (and why switching to a pseudo-TTY would require deeper subprocess changes), and it scaffolds a new test file stub for run.py.

File-Level Changes

Change	Details	Files
Clarify Git progress output behavior when not on a TTY	Added notes on stderr TTY detection and its effect on progress formatting Described that without a pseudo-TTY, Git emits lines instead of carriage returns Explained that supporting proper in-place updates would need substantial subprocess handling changes	`src/libvcs/_internal/run.py`
Add empty test module for run.py	Created new test file stub to support future run module tests	`tests/_internal/test_run.py`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @tony - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2025-06-20T11:46:47Z

tests/_internal/test_run.py

+        )
+
+        # Verify we got multiple chunks
+        assert len(captured_chunks) > 0, "Should receive progress chunks"


issue (code-quality): Simplify sequence length comparison (simplify-len-comparison)

sourcery-ai · 2025-06-20T11:46:47Z

tests/_internal/test_run.py

+        )
+
+        # Check that output is fragmented
+        assert len(captured_chunks) > 0, "Should capture output"


suggestion (code-quality): Simplify sequence length comparison (simplify-len-comparison)

Suggested change

assert len(captured_chunks) > 0, "Should capture output"

assert captured_chunks, "Should capture output"

sourcery-ai · 2025-06-20T11:46:47Z

tests/_internal/test_run.py

+
+        # In current implementation, all stderr is read in one chunk
+        # This demonstrates the buffering issue
+        assert len(captured_chunks) >= 1, "Should get at least 1 chunk"


suggestion (code-quality): Simplify sequence length comparison (simplify-len-comparison)

Suggested change

assert len(captured_chunks) >= 1, "Should get at least 1 chunk"

assert captured_chunks, "Should get at least 1 chunk"

sourcery-ai · 2025-06-20T11:46:47Z

tests/_internal/test_run.py

+        # Desired behavior: each complete line is one chunk
+
+        # For now, just verify we got output
+        assert len(captured_chunks) > 0, "Should capture output"


suggestion (code-quality): Simplify sequence length comparison (simplify-len-comparison)

Suggested change

assert len(captured_chunks) > 0, "Should capture output"

assert captured_chunks, "Should capture output"

sourcery-ai · 2025-06-20T11:46:48Z

tests/_internal/test_run.py

+            "Output should be fragmented into multiple chunks"
+        )
+
+        assert len(captured_chunks) > 0


issue (code-quality): Simplify sequence length comparison (simplify-len-comparison)

…ate broken state)

… behavior - Add TestFlushingBehavior class with tests for: - Timing verification of immediate flushing - Unbuffered subprocess output handling - Carriage return overwrites for progress bars - ANSI escape sequence preservation - Git-style progress simulation - Mixed line endings (\n, \r, \r\n) - Large output streaming without blocking - stderr vs stdout stream separation - Add TestRealWorldScenarios class covering: - npm-style progress with Unicode spinners - Long-running processes with periodic updates - Multi-line progress bars (Docker-style) - Binary/Unicode output handling - Interleaved stdout/stderr timing - Add TestEdgeCases class for: - Empty output handling - stdout-only processes - Very long lines exceeding buffer size - Rapid output bursts - Callback exception handling - Enhance existing tests with better type annotations - Fix linting issues (imports, line length, docstrings) - Follow libvcs testing conventions and patterns These tests verify the current behavior of the run() function's real-time output streaming via callbacks, including the 128-byte chunking behavior and proper handling of control characters. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…e progress updates why: Commit 80b480e switched to text=True which interfered with carriage return handling, causing git progress output to appear on separate lines instead of overwriting the same line. what: - Revert subprocess to text=False (bytes mode) - Restore console_to_str() function for proper bytes-to-string conversion 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

… of broken fragmentation why: Previous tests were verifying the broken 128-byte fragmentation behavior. With the fix, we need tests that verify proper in-place progress streaming. what: - Update existing tests to verify correct behavior instead of broken fragmentation - Add comprehensive TestStreamingFix class with tests for: * Line-by-line streaming without fragmentation * Real-time streaming with timing verification * Proper handling of long lines without breaking them 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

why: Fix mypy type errors and improve type safety in subprocess handling. what: - Add type annotations for lines and stderr_lines variables - Add null checks for proc.stdout and proc.stderr before accessing readlines() - Update ProgressCallbackProtocol to use str instead of t.AnyStr for consistency 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…ations why: Comply with 88-character line length limit for linting requirements. what: - Split long assert statement across multiple lines in line 438 - Break long string literal across multiple lines in line 965 - Update CallbackOutput type alias from t.AnyStr to str for consistency 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

sourcery-ai bot reviewed Jun 20, 2025

View reviewed changes

tony force-pushed the sync-progress-flushes branch from b505738 to b0b3cab Compare June 20, 2025 11:54

tony and others added 3 commits June 20, 2025 07:06

tests(run) Add tests for newline behavior + streaming behavior (recre…

b940bd1

…ate broken state)

chore(run) Note subprocess issue with streaming

70b259f

tony force-pushed the sync-progress-flushes branch from b0b3cab to 4f85cd7 Compare June 20, 2025 12:06

tony and others added 4 commits June 20, 2025 08:32

tony force-pushed the master branch from a0d8e27 to 452ebc9 Compare December 25, 2025 21:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync progress flushes#492

Sync progress flushes#492
tony wants to merge 7 commits intomasterfrom
sync-progress-flushes

tony commented Jun 20, 2025 •

edited by sourcery-ai bot

Loading

Uh oh!

sourcery-ai bot commented Jun 20, 2025 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Uh oh!

sourcery-ai bot Jun 20, 2025

Uh oh!

sourcery-ai bot Jun 20, 2025

Uh oh!

sourcery-ai bot Jun 20, 2025

Uh oh!

sourcery-ai bot Jun 20, 2025

Uh oh!

sourcery-ai bot Jun 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	assert len(captured_chunks) > 0, "Should capture output"
	assert captured_chunks, "Should capture output"

	assert len(captured_chunks) >= 1, "Should get at least 1 chunk"
	assert captured_chunks, "Should get at least 1 chunk"

Conversation

tony commented Jun 20, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tony commented Jun 20, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jun 20, 2025 •

edited

Loading