Skip to content

Support asciinema v1 format playback#281

Merged
ibigbug merged 3 commits intomainfrom
copilot/fix-powersession-file-playback
Mar 10, 2026
Merged

Support asciinema v1 format playback#281
ibigbug merged 3 commits intomainfrom
copilot/fix-powersession-file-playback

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 10, 2026 •
edited
Loading

  • Understand the issue: v1 asciinema format files cause a panic instead of a friendly error
  • Add V1Recording struct to types.rs for deserializing v1 format
  • Add SessionLineSource enum in play.rs to support both v2 (file-based) and v1 (vec-based) iterators
  • Modify Session::new to detect v1 format and handle it properly
  • Add v1 test data file
  • Add tests for v1 format playback
  • Fix CI failure: testdata/play_v1.json had literal newlines inside JSON strings (invalid JSON)
  • Replace all unwrap()/panic! with eprintln! + exit(1) for clean error reporting
  • Error messages now use eprintln! (stderr) with filename and underlying error details
  • Separate Ok(version≠1) and Err(parse_failure) arms for precise diagnostics
Original prompt

This section details on the original issue you should resolve

<issue_title>PowerSession can't play this file</issue_title>
<issue_description>Hi. PowerSession can't play this file.

https://asciinema.org/a/8muthjs1m0bcbo6f1c4tp8kt1

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
Error("EOF while parsing an object", line: 1, column: 1)', ...\PowerSession-0.1.6\src\commands\play.rs:95:88
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@ibigbug</author><body>
this looks like a v1 format, which isn't supported.

<img width="629" alt="image" src="https://user-images.githubusercontent.com/543405/187036716-aa23c725-03f4-403f-9ccb-a88f18d2df04.png">
</body></comment_new>
<comment_new><author>@ibigbug</author><body>
good point.</body></comment_new>
</comments>

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@ibigbug
Add asciinema v1 format support for playback
fe1a77e 
Co-authored-by: ibigbug <543405+ibigbug@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix PowerSession playback issue for unsupported file format Support asciinema v1 format playback Mar 10, 2026
@ibigbug ibigbug requested a review from Copilot March 10, 2026 02:33
Copilot AI reviewed Mar 10, 2026
View reviewed changes
Copy link
Copy Markdown

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

Adds support for playing asciinema v1 recordings by detecting the v1 single-JSON-object format and translating its relative delays into the existing playback event pipeline.

Changes:

  • Introduces a V1Recording type to deserialize v1 recordings (single JSON blob with relative stdout delays).
  • Updates playback to detect v2 first, then fall back to v1 by buffering the already-read content and converting to absolute timestamps.
  • Adds a v1 fixture and tests to cover v1 playback (including speed and idle-time-limit variants).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
testdata/play_v1.json Adds a v1-format recording fixture for tests.
src/commands/types.rs Adds V1Recording for v1 deserialization.
src/commands/play.rs Adds v1/v2 detection and a unified line source to run both through the same playback pipeline; adds tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread testdata/play_v1.json Outdated
Comment thread src/commands/play.rs Outdated
Comment thread src/commands/play.rs
Comment on lines +115 to +118
for line in line_iter {
file_content.push('\n');
file_content.push_str(&line.unwrap());
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The v1/v2 fallback path still has multiple unwrap() calls that can panic (file open error, empty file, I/O error while reading remaining lines), which contradicts the PR goal of exiting cleanly on unsupported/corrupt input. Handle these errors explicitly (e.g., print a clear error to stderr and exit(1), or refactor Session::new to return a Result<Session, _> and let the caller handle reporting).

Copilot uses AI. Check for mistakes.
Comment thread src/commands/play.rs Outdated
Comment thread src/commands/play.rs Outdated
@ibigbug
Copy link
Copy Markdown
Member

ibigbug commented Mar 10, 2026

@copilot ci failed. Fix it? Also check pr comments.

@ibigbug
Fix CI failure and address PR review comments
947907c 
- Fix testdata/play_v1.json: invalid JSON due to literal newlines in strings
- Replace unwrap/panic with eprintln+exit(1) throughout play.rs
- Improve error messages to use stderr with filename and error details

Co-authored-by: ibigbug <543405+ibigbug@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 10, 2026

@copilot ci failed. Fix it? Also check pr comments.

Fixed in 947907c. The CI failure was caused by testdata/play_v1.json containing literal CR+LF characters inside JSON strings (invalid JSON), which caused serde_json::from_str to fail and hit our exit(1) path during the test run.

Also addressed all the PR review comments in the same commit:

  • Fixed the invalid JSON test fixture (proper \r\n escape sequences)
  • Replaced all unwrap()/panic! calls with eprintln! + exit(1) throughout play.rs (in Session::new, the v1 fallback loop, and StdoutIter::next)
  • Error messages now go to stderr via eprintln! and include the filename plus the underlying error (or version number for unsupported format versions)

@ibigbug ibigbug marked this pull request as ready for review March 10, 2026 04:11
@ibigbug ibigbug merged commit 46e7279 into main Mar 10, 2026
2 checks passed
@ibigbug ibigbug deleted the copilot/fix-powersession-file-playback branch March 10, 2026 04:19
Copilot AI mentioned this pull request Mar 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@ibigbug ibigbug

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PowerSession can't play this file

3 participants

@ibigbug