Add Lightpanda browser backend and MCP catalog option#220
Open
adybag14-cyber wants to merge 11 commits intompfaffenberger:mainfrom
Open
Add Lightpanda browser backend and MCP catalog option#220adybag14-cyber wants to merge 11 commits intompfaffenberger:mainfrom
adybag14-cyber wants to merge 11 commits intompfaffenberger:mainfrom
Conversation
…T.md cache (mpfaffenberger#212) * fix(cd): reload agent after /cd to flush stale system prompt and AGENT.md cache After a successful /cd, the agent's PydanticAgent instance was reused verbatim for subsequent messages. This meant: 1. Any CWD-dependent content baked into the system prompt at agent construction time (e.g. the Walmart plugin's working-directory line) was never refreshed, so the LLM kept receiving the old path. 2. Project-local AGENT.md rules loaded by load_puppy_rules() were cached in self._puppy_rules and never re-read for the new directory, silently ignoring the new project's rules (or incorrectly carrying the old ones forward). Fixes: * base_agent.py — set self._puppy_rules = None at the top of reload_code_generation_agent() so every reload always performs a fresh disk read from the current working directory. * core_commands.py — call get_current_agent().reload_code_generation_agent() after os.chdir() succeeds. The reload is best-effort (wrapped in try/except) so a failure never aborts the directory change itself. Tests: * test_command_handler.py — update test_cd_valid_change to assert the agent is reloaded; add test_cd_valid_change_reload_failure_is_nonfatal to verify a broken reload does not bubble up. * test_base_agent_full_coverage.py — add test_reload_clears_puppy_rules_cache which uses a spy on load_puppy_rules() to prove _puppy_rules is None (i.e. cache cleared) at the moment the fresh read begins. Closes mpfaffenberger#205 * fix(cd): emit warning on agent reload failure instead of silent pass Address CodeRabbit review comments on PR mpfaffenberger#212: - Replace bare `except: pass` with `emit_warning()` so users see feedback when agent reload fails after /cd - Strengthen test assertions to verify chdir, success message, reload attempt, and warning emission Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: n0h0123 <nathan.hicks0@walmart.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tions The DEFAULT_BANNER_COLORS dict in config.py doesn't have 'mcp_tool_call', so the tests were failing. Updated both test cases in TestGetConfigKeys to match the actual implementation.
📝 WalkthroughWalkthroughAdds a Lightpanda MCP server template and integrates Lightpanda as a BrowserManager backend; refines MCPServerTemplate env-var parsing to ignore Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant BM as BrowserManager
participant Sys as System
participant LP as Lightpanda
participant PW as Playwright
Client->>BM: async_initialize(browser_type="lightpanda")
BM->>Sys: resolve executable, host, port (or find free port)
BM->>Sys: spawn Lightpanda subprocess (with args)
Sys-->>LP: process started
BM->>LP: read stderr / poll for readiness
BM->>BM: _connect_lightpanda_over_cdp (retry / timeout)
LP-->>BM: CDP endpoint responds
BM->>PW: attach/connect over CDP / create context
PW-->>BM: browser/context ready
BM-->>Client: initialized
Client->>BM: cleanup()
BM->>PW: close context/browser
BM->>Sys: terminate Lightpanda subprocess
Sys-->>LP: process stopped
BM-->>Client: cleaned up
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🧹 Nitpick comments (1)
code_puppy/tools/browser/browser_manager.py (1)
154-159: Free-port probing is race-prone during concurrent startup.At Lines 154-159, the socket is closed before Lightpanda binds, so another process can grab the same port and cause intermittent startup failures. Consider retrying startup with a newly selected port when bind/connect fails.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/tools/browser/browser_manager.py` around lines 154 - 159, The current _find_free_port() probe is race-prone because the socket is closed before the browser (Lightpanda) binds; change the startup/launch logic that uses _find_free_port() so it handles bind/connect failures by catching the binding-related exceptions (e.g., OSError/ConnectionRefused) from the Lightpanda process or client socket and retrying: on failure call _find_free_port() again, pick a new port, and retry launching/connecting up to a reasonable max attempts with short backoff; ensure the retry loop lives around the code that starts or connects to Lightpanda (the caller of _find_free_port()) so a new free port is chosen only when a bind/connect error is observed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/mcp_/server_registry_catalog.py`:
- Around line 474-475: The env entry "LIGHTPANDA_CDP_URL": "${cdp_url}" is being
parsed by get_environment_vars() as an environment-variable reference; change
this to a non-env placeholder (e.g., "LIGHTPANDA_CDP_URL": "{cdp_url}" or remove
it from the "env" dict and pass it as a command arg) so get_environment_vars()
does not treat it as a required env var; update usages that substitute the
placeholder accordingly (refer to the LIGHTPANDA_CDP_URL key and the
get_environment_vars() function to locate where to change).
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 360-364: The branch handling self.browser_type in the
BrowserManager silently falls through to the Chromium path for unknown values;
update the logic in the method that contains the lightpanda check (references:
self.browser_type, _initialize_lightpanda_browser, self._initialized) to
explicitly raise a clear exception (e.g., ValueError) for unsupported
browser_type values instead of silently defaulting to Chromium, so callers fail
fast and can correct invalid inputs.
- Around line 184-193: The _get_lightpanda_port function currently parses
LIGHTPANDA_PORT but doesn't validate its numeric range; update the code in
_get_lightpanda_port to parse configured_port (as it's already doing) and then
explicitly validate that the integer is within the valid TCP port range
(1–65535), raising a RuntimeError with a clear message including configured_port
if it is out of range or not a valid integer (reuse the existing except
ValueError handling to wrap parsing errors), otherwise return the validated int;
reference: function _get_lightpanda_port and variable configured_port.
---
Nitpick comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 154-159: The current _find_free_port() probe is race-prone because
the socket is closed before the browser (Lightpanda) binds; change the
startup/launch logic that uses _find_free_port() so it handles bind/connect
failures by catching the binding-related exceptions (e.g.,
OSError/ConnectionRefused) from the Lightpanda process or client socket and
retrying: on failure call _find_free_port() again, pick a new port, and retry
launching/connecting up to a reasonable max attempts with short backoff; ensure
the retry loop lives around the code that starts or connects to Lightpanda (the
caller of _find_free_port()) so a new free port is chosen only when a
bind/connect error is observed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code_puppy/mcp_/server_registry_catalog.pycode_puppy/tools/browser/browser_control.pycode_puppy/tools/browser/browser_manager.pytests/tools/browser/test_browser_manager_full.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/mcp/test_server_registry_catalog.py (1)
261-264: Strengthen the negative assertion for placeholder leakage.Line 263 only checks
"{cdp_url}", which is specific to one failure shape. Add a direct check for"cdp_url"too, so regressions are caught even if parsing changes.🧪 Suggested assertion hardening
env_vars = template.get_environment_vars() assert "REAL_API_KEY" in env_vars + assert "cdp_url" not in env_vars assert "{cdp_url}" not in env_vars + assert len(env_vars) == 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/mcp/test_server_registry_catalog.py` around lines 261 - 264, Add a stronger negative assertion to the test that checks for placeholder leakage: in the test using template.get_environment_vars() (the same block that currently asserts "{cdp_url}" not in env_vars), also assert that "cdp_url" not in env_vars to catch regressions where the braces are stripped or parsing changes; update the assertions near the env_vars = template.get_environment_vars() line accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 323-327: The subprocess stderr is currently piped in
self._lightpanda_process via asyncio.create_subprocess_exec which risks blocking
if stderr fills before you read it; either change
stderr=asyncio.subprocess.DEVNULL in the create_subprocess_exec call to discard
output, or keep stderr=PIPE but spawn a background asyncio Task immediately
after process creation (e.g., _drain_lightpanda_stderr task) that continuously
reads from self._lightpanda_process.stderr (readuntil/read or readexactly in a
loop until EOF) to drain the buffer while you await the CDP connection in the
existing connection logic (the background task should be cancelled/awaited on
process exit).
---
Nitpick comments:
In `@tests/mcp/test_server_registry_catalog.py`:
- Around line 261-264: Add a stronger negative assertion to the test that checks
for placeholder leakage: in the test using template.get_environment_vars() (the
same block that currently asserts "{cdp_url}" not in env_vars), also assert that
"cdp_url" not in env_vars to catch regressions where the braces are stripped or
parsing changes; update the assertions near the env_vars =
template.get_environment_vars() line accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code_puppy/mcp_/server_registry_catalog.pycode_puppy/tools/browser/browser_manager.pytests/mcp/test_server_registry_catalog.pytests/tools/browser/test_browser_manager_full.py
Author
|
thank you code rabbit for accepting me |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code_puppy/tools/browser/browser_manager.py (1)
173-177: Validate path-likeLIGHTPANDA_EXECUTABLEas an executable file, not just existing.At Line 174,
Path(...).exists()also accepts directories/non-executable files, which fails later with less clear process-launch errors.🛠️ Proposed refinement
if path_like: - if Path(executable).exists(): + candidate = Path(executable) + if candidate.is_file() and os.access(candidate, os.X_OK): return executable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/tools/browser/browser_manager.py` around lines 173 - 177, The current check uses Path(executable).exists() which allows directories or non-executable files; update the path-like branch to ensure the target is a regular file and is executable before returning. In the function where you inspect path_like and the variable executable (the block using Path(executable).exists()), replace the exists() check with a combined check such as Path(executable).is_file() and os.access(executable, os.X_OK) (or equivalent Path permission check) so only an actual executable file is accepted; keep the else branch using shutil.which(executable) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 470-477: The lightpanda branch is doing a case-sensitive check (if
self.browser_type == "lightpanda") while the later logic lowercases
requested_browser; normalize detection by using the same lowercasing approach:
compute a normalized value from self.browser_type (e.g., requested_browser =
(self.browser_type or "chromium").lower()) before the lightpanda check, then use
that normalized value to decide whether to call
_initialize_lightpanda_browser(), emit_info(session_id) and set _initialized;
ensure references to self.browser_type, requested_browser,
_initialize_lightpanda_browser, session_id, and _initialized are updated
accordingly so inputs like "LightPanda" match.
- Around line 330-340: The loop that repeatedly tries to connect via
self._playwright.chromium.connect_over_cdp(endpoint) can block past the overall
LIGHTPANDA_STARTUP_TIMEOUT because connect_over_cdp is awaited directly; wrap
each connect attempt in asyncio.wait_for with the remaining time until the
startup deadline so any single hung attempt is bounded, compute a deadline =
start + LIGHTPANDA_STARTUP_TIMEOUT and before each try calculate remaining =
max(0.0, deadline - loop.time()), call
asyncio.wait_for(self._playwright.chromium.connect_over_cdp(endpoint),
timeout=remaining_or_short_default) and catch asyncio.TimeoutError to continue
looping and set last_error, keeping existing last_error handling and the final
RuntimeError message that includes last_error and endpoint.
---
Nitpick comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 173-177: The current check uses Path(executable).exists() which
allows directories or non-executable files; update the path-like branch to
ensure the target is a regular file and is executable before returning. In the
function where you inspect path_like and the variable executable (the block
using Path(executable).exists()), replace the exists() check with a combined
check such as Path(executable).is_file() and os.access(executable, os.X_OK) (or
equivalent Path permission check) so only an actual executable file is accepted;
keep the else branch using shutil.which(executable) unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code_puppy/tools/browser/browser_manager.pytests/mcp/test_server_registry_catalog.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/mcp/test_server_registry_catalog.py
Author
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 `@code_puppy/tools/browser/browser_manager.py`:
- Around line 217-227: Parse LIGHTPANDA_STARTUP_TIMEOUT as a float in
_get_lightpanda_startup_timeout, then validate that the parsed value is finite
and strictly positive using math.isfinite(timeout) and timeout > 0.0; if the
value is non-finite (nan/inf) or non-positive, raise a RuntimeError with a clear
message including the raw value, otherwise return the timeout (optionally
preserving the existing minimum of 1.0 via max(timeout, 1.0)).
In `@tests/tools/browser/test_browser_manager_full.py`:
- Around line 424-425: The test currently uses
mock_playwright.stop.assert_called_once() and
mock_stop_lightpanda.assert_called_once() but those targets are awaited in
BrowserManager._cleanup(), so change the assertions to use assert_awaited_once()
to verify they were awaited; locate the assertions referencing
mock_playwright.stop and mock_stop_lightpanda in
tests/tools/browser/test_browser_manager_full.py and replace
assert_called_once() with assert_awaited_once() for those async mocks.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code_puppy/tools/browser/browser_manager.pytests/tools/browser/test_browser_manager_full.py
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/tools/browser/test_browser_manager_full.py (1)
150-150:⚠️ Potential issue | 🟡 MinorUse awaited assertions for async initializer mocks.
Line 150 and Line 173 currently use
assert_called_once(), which does not guarantee the coroutine was awaited.Proposed fix
- mock_init_lightpanda.assert_called_once() + mock_init_lightpanda.assert_awaited_once() ... - mock_init_lightpanda.assert_called_once() + mock_init_lightpanda.assert_awaited_once()Also applies to: 173-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/browser/test_browser_manager_full.py` at line 150, The test uses synchronous assert_called_once() on async initializer mocks; replace those with await-aware assertions (e.g., use AsyncMock.assert_awaited_once() or assert_awaited_once()) for the async initializer mocks such as mock_init_lightpanda (and the other initializer mock at line 173) so the test verifies the coroutine was actually awaited rather than only called.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 428-447: The shutdown sequence in _stop_lightpanda_process should
guarantee calling _stop_lightpanda_stderr_drain even if terminate/kill/wait
raise; wrap the existing "if process: ..." termination/kill/wait logic in a
try/finally and move the await self._stop_lightpanda_stderr_drain() call into
the finally block so cleanup always runs, preserving existing
contextlib.suppress usages and allowing any exceptions from the termination
logic to propagate after the finally completes.
---
Duplicate comments:
In `@tests/tools/browser/test_browser_manager_full.py`:
- Line 150: The test uses synchronous assert_called_once() on async initializer
mocks; replace those with await-aware assertions (e.g., use
AsyncMock.assert_awaited_once() or assert_awaited_once()) for the async
initializer mocks such as mock_init_lightpanda (and the other initializer mock
at line 173) so the test verifies the coroutine was actually awaited rather than
only called.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code_puppy/tools/browser/browser_manager.pytests/tools/browser/test_browser_manager_full.py
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
code_puppy/tools/browser/browser_manager.py (1)
482-511: Normalize custom browser type lookup for consistency.Built-in and Lightpanda routing are case-insensitive, but plugin custom type lookup still uses exact-case keys. Consider normalizing custom type matching too, so
browser_type="MyPlugin"and"myplugin"behave consistently.♻️ Suggested adjustment
- if self.browser_type and self.browser_type in _CUSTOM_BROWSER_TYPES: + custom_key_map = {key.lower(): key for key in _CUSTOM_BROWSER_TYPES} + if self.browser_type and requested_browser in custom_key_map: + custom_key = custom_key_map[requested_browser] emit_info( - f"Using custom browser type '{self.browser_type}' " + f"Using custom browser type '{custom_key}' " f"(session: {self.session_id})" ) - init_func = _CUSTOM_BROWSER_TYPES[self.browser_type] + init_func = _CUSTOM_BROWSER_TYPES[custom_key]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/tools/browser/browser_manager.py` around lines 482 - 511, The custom browser lookup is case-sensitive while built-in routing is not; modify the logic in initialize (where requested_browser is computed, and where _CUSTOM_BROWSER_TYPES is consulted) to normalize case for custom types too: compute a lowercased key (e.g., requested_browser) and map it to the actual init function from _CUSTOM_BROWSER_TYPES by using a normalized-key mapping or by storing/looking-up keys lowercased before calling init_func (refer to self.browser_type, requested_browser, _CUSTOM_BROWSER_TYPES, and init_func) so that "MyPlugin" and "myplugin" resolve the same way and still set self._context/self._browser and self._initialized as before.tests/tools/browser/test_browser_manager_full.py (1)
192-195: Useasyncio.StreamReaderformock_process.stderrto exercise drain startup path.These tests currently set
stderrtoAsyncMock, which skips_start_lightpanda_stderr_drain()’sisinstance(..., asyncio.StreamReader)branch. Swapping to a realStreamReaderwould cover the drain-task path more accurately.Also applies to: 229-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/browser/test_browser_manager_full.py` around lines 192 - 195, Replace the test's mock_process.stderr AsyncMock with a real asyncio.StreamReader so _start_lightpanda_stderr_drain()'s isinstance(..., asyncio.StreamReader) branch executes; specifically, create an asyncio.StreamReader() instance and assign it to mock_process.stderr (and if needed use a helper to attach a dummy transport or call feed_data/feed_eof to simulate stderr output) for both the mock_process setup at mock_process.returncode = None / mock_process.stderr and the similar block around lines 229-235 so the drain task path is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 482-511: The custom browser lookup is case-sensitive while
built-in routing is not; modify the logic in initialize (where requested_browser
is computed, and where _CUSTOM_BROWSER_TYPES is consulted) to normalize case for
custom types too: compute a lowercased key (e.g., requested_browser) and map it
to the actual init function from _CUSTOM_BROWSER_TYPES by using a normalized-key
mapping or by storing/looking-up keys lowercased before calling init_func (refer
to self.browser_type, requested_browser, _CUSTOM_BROWSER_TYPES, and init_func)
so that "MyPlugin" and "myplugin" resolve the same way and still set
self._context/self._browser and self._initialized as before.
In `@tests/tools/browser/test_browser_manager_full.py`:
- Around line 192-195: Replace the test's mock_process.stderr AsyncMock with a
real asyncio.StreamReader so _start_lightpanda_stderr_drain()'s isinstance(...,
asyncio.StreamReader) branch executes; specifically, create an
asyncio.StreamReader() instance and assign it to mock_process.stderr (and if
needed use a helper to attach a dummy transport or call feed_data/feed_eof to
simulate stderr output) for both the mock_process setup at
mock_process.returncode = None / mock_process.stderr and the similar block
around lines 229-235 so the drain task path is exercised.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code_puppy/tools/browser/browser_manager.pytests/tools/browser/test_browser_manager_full.py
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/tools/browser/test_browser_manager_full.py (1)
298-298: Minor: Redundant import inside test function.
asynciois already imported at the module level (line 3). The import at line 298 is unnecessary.Proposed fix
`@pytest.mark.asyncio` async def test_connect_lightpanda_over_cdp_attempt_respects_timeout_budget(self): """Test CDP connect attempts are bounded by LIGHTPANDA_STARTUP_TIMEOUT.""" - import asyncio - from code_puppy.tools.browser.browser_manager import BrowserManager🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tools/browser/test_browser_manager_full.py` at line 298, Remove the redundant local "import asyncio" inside the test function and rely on the module-level asyncio import (already present near the top of the file); locate the inline "import asyncio" statement and delete it so there is only the single module-level import left.code_puppy/tools/browser/browser_manager.py (1)
228-233: Minor: Silent floor adjustment after validation may cause confusion.The validation at line 228 requires
timeout > 0, but line 233 then silently bumps values in(0, 1)to1.0. A user settingLIGHTPANDA_STARTUP_TIMEOUT=0.5might expect a 500ms timeout but will get 1 second without notice.Consider either:
- Moving the floor check into the validation with a clear error message, or
- Documenting this behavior
This is a minor UX quirk, not a bug.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/tools/browser/browser_manager.py` around lines 228 - 233, The current validation accepts timeout > 0 but then silently returns max(timeout, 1.0), which hides values in (0,1); update the validation in the function that parses LIGHTPANDA_STARTUP_TIMEOUT (variables: timeout, timeout_raw) to enforce the minimum floor at validation time (e.g., require timeout >= 1.0) and raise a RuntimeError with a clear message like "LIGHTPANDA_STARTUP_TIMEOUT must be a finite number >= 1.0, got: {timeout_raw}"; alternatively, if you prefer non-breaking behavior, emit a warning or info log when max(timeout, 1.0) is applied so users know a value <1.0 was bumped to 1.0 — but do not silently adjust without notification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code_puppy/tools/browser/browser_manager.py`:
- Around line 228-233: The current validation accepts timeout > 0 but then
silently returns max(timeout, 1.0), which hides values in (0,1); update the
validation in the function that parses LIGHTPANDA_STARTUP_TIMEOUT (variables:
timeout, timeout_raw) to enforce the minimum floor at validation time (e.g.,
require timeout >= 1.0) and raise a RuntimeError with a clear message like
"LIGHTPANDA_STARTUP_TIMEOUT must be a finite number >= 1.0, got: {timeout_raw}";
alternatively, if you prefer non-breaking behavior, emit a warning or info log
when max(timeout, 1.0) is applied so users know a value <1.0 was bumped to 1.0 —
but do not silently adjust without notification.
In `@tests/tools/browser/test_browser_manager_full.py`:
- Line 298: Remove the redundant local "import asyncio" inside the test function
and rely on the module-level asyncio import (already present near the top of the
file); locate the inline "import asyncio" statement and delete it so there is
only the single module-level import left.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code_puppy/tools/browser/browser_manager.pytests/tools/browser/test_browser_manager_full.py
Author
|
okay code rabbit you are just rage baiting now |
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.
Light panda is fully in zig and the CDP server too, TLDR: way faster than playwright and way less ram used.
Also read all code, I don't want to cause any regressions at all, perhaps i can add a minimal readme if you want.
Summary by CodeRabbit
New Features
Bug Fixes
Tests