-
Notifications
You must be signed in to change notification settings - Fork 90
add step navigation buttons in xray. #300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
Category | Issue | Status |
---|---|---|
Global Variable Usage Reduces Readability ▹ view |
Files scanned
File Path | Reviewed |
---|---|
src/agentlab/analyze/agent_xray.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
src/agentlab/analyze/agent_xray.py
Outdated
def format_step_indicator(step_id): | ||
global info | ||
if not step_id or not info.exp_result or not info.exp_result.steps_info: | ||
return "Step 0/0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Global Variable Usage Reduces Readability 
Tell me more
What is the issue?
The function uses a global variable which makes the code harder to understand and maintain.
Why this matters
Global state makes code flow harder to follow and increases the likelihood of bugs when the global state changes unexpectedly.
Suggested change ∙ Feature Preview
def format_step_indicator(step_id: StepId, info: Info) -> str:
if not step_id or not info.exp_result or not info.exp_result.steps_info:
return "Step 0/0"
Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you validate that the shortcut shit -> and shift <- still works for next/prev. Or is it cmd -> cmd <-. If they still work, can you add indication around these new buttons that this is the shortcut.
8b08c1f
to
54a729c
Compare
Adds step navigation buttons and fix keyboard shortcuts.
Description by Korbit AI
What change is being made?
Add Prev/Next step navigation buttons and a dynamic step indicator to the XRay Gradio UI, wire their events to keyboard-like navigation, expose the Gradio server to public when configured, and adjust the episode info header to remove the explicit Step line.
Why are these changes being made?
Enhance usability by enabling quick step navigation and a live step indicator; allow public access when configured without altering core behavior. Minor UI header tweak aligns with the updated navigation display.