-
Notifications
You must be signed in to change notification settings - Fork 184
feat(session): evaluate_js() returning the evaluated string #909
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
Changes from 1 commit
afc4f87
beafca5
ce4b965
02e8ccb
a503fbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| --- | ||
| title: "evaluate_js" | ||
| description: "Evaluate JavaScript on the current page and return its result as a string" | ||
| --- | ||
|
|
||
|
|
||
|
|
||
| The result is stringified the way `evaluate_js` records it: objects and | ||
| arrays as JSON, a JS `null` as the string `"null"`. On failure the typed | ||
| exception is raised with the actual JavaScript error; pass | ||
| `raise_on_failure=False` to get the `ExecutionResult` envelope instead. | ||
|
|
||
| ```python | ||
| payload = json.loads(session.evaluate_js("(async () => JSON.stringify(await res.json()))()")) | ||
| ``` | ||
|
|
||
|
|
||
| ## Parameters | ||
|
|
||
| <ParamField path="code" type="str" required> | ||
| </ParamField> | ||
|
|
||
| <ParamField path="raise_on_failure" type="bool" default="True"> | ||
| </ParamField> | ||
|
|
||
| ## Returns | ||
|
|
||
| `UnionType`[`str`, <Visibility for="humans">[`ExecutionResult`](/sdk-reference/misc/executionresult)</Visibility><Visibility for="agents">[`ExecutionResult`](/sdk-reference/misc/executionresult.md)</Visibility>] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1047,6 +1047,41 @@ def execute( | |
| self.aexecute(action=action, raise_on_failure=raise_on_failure, **kwargs) # pyright: ignore [reportArgumentType] | ||
| ) | ||
|
|
||
| # raise_on_failure=True (default) -> the evaluated string; failures raise | ||
| @overload | ||
| async def aevaluate_js(self, code: str, *, raise_on_failure: Literal[True] = ...) -> str: ... | ||
|
|
||
| # raise_on_failure=False -> the ExecutionResult envelope, like execute() | ||
| @overload | ||
| async def aevaluate_js(self, code: str, *, raise_on_failure: Literal[False]) -> ExecutionResult: ... | ||
|
|
||
| async def aevaluate_js(self, code: str, *, raise_on_failure: bool = True) -> str | ExecutionResult: | ||
|
Comment on lines
+1051
to
+1058
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
# Confirm that the type-case configuration and overload declarations are present.
rg -n -C 3 'evaluate_js\(|aevaluate_js\(|raise_on_failure: Literal|raise_on_failure: bool' \
packages/notte-browser/src/notte_browser/session.py \
packages/notte-sdk/src/notte_sdk/endpoints/sessions.py \
typing_cases/evaluate_js_overloads.pyRepository: nottelabs/notte Length of output: 50373 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- local overloads and implementations ---'
sed -n '1044,1085p' packages/notte-browser/src/notte_browser/session.py
sed -n '1596,1640p' packages/notte-sdk/src/notte_sdk/endpoints/sessions.py
printf '%s\n' '--- typing case ---'
cat -n typing_cases/evaluate_js_overloads.py
printf '%s\n' '--- typing configuration and checker references ---'
rg -n -C 2 'pyright|mypy|typing_cases|evaluate_js_overloads|reportCallIssue|overload' \
pyproject.toml setup.cfg tox.ini .github packages typing_cases 2>/dev/null | head -n 240Repository: nottelabs/notte Length of output: 23713 Add a non-literal
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| """ | ||
| Evaluate JavaScript on the current page and return its result as a string. | ||
|
|
||
| The result is stringified the way `evaluate_js` records it: objects and | ||
| arrays as JSON, a JS `null` as the string `"null"`. On failure the typed | ||
| exception is raised with the actual JavaScript error; pass | ||
| `raise_on_failure=False` to get the `ExecutionResult` envelope instead. | ||
| """ | ||
| result = await self.aexecute(type="evaluate_js", code=code, raise_on_failure=raise_on_failure) | ||
| if not raise_on_failure: | ||
| return result | ||
| assert result.data is not None # evaluate_js always sets data on success | ||
| return result.data.markdown | ||
|
|
||
| @overload | ||
| def evaluate_js(self, code: str, *, raise_on_failure: Literal[True] = ...) -> str: ... | ||
|
|
||
| @overload | ||
| def evaluate_js(self, code: str, *, raise_on_failure: Literal[False]) -> ExecutionResult: ... | ||
|
|
||
| def evaluate_js(self, code: str, *, raise_on_failure: bool = True) -> str | ExecutionResult: | ||
| """ | ||
| Synchronous version of aevaluate_js. | ||
| """ | ||
| return asyncio.run(self.aevaluate_js(code, raise_on_failure=raise_on_failure)) | ||
|
|
||
| @overload | ||
| async def ascrape(self, /, *, only_images: Literal[True], raise_on_failure: bool = True) -> list[ImageData]: ... | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1596,3 +1596,36 @@ def execute( | |
| agent_message=fallback_message, | ||
| ) | ||
| return result | ||
|
|
||
| # raise_on_failure=True (default) -> the evaluated string; failures raise | ||
| @overload | ||
| def evaluate_js(self, code: str, *, raise_on_failure: Literal[True] = ...) -> str: ... | ||
|
|
||
| # raise_on_failure=False -> the ExecutionResult envelope, like execute() | ||
| @overload | ||
| def evaluate_js(self, code: str, *, raise_on_failure: Literal[False]) -> ExecutionResult: ... | ||
|
Comment on lines
+1603
to
+1607
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When callers pass a computed Knowledge Base Used: SDK client and remote resources Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/notte-sdk/src/notte_sdk/endpoints/sessions.py
Line: 1602-1606
Comment:
**Boolean flags miss overloads**
When callers pass a computed `bool` to `raise_on_failure`, neither literal-only overload matches even though the implementation accepts `bool`, causing valid `evaluate_js` calls to fail static type checking. Add a plain-`bool` overload returning `str | ExecutionResult` here and for both local helper variants, as the analogous scrape API does.
**Knowledge Base Used:** [SDK client and remote resources](https://app.greptile.com/nottelabs/-/custom-context/knowledge-base/nottelabs/notte/-/docs/sdk-client.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
| def evaluate_js(self, code: str, *, raise_on_failure: bool = True) -> str | ExecutionResult: | ||
| """ | ||
| Evaluate JavaScript on the current page and return its result as a string. | ||
|
|
||
| The result is stringified the way `evaluate_js` records it: objects and | ||
| arrays as JSON, a JS `null` as the string `"null"`. On failure the typed | ||
| exception is raised with the actual JavaScript error; pass | ||
| `raise_on_failure=False` to get the `ExecutionResult` envelope instead. | ||
|
|
||
| ```python | ||
| payload = json.loads(session.evaluate_js("(async () => JSON.stringify(await res.json()))()")) | ||
| ``` | ||
| """ | ||
| result = self.execute(type="evaluate_js", code=code, raise_on_failure=raise_on_failure) | ||
| if not raise_on_failure: | ||
| return result | ||
| if result.data is None: | ||
| # an API build that predates the eval-js fix reports success without data | ||
| raise NotteBaseError( | ||
| dev_message="evaluate_js returned no data", | ||
| user_message="evaluate_js returned no data", | ||
| agent_message="evaluate_js returned no data", | ||
| ) | ||
| return result.data.markdown | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """Remote `evaluate_js()`: the string on success, the typed raise on failure.""" | ||
|
|
||
| import datetime as dt | ||
|
|
||
| import pytest | ||
| from notte_core.actions import EvaluateJsAction | ||
| from notte_core.browser.observation import ExecutionResult | ||
| from notte_core.data.space import DataSpace | ||
| from notte_core.errors.actions import ActionExecutionError | ||
| from notte_core.errors.base import ErrorConfig, NotteBaseError | ||
|
|
||
| from tests.sdk.test_execute_raise_on_failure import over_the_wire, remote_session | ||
|
|
||
| CODE = "1 + 1" | ||
|
|
||
|
|
||
| def eval_result(*, success: bool, markdown: str | None = None, exception: Exception | None = None) -> ExecutionResult: | ||
| now = dt.datetime.now(dt.timezone.utc) | ||
| return ExecutionResult( | ||
| action=EvaluateJsAction(code=CODE), | ||
| success=success, | ||
| message="ok" if success else "JavaScript evaluation failed: boom", | ||
| data=DataSpace(markdown=markdown) if markdown is not None else None, | ||
| exception=exception, | ||
| started_at=now, | ||
| ended_at=now, | ||
| ) | ||
|
|
||
|
|
||
| def test_evaluate_js_returns_the_string() -> None: | ||
| session = remote_session(over_the_wire(eval_result(success=True, markdown="2"))) | ||
|
|
||
| assert session.evaluate_js(CODE) == "2" | ||
|
|
||
|
|
||
| def test_evaluate_js_failure_raises_the_typed_error() -> None: | ||
| with ErrorConfig.message_mode("user"): | ||
| exception = ActionExecutionError(action_id="evaluate_js", url="https://example.com", reason="boom") | ||
| session = remote_session(over_the_wire(eval_result(success=False, exception=exception))) | ||
|
|
||
| with pytest.raises(ActionExecutionError): | ||
| _ = session.evaluate_js(CODE) | ||
|
|
||
|
|
||
| def test_evaluate_js_returns_the_envelope_when_not_raising() -> None: | ||
| session = remote_session(over_the_wire(eval_result(success=False))) | ||
|
|
||
| result = session.evaluate_js(CODE, raise_on_failure=False) | ||
|
|
||
| assert isinstance(result, ExecutionResult) | ||
| assert result.success is False | ||
|
|
||
|
|
||
| def test_evaluate_js_success_without_data_raises_instead_of_returning_none() -> None: | ||
| """An API build that predates the eval-js fix can report success with no data.""" | ||
| session = remote_session(over_the_wire(eval_result(success=True))) | ||
|
|
||
| with pytest.raises(NotteBaseError, match="returned no data"): | ||
| _ = session.evaluate_js(CODE) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| """`evaluate_js()` returns the evaluated string; the envelope stays on the `False` overload.""" | ||
|
|
||
| import pytest | ||
| from notte_browser.session import NotteSession | ||
| from notte_core.browser.observation import ExecutionResult | ||
| from notte_core.errors.actions import ActionExecutionError | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_aevaluate_js_returns_the_string() -> None: | ||
| async with NotteSession(headless=True) as session: | ||
| assert await session.aevaluate_js("1 + 1") == "2" | ||
| # a JS `null` is a successful evaluation and arrives as the string "null" | ||
| assert await session.aevaluate_js("null") == "null" | ||
| assert await session.aevaluate_js("[1, 2]") == "[\n 1,\n 2\n]" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_aevaluate_js_failure_raises_the_js_error() -> None: | ||
| async with NotteSession(headless=True) as session: | ||
| with pytest.raises(ActionExecutionError, match="JavaScript evaluation failed"): | ||
| _ = await session.aevaluate_js("notAFunction()") | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_aevaluate_js_returns_the_envelope_when_not_raising() -> None: | ||
| async with NotteSession(headless=True) as session: | ||
| result = await session.aevaluate_js("notAFunction()", raise_on_failure=False) | ||
|
|
||
| assert isinstance(result, ExecutionResult) | ||
| assert result.success is False | ||
| assert result.message.startswith("JavaScript evaluation failed:") | ||
|
|
||
|
|
||
| def test_evaluate_js_sync_returns_the_string() -> None: | ||
| with NotteSession(headless=True) as session: | ||
| assert session.evaluate_js("1 + 1") == "2" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| """Reveal-type cases for evaluate_js overload resolution (checked by basedpyright and ty). | ||
|
|
||
| Keep this file free of runtime side effects; checkers only need the annotations. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import reveal_type | ||
|
|
||
| from notte_browser.session import NotteSession | ||
| from notte_sdk.endpoints.sessions import RemoteSession | ||
|
|
||
|
|
||
| def _check_remote_session(session: RemoteSession) -> None: | ||
| text = session.evaluate_js("1 + 1") | ||
| reveal_type(text) # str | ||
| envelope = session.evaluate_js("1 + 1", raise_on_failure=False) | ||
| reveal_type(envelope) # ExecutionResult | ||
|
|
||
|
|
||
| async def _check_local_session(session: NotteSession) -> None: | ||
| text = await session.aevaluate_js("1 + 1") | ||
| reveal_type(text) # str | ||
| envelope = await session.aevaluate_js("1 + 1", raise_on_failure=False) | ||
| reveal_type(envelope) # ExecutionResult | ||
| sync_text = session.evaluate_js("1 + 1") | ||
| reveal_type(sync_text) # str | ||
| sync_envelope = session.evaluate_js("1 + 1", raise_on_failure=False) | ||
| reveal_type(sync_envelope) # ExecutionResult |
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the evaluate_js examples self-contained.
The examples use
reswithout defining it, and the documentation example also usesjson.loadswithout importingjson. Copying them as shown can fail with a JavaScriptReferenceErroror PythonNameError.Please define the JavaScript value directly and include the required Python import, for example:
📍 Affects 2 files
docs/src/sdk-reference/remotesession/evaluate_js.mdx#L13-L15(this comment)packages/notte-sdk/src/notte_sdk/endpoints/sessions.py#L1617-L1619🤖 Prompt for AI Agents