Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds five new VHS terminal-recording scripts that automate bash CLI sessions to produce GIF demos by invoking and verifying Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vhs/02_short.tape`:
- Line 11: Several `Type` arguments open a quoted string but lack a closing
quote (e.g., the literal starting with Type "export
KORTEX_CLI_DEFAULT_RUNTIME=podman); fix each occurrence by terminating the
quoted string properly so the export command is fully enclosed in quotes.
Specifically, locate the `Type "export ...` lines (the instances that set
KORTEX_CLI_DEFAULT_RUNTIME and the other export commands) and add the missing
closing double-quote at the end of each export command so VHS can parse the
input correctly.
In `@vhs/03_model.tape`:
- Line 11: The Type commands for the export of KORTEX_CLI_DEFAULT_RUNTIME are
missing closing quote marks; locate the Type statements that begin with Type
"export KORTEX_CLI_DEFAULT_RUNTIME=podman (also the corresponding Type lines at
the other two occurrences) and add the missing trailing double-quote to make
them Type "export KORTEX_CLI_DEFAULT_RUNTIME=podman"; repeat the same fix for
the matching lines in vhs/02_short.tape to ensure all Type "<string>" literals
are properly closed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6da2ca5-0170-41c6-a350-4b45e89cfc11
⛔ Files ignored due to path filters (3)
demos/01_detailed.gifis excluded by!**/*.gifdemos/02_short.gifis excluded by!**/*.gifdemos/03_model.gifis excluded by!**/*.gif
📒 Files selected for processing (3)
vhs/01_detailed.tapevhs/02_short.tapevhs/03_model.tape
serbangeorge-m
approved these changes
Apr 7, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vhs/05_config_projects.tape`:
- Line 18: Backup/restore moves for ~/.kortex-cli/config/projects.json are
unguarded and fail when files are missing; update the script to check for file
existence before moving or restoring (e.g., test -f or [ -f ] checks) so the
initial mv to projects.json.orig only runs if projects.json exists and the
restore mv only runs if projects.json.orig exists, and handle missing-source
cases gracefully (skip the move or log a notice) to avoid demo init/cleanup
failures.
- Line 18: The JSON payload in the mv && echo -n ... | json_pp pipeline uses
single quotes so the mount path "$HOME/.gitconfig" is not expanded and ends up
literal in projects.json; change the write to expand the variable (e.g., switch
to double quotes with proper JSON escaping, or construct the JSON via
printf/envsubst/jq so $HOME is interpolated) so that the mounts host and target
contain the actual expanded home path instead of the literal "$HOME/.gitconfig".
- Around line 33-34: The Type command string is missing a closing quote which
breaks the tape parser; update the Type instruction (the line starting with Type
"kortex-cli init) to include the trailing double-quote so it becomes Type
"kortex-cli init" ensuring the quoted argument is balanced for the tape parser.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f188c607-e9b3-49f3-81c5-714896b67f52
⛔ Files ignored due to path filters (2)
demos/04_config_local.gifis excluded by!**/*.gifdemos/05_config_projects.gifis excluded by!**/*.gif
📒 Files selected for processing (2)
vhs/04_config_local.tapevhs/05_config_projects.tape
✅ Files skipped from review due to trivial changes (1)
- vhs/04_config_local.tape
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
vhs/01_detailed.tape (1)
35-42: Harden terminal transition sync to reduce recording flakiness.This block uses fixed sleeps only. Consider adding
Wait+Screenchecks before sending/exitand before continuing, so recordings stay deterministic across different machine speeds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vhs/01_detailed.tape` around lines 35 - 42, The recording uses fixed Sleep 5000ms delays which causes flakiness; replace those sleeps with deterministic wait-for-screen checks: after the "Type \"kdn terminal demo\" / Enter" step, add a WaitForScreen or WaitUntilVisible/Wait+Screen assertion that verifies the demo prompt or expected output appears before continuing, and before sending "Type \"/exit\" / Enter add a WaitForScreen check that the terminal is ready for input; likewise replace the final Sleep 5000ms with a WaitForScreen or confirmation that the exit completed (shell prompt or goodbye message) so transitions in the sequence (the steps around the strings \"kdn terminal demo\" and \"/exit\") are synchronized to actual UI state rather than fixed time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@vhs/01_detailed.tape`:
- Around line 35-42: The recording uses fixed Sleep 5000ms delays which causes
flakiness; replace those sleeps with deterministic wait-for-screen checks: after
the "Type \"kdn terminal demo\" / Enter" step, add a WaitForScreen or
WaitUntilVisible/Wait+Screen assertion that verifies the demo prompt or expected
output appears before continuing, and before sending "Type \"/exit\" / Enter add
a WaitForScreen check that the terminal is ready for input; likewise replace the
final Sleep 5000ms with a WaitForScreen or confirmation that the exit completed
(shell prompt or goodbye message) so transitions in the sequence (the steps
around the strings \"kdn terminal demo\" and \"/exit\") are synchronized to
actual UI state rather than fixed time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95479ac1-13da-4bfc-9744-676946ea69ce
⛔ Files ignored due to path filters (5)
demos/01_detailed.gifis excluded by!**/*.gifdemos/02_short.gifis excluded by!**/*.gifdemos/03_model.gifis excluded by!**/*.gifdemos/04_config_local.gifis excluded by!**/*.gifdemos/05_config_projects.gifis excluded by!**/*.gif
📒 Files selected for processing (5)
vhs/01_detailed.tapevhs/02_short.tapevhs/03_model.tapevhs/04_config_local.tapevhs/05_config_projects.tape
✅ Files skipped from review due to trivial changes (2)
- vhs/02_short.tape
- vhs/03_model.tape
🚧 Files skipped from review as they are similar to previous changes (2)
- vhs/04_config_local.tape
- vhs/05_config_projects.tape
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Demos:
workspace.jsonfile in the repositoryprojects.jsonfile in user's home directoryfixes #196