-
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 3 commits
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 |
|---|---|---|
|
|
@@ -64,7 +64,7 @@ | |
| from notte_core.common.telemetry import track_usage | ||
| from notte_core.credentials.base import BaseVault, LocatorAttributes | ||
| from notte_core.data.space import DataSpace, ImageData, StructuredData, TBaseModel | ||
| from notte_core.errors.actions import ActionExecutionError, InvalidActionError | ||
| from notte_core.errors.actions import ActionExecutionError, EvaluateJsNoDataError, InvalidActionError | ||
| from notte_core.errors.base import NotteBaseError | ||
| from notte_core.errors.provider import RateLimitError | ||
| from notte_core.profiling import profiler | ||
|
|
@@ -1047,6 +1047,44 @@ 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 | ||
| if result.data is None: | ||
| # cannot happen with this package's execute path, which always sets | ||
| # data on a successful eval; a typed error beats a stripped assert | ||
| raise EvaluateJsNoDataError() | ||
| 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 |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ | |
| from notte_core.common.resource import SyncResource | ||
| from notte_core.common.telemetry import track_usage | ||
| from notte_core.data.space import ImageData, StructuredData, TBaseModel | ||
| from notte_core.errors.actions import EvaluateJsNoDataError | ||
| from notte_core.errors.base import NotteBaseError | ||
| from notte_core.utils.files import create_or_append_cookies_to_file | ||
| from pydantic import BaseModel | ||
|
|
@@ -1596,3 +1597,32 @@ 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 EvaluateJsNoDataError() | ||
| 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, EvaluateJsNoDataError | ||
| from notte_core.errors.base import ErrorConfig | ||
|
|
||
| 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(EvaluateJsNoDataError, match="returned no data"): | ||
| _ = session.evaluate_js(CODE) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| """`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:") | ||
|
|
||
|
|
||
| # NOTE: no sync-variant test here on purpose. A sync NotteSession (asyncio.run | ||
| # under nest_asyncio) breaks the next async browser launch in the same pytest | ||
| # process, so mixing the two in one file flakes under random test ordering. | ||
| # The sync wrapper is a one-line delegation to aevaluate_js and its overload | ||
| # typing is pinned by typing_cases/evaluate_js_overloads.py. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| """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 | ||
|
|
||
|
|
||
| def _check_computed_bool(session: RemoteSession, flag: bool) -> None: | ||
| # a non-literal bool matches via argument expansion on both checkers; | ||
| # no plain-bool overload is needed | ||
| either = session.evaluate_js("1 + 1", raise_on_failure=flag) | ||
| reveal_type(either) # str | ExecutionResult | ||
|
Comment on lines
+24
to
+25
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 | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -A12 -B3 'Literal\[True\]|Literal\[False\]|def evaluate_js' \
packages/notte-sdk/src/notte_sdk/endpoints/sessions.py \
packages/notte-browser/src/notte_browser/session.py
sed -n '21,25p' typing_cases/evaluate_js_overloads.pyRepository: nottelabs/notte Length of output: 20617 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- typing case ---'
cat -n typing_cases/evaluate_js_overloads.py
printf '%s\n' '--- evaluate_js overloads and implementation ---'
sed -n '1588,1620p' packages/notte-sdk/src/notte_sdk/endpoints/sessions.py
sed -n '1038,1088p' packages/notte-browser/src/notte_browser/session.py
printf '%s\n' '--- typing-check configuration and references ---'
rg -n -A8 -B4 'evaluate_js_overloads|pyright|mypy|computed_bool|argument expansion' \
pyproject.toml setup.cfg tox.ini .github typing_cases packages 2>/dev/null | head -220Repository: nottelabs/notte Length of output: 22139 🌐 Web query:
💡 Result: In basedpyright (and its upstream, pyright), overload resolution does not automatically expand the Citations:
🌐 Web query:
💡 Result: In Python type checking, "argument type expansion" is a mechanism used during overload resolution to handle types that represent a finite set of possibilities [1][2]. Specifically, the type bool is expanded into the union Literal[True] | Literal[False] during this process [1][2]. This expansion is critical when using Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- checker declarations and lock entries ---'
rg -n -A3 -B3 '"(basedpyright|ty)|basedpyright|ty ' \
pyproject.toml uv.lock Makefile justfile .github 2>/dev/null | head -180
printf '%s\n' '--- typing-case check commands ---'
rg -n -A8 -B5 'typing_cases|basedpyright|ty check|reveal_type' \
pyproject.toml Makefile justfile .github scripts 2>/dev/null | head -220Repository: nottelabs/notte Length of output: 10747 🌐 Web query:
💡 Result: In Python's type system, Literal types and the boolean type interact through specific rules regarding type narrowing and overloads. The boolean type bool is a subclass of int, and for type checking purposes, it is essentially a union of Literal[True] and Literal[False] [1]. When using Citations: 🌐 Web query:
💡 Result: In Python type checking, argument type expansion is a mechanism used during overload resolution to handle cases where an argument type (like Citations:
🌐 Web query:
💡 Result: The inquiry likely refers to ongoing discussions regarding type-checking behavior in the Astral Citations:
🌐 Web query:
💡 Result: In Python's typing system, the relationship between Citations:
🌐 Web query:
💡 Result: In Python type checking, Python type checkers such as Pyright and Mypy do not automatically expand the type bool into the union of Literal[True] and Literal[False] when performing overload resolution [1]. If you define a function with overloads specifically for Literal[True] and Literal[False], passing a general bool value to that function will typically result in a type checker error because the bool type is not considered assignable to the specific literal types [1]. To support a boolean value, you must explicitly include a separate overload that accepts the bool type: Citations: 🌐 Web query:
💡 Result: Issue Citations: Add a plain-
🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| 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