Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds CLI flags to start an example agent subprocess with lifecycle management, updates docs/install instructions and packaging to include the example, restores TLS verification on external requests, adjusts TUI help/report layout and sizing, normalizes evaluator scenario inputs with fallbacks, updates example imports, and bumps version to 0.1.6. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CLI as rogue (__main__)
participant EA as start_example_agent()
participant P as Example Agent (subprocess)
participant M as Mode Runner (ui/server/cli/tui)
U->>CLI: rogue --example tshirt_store [--example-host H --example-port R]
CLI->>EA: start_example_agent("tshirt_store", H, R)
EA-->>CLI: subprocess handle or failure
alt start failed
CLI-->>U: Exit with error (logs output)
else start ok
CLI->>M: Initialize selected mode
alt mode error
CLI->>P: terminate() and wait()
CLI-->>U: Report failure and exit
else mode runs
U-->>M: Interact
M-->>CLI: Shutdown
CLI->>P: terminate() and wait()
CLI-->>U: Exit
end
end
sequenceDiagram
autonumber
participant Eval as _log_evaluation()
participant N as Normalize
participant V as Scenario.model_validate
participant F as Fallback Builder
Eval->>N: Receive scenario (str/dict/other)
alt string
N-->>Eval: dict{text, expected_outcome?}
else dict
N-->>Eval: dict (unchanged)
else other
N-->>Eval: log error, return
end
Eval->>V: Validate normalized dict
alt valid
V-->>Eval: Scenario
Eval-->>Eval: Log using normalized data
else ValidationError
Eval->>F: Build minimal Scenario (default type)
alt success
F-->>Eval: Scenario
Eval-->>Eval: Proceed with logging
else failure
F-->>Eval: Log and return
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rogue/evaluator_agent/evaluator_agent.py (1)
366-431: Restore_log_evaluationtool signature compatibility
_log_evaluationnow demands a fifth positional argument (scenario_type) with no default. The tool prompt (Lines 120‑134) and existing callers still send only four arguments, so the first call will raiseTypeError: _log_evaluation() missing 1 required positional argument. Give the parameter a default and derive the value from the normalized scenario dict instead of forcing every caller (especially the LLM tool) to change its call shape.- reason: str, - scenario_type: Optional[str], + reason: str, + scenario_type: Optional[str] = None, @@ - "scenario_type": scenario_type, + "scenario_type": scenario_type + or scenario_dict.get("scenario_type"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
AGENTS.md(2 hunks)README.md(2 hunks)examples/tshirt_store_agent/__main__.py(1 hunks)packages/tui/internal/tui/app.go(1 hunks)packages/tui/internal/tui/report_view.go(4 hunks)pyproject.toml(2 hunks)rogue/__init__.py(1 hunks)rogue/__main__.py(7 hunks)rogue/common/tui_installer.py(0 hunks)rogue/common/update_checker.py(0 hunks)rogue/evaluator_agent/evaluator_agent.py(3 hunks)rogue/server/services/qualifire_service.py(0 hunks)
💤 Files with no reviewable changes (3)
- rogue/common/update_checker.py
- rogue/server/services/qualifire_service.py
- rogue/common/tui_installer.py
🧰 Additional context used
📓 Path-based instructions (4)
pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Add new dependencies to pyproject.toml
Files:
pyproject.toml
{rogue,tests,examples}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
{rogue,tests,examples}/**/*.py: Format Python code with Black
Organize imports according to isort conventions
Use type hints for all function signatures
Follow PEP 8 naming (snake_case for variables and functions, PascalCase for classes)
Use try/except blocks for code that may raise exceptions
Ensure code passes flake8 linting
Ensure code passes mypy type checks per .mypy.ini
Address Bandit-reported security issues in Python code
Files:
examples/tshirt_store_agent/__main__.pyrogue/__main__.pyrogue/evaluator_agent/evaluator_agent.pyrogue/__init__.py
examples/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Place example agents under the examples/ directory
Files:
examples/tshirt_store_agent/__main__.py
rogue/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Place application/package code under the rogue/ directory
Files:
rogue/__main__.pyrogue/evaluator_agent/evaluator_agent.pyrogue/__init__.py
🧬 Code graph analysis (4)
packages/tui/internal/tui/report_view.go (1)
packages/tui/internal/styles/styles.go (1)
NewStyle(45-47)
examples/tshirt_store_agent/__main__.py (2)
examples/tshirt_store_agent/tshirt_store_agent.py (1)
create_tshirt_store_agent(75-90)examples/tshirt_store_agent/tshirt_store_agent_executor.py (1)
TShirtStoreAgentExecutor(26-137)
rogue/__main__.py (5)
rogue/run_ui.py (1)
run_ui(20-33)rogue/run_server.py (1)
run_server(122-157)rogue/run_cli.py (1)
run_cli(342-397)rogue/common/tui_installer.py (2)
RogueTuiInstaller(17-205)install_rogue_tui(168-205)rogue/run_tui.py (1)
run_rogue_tui(7-24)
rogue/evaluator_agent/evaluator_agent.py (1)
sdks/python/rogue_sdk/types.py (2)
Scenario(94-133)ScenarioType(45-49)
🪛 GitHub Actions: Rogue
examples/tshirt_store_agent/__main__.py
[error] 19-19: ImportError: attempted relative import with no known parent package.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: codestyle
🔇 Additional comments (2)
pyproject.toml (2)
3-3: LGTM!Version bump is appropriate for introducing new features (example script entry point and expanded packaging).
10-10: LGTM!The
click>=8.0.0dependency is correctly added to support the CLI interface in the example agent (examples/tshirt_store_agent/__main__.pyuses@click.command()and@click.option()decorators).
| from .tshirt_store_agent import create_tshirt_store_agent | ||
| from .tshirt_store_agent_executor import TShirtStoreAgentExecutor |
There was a problem hiding this comment.
🧩 Analysis chain
Verify package structure to resolve ImportError.
The pipeline failure indicates ImportError: attempted relative import with no known parent package on line 19. Relative imports require that examples/tshirt_store_agent/ is a proper Python package with an __init__.py file.
Ensure the following:
- Verify that
examples/__init__.pyandexamples/tshirt_store_agent/__init__.pyexist (even if empty) to establish the package hierarchy. - The module should be executed as a package (
python -m examples.tshirt_store_agent) or via the new script entry (rogue-ai-example-tshirt), not as a standalone script (python examples/tshirt_store_agent/__main__.py).
Run this script to verify the package structure:
🏁 Script executed:
#!/bin/bash
# Description: Check for required __init__.py files in the examples package hierarchy
# Check for __init__.py files
echo "Checking for __init__.py files..."
for path in "examples/__init__.py" "examples/tshirt_store_agent/__init__.py"; do
if [ -f "$path" ]; then
echo "✓ Found: $path"
else
echo "✗ Missing: $path"
echo " This file is required for relative imports to work."
fi
done
# Check how tests are running the example
echo -e "\nChecking test execution method..."
rg -n "examples/tshirt_store_agent/__main__.py" -g "*.{yml,yaml,py,sh}"Length of output: 203
Add missing examples/__init__.py to fix relative imports.
The top‐level examples directory needs an (even empty) __init__.py so that from .tshirt_store_agent import … works. After adding it, run the agent via python -m examples.tshirt_store_agent or the CLI entry, not by executing __main__.py directly.
🧰 Tools
🪛 GitHub Actions: Rogue
[error] 19-19: ImportError: attempted relative import with no known parent package.
🤖 Prompt for AI Agents
In examples/tshirt_store_agent/__main__.py around lines 19-20: the module uses
relative imports (from .tshirt_store_agent ...) which fail unless the examples
package is a proper package; add an __init__.py file (can be empty) to the
top-level examples directory so Python recognizes it as a package, then run the
agent as a module (python -m examples.tshirt_store_agent) or via the CLI entry
rather than executing __main__.py directly.
| packages = ["rogue", "examples"] | ||
|
|
||
| [tool.hatch.build.targets.wheel.sources] | ||
| "rogue" = "rogue" | ||
| "examples" = "examples" | ||
|
|
||
| [project.scripts] | ||
| rogue-ai = "rogue.__main__:main" | ||
| rogue-ai-example-tshirt = "examples.tshirt_store_agent.__main__:main" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify package structure is complete for the new script entry point.
The packaging configuration correctly includes the examples package and defines the new rogue-ai-example-tshirt script entry point. However, ensure the package structure is complete for this to work properly.
The script entry point examples.tshirt_store_agent.__main__:main requires:
examples/__init__.pymust exist (makingexamplesa package)examples/tshirt_store_agent/__init__.pymust exist (makingtshirt_store_agenta subpackage)
Run this script to verify the complete package structure:
🏁 Script executed:
#!/bin/bash
# Description: Verify examples package structure for script entry point
echo "Verifying package structure for rogue-ai-example-tshirt script..."
# Check for required __init__.py files
missing=0
for path in "examples/__init__.py" "examples/tshirt_store_agent/__init__.py"; do
if [ -f "$path" ]; then
echo "✓ $path exists"
else
echo "✗ $path is missing (required for package structure)"
missing=$((missing + 1))
fi
done
# Check for main module
if [ -f "examples/tshirt_store_agent/__main__.py" ]; then
echo "✓ examples/tshirt_store_agent/__main__.py exists"
else
echo "✗ examples/tshirt_store_agent/__main__.py is missing"
missing=$((missing + 1))
fi
# Check for dependencies
echo -e "\nChecking required modules..."
for module in "tshirt_store_agent.py" "tshirt_store_agent_executor.py"; do
if [ -f "examples/tshirt_store_agent/$module" ]; then
echo "✓ examples/tshirt_store_agent/$module exists"
else
echo "✗ examples/tshirt_store_agent/$module is missing"
missing=$((missing + 1))
fi
done
if [ $missing -eq 0 ]; then
echo -e "\n✓ Package structure is complete"
exit 0
else
echo -e "\n✗ Package structure is incomplete ($missing files missing)"
exit 1
fiLength of output: 440
Add missing examples/__init__.py
examples/__init__.py is missing, preventing the examples directory from being recognized as a Python package. Add an empty __init__.py file at examples/__init__.py to fix the rogue-ai-example-tshirt entry point.
🤖 Prompt for AI Agents
In pyproject.toml around lines 53 to 61, the package entry includes "examples"
but the examples directory lacks an __init__.py, so it isn’t recognized as a
package and the rogue-ai-example-tshirt entry point fails; fix by adding an
empty file at examples/__init__.py (or, if using namespace packages
intentionally, update pyproject to use PEP 420 namespace packaging and adjust
package discovery accordingly) — create the empty __init__.py in the examples
folder and commit it.
| process = subprocess.Popen( # nosec: B603 | ||
| cmd, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| ) | ||
|
|
||
| # Give it a moment to start | ||
| time.sleep(2) | ||
|
|
||
| # Check if it's still running | ||
| if process.poll() is None: | ||
| logger.info( | ||
| f"Example agent '{example_name}' started successfully at " | ||
| f"http://{host}:{port}", | ||
| ) | ||
| return process | ||
| else: | ||
| stdout, stderr = process.communicate() | ||
| logger.error( | ||
| f"Failed to start example agent '{example_name}'. " | ||
| f"Exit code: {process.returncode}", | ||
| ) | ||
| if stderr: | ||
| logger.error(f"Error output: {stderr.decode()}") | ||
| return None |
There was a problem hiding this comment.
Stop capturing example agent stdout/stderr to prevent deadlock
The example agent runs indefinitely, but we launch it with stdout=PIPE and stderr=PIPE and never drain those pipes. As soon as the agent logs enough output to fill the OS pipe buffer (~64 KB), the agent blocks on write and the whole workflow stalls. Drop the pipes (or actively drain them) so the agent can run without blocking.
- process = subprocess.Popen( # nosec: B603
- cmd,
- stdout=subprocess.PIPE,
- stderr=subprocess.PIPE,
- )
+ process = subprocess.Popen(cmd) # nosec: B603📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| process = subprocess.Popen( # nosec: B603 | |
| cmd, | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE, | |
| ) | |
| # Give it a moment to start | |
| time.sleep(2) | |
| # Check if it's still running | |
| if process.poll() is None: | |
| logger.info( | |
| f"Example agent '{example_name}' started successfully at " | |
| f"http://{host}:{port}", | |
| ) | |
| return process | |
| else: | |
| stdout, stderr = process.communicate() | |
| logger.error( | |
| f"Failed to start example agent '{example_name}'. " | |
| f"Exit code: {process.returncode}", | |
| ) | |
| if stderr: | |
| logger.error(f"Error output: {stderr.decode()}") | |
| return None | |
| process = subprocess.Popen(cmd) # nosec: B603 | |
| # Give it a moment to start | |
| time.sleep(2) | |
| # Check if it's still running | |
| if process.poll() is None: | |
| logger.info( | |
| f"Example agent '{example_name}' started successfully at " | |
| f"http://{host}:{port}", | |
| ) | |
| return process | |
| else: | |
| stdout, stderr = process.communicate() | |
| logger.error( | |
| f"Failed to start example agent '{example_name}'. " | |
| f"Exit code: {process.returncode}", | |
| ) | |
| if stderr: | |
| logger.error(f"Error output: {stderr.decode()}") | |
| return None |
🤖 Prompt for AI Agents
In rogue/__main__.py around lines 135 to 159, the subprocess is started with
stdout=PIPE and stderr=PIPE and later never drained, which can deadlock when the
agent writes enough output; change the Popen call to not capture the pipes
(remove stdout=PIPE and stderr=PIPE) or redirect them to subprocess.DEVNULL so
the child cannot block on full pipes, and eliminate the later
process.communicate() and stderr.decode() handling (instead check
process.returncode if you need synchronous failure handling or let the caller
manage logs); ensure the function still returns the process on successful start
or None on failure without reading from the pipes.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/tui/internal/tui/help.go (2)
118-119: Add bounds checking for small terminal sizes.The viewport dimension calculations could result in negative values for very small terminal sizes:
- If m.width < 12, viewportWidth would be negative after the -4 adjustment at line 123
- If m.height < 10, viewportHeight would be negative after the -4 adjustment at line 123
Add bounds checking:
// Calculate viewport dimensions viewportWidth := m.width - 8 + if viewportWidth < 40 { + viewportWidth = 40 // Minimum width + } viewportHeight := m.height - 6 + if viewportHeight < 10 { + viewportHeight = 10 // Minimum height + }
61-184: Consider simplifying the viewport rendering approach.The current implementation works but has some complexity:
- Creates a temporary viewport copy (line 122) to avoid modifying the original, which is good
- Resizes it (line 123) despite it being sized in the window resize handler
- Has nested styling layers (viewport.Style and viewportStyle) with multiple dimension adjustments
While this approach achieves the desired centered, bordered layout, the sizing logic could be simplified by relying more on the viewport's pre-configured dimensions from the resize handler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/tui/internal/tui/app.go(5 hunks)packages/tui/internal/tui/help.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/tui/internal/tui/help.go (2)
packages/tui/internal/tui/app.go (1)
Model(179-214)packages/tui/internal/theme/manager.go (1)
CurrentTheme(67-76)
packages/tui/internal/tui/app.go (1)
packages/tui/internal/components/viewport.go (2)
Viewport(52-71)NewViewport(74-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: codestyle
🔇 Additional comments (3)
packages/tui/internal/tui/app.go (2)
205-205: LGTM! Consistent viewport integration.The helpViewport is properly integrated following the same pattern as other viewports (events, summary, report). Initialization, sizing, and lifecycle management are correct.
Also applies to: 290-290, 428-428
1232-1252: LGTM! Help screen navigation is correctly implemented.The keyboard handling for the help screen properly supports home/end navigation and scrolling through the help viewport, following the same pattern as the report screen.
packages/tui/internal/tui/help.go (1)
14-29: LGTM! Styling setup is clean and follows the theme pattern.The mainStyle, title, and header setup correctly use the theme system for consistent styling.
| APIKeys: make(map[string]string), | ||
| }, | ||
| version: "v0.1.5", | ||
| version: "v0.1.6", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify version consistency across the codebase.
The version has been updated to "v0.1.6". Ensure this is consistent with other version references (e.g., package.json, setup.py, README, etc.).
Run the following script to check for version references:
🏁 Script executed:
#!/bin/bash
# Search for version references in common files
rg -n --type-add 'config:*.{json,toml,yaml,yml}' --type config -C2 "version.*0\.1\.[0-9]"
rg -n --type py -C2 "version.*0\.1\.[0-9]"
rg -n --type md -C2 "version.*0\.1\.[0-9]|v0\.1\.[0-9]"Length of output: 3411
🏁 Script executed:
#!/bin/bash
# List version fields from all package.json files
rg -n '"version"\s*:' -g "package.json"Length of output: 239
Sync Python SDK version
- sdks/python/pyproject.toml still declares version = "0.1.4"; bump to "0.1.6" to match root and CLI.
- Root pyproject.toml and rogue/init.py are at 0.1.6; Go TUI code at v0.1.6.
- package.json files remain at 1.0.0—confirm whether the Node SDK/examples should follow their own 1.x scheme or align with 0.1.x.
🤖 Prompt for AI Agents
packages/tui/internal/tui/app.go around line 277: the Go TUI hardcodes version
"v0.1.6" while the Python SDK (sdks/python/pyproject.toml) is still at "0.1.4";
update sdks/python/pyproject.toml to version = "0.1.6" to match root and CLI,
and verify sdks/python/rogue/__init__.py (if present) also reflects 0.1.6;
additionally confirm whether Node package.json files should remain on their 1.x
scheme or be aligned—if Node SDK/examples use a separate major version strategy,
leave them as-is and document the divergence, otherwise bump their version to
match the 0.1.6 scheme.
| // Calculate viewport dimensions | ||
| viewportWidth := m.width - 8 | ||
| viewportHeight := m.height - 6 | ||
|
|
||
| // Create a temporary copy of the viewport to avoid modifying the original | ||
| viewport := m.helpViewport | ||
| viewport.SetSize(viewportWidth-4, viewportHeight-4) | ||
| viewport.SetContent(helpContent) |
There was a problem hiding this comment.
Flag viewport sizing inconsistency between render and resize handler.
The viewport is sized twice with different dimensions:
- In the window resize handler (app.go:428):
m.helpViewport.SetSize(msg.Width - 4, msg.Height - 8) - In this render function (line 123):
viewport.SetSize(viewportWidth-4, viewportHeight-4)where viewportWidth = m.width - 8, resulting in m.width - 12
This inconsistency could cause visual glitches or unexpected behavior. Consider either:
- Using the viewport's already-set size from the resize handler instead of resizing in the render function, or
- Ensuring consistent sizing calculations in both places
Apply this approach to simplify and use the resize handler's dimensions:
- // Calculate viewport dimensions
- viewportWidth := m.width - 8
- viewportHeight := m.height - 6
-
// Create a temporary copy of the viewport to avoid modifying the original
viewport := m.helpViewport
- viewport.SetSize(viewportWidth-4, viewportHeight-4)
viewport.SetContent(helpContent)
+
+ // Get dimensions from the already-sized viewport
+ viewportWidth := viewport.Width + 8 // Add back the margins/borders
+ viewportHeight := viewport.Height + 6🤖 Prompt for AI Agents
In packages/tui/internal/tui/help.go around lines 117 to 124, the viewport is
being resized in render using calculations (viewportWidth-4, viewportHeight-4)
that conflict with the resize handler (app.go:428) which sets m.helpViewport
size to msg.Width-4 by msg.Height-8; to fix, stop recomputing and overriding the
viewport size in the render path: remove the SetSize call here and use the
already-sized m.helpViewport (or, if a copy is needed, copy the viewport without
altering its size or query and reuse its existing dimensions), then only call
SetContent(helpContent) on that viewport copy so sizing is exclusively handled
by the resize handler.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores