v0.5.1 add listener on 'page' in context, add screenshots, fix Laminar.flush

[dinmukhamedm]

Important

Add 'page' event listener in Playwright context, enhance agent functionality with screenshot support, and update version to 0.5.1.

• Playwright Integration:
  • Add listener for 'page' events in playwright_otel.py to handle navigation in both sync and async contexts.
  • Introduce _wrap_new_context_sync and _wrap_new_context_async to wrap new context creation.
• Agent Enhancements:
  • Add return_screenshots parameter to run() methods in agent.py for both async and sync clients.
  • Update RunAgentRequest in types.py to include return_screenshots.
• Miscellaneous:
  • Change lmnrRrwebEventsBatch from list to set in pw_utils.py to prevent duplicate events.
  • Fix typo in README.md (else if to elif).
  • Update version to 0.5.1 in pyproject.toml and version.py.
  • Add flush() method to TracerManager in __init__.py and tracing.py to return a boolean.

^{This description was created by for 404ad3c. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to b532f83 in 2 minutes and 48 seconds

More details

• Looked at 527 lines of code in 12 files
• Skipped 0 files when reviewing.
• Skipped posting 20 drafted comments based on config settings.

1. README.md:231

• Draft comment:
  Good fix replacing 'else if' with 'elif'.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

2. pyproject.toml:9

• Draft comment:
  Version bump to 0.5.1 looks correct.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

3. src/lmnr/openllmetry_sdk/__init__.py:70

• Draft comment:
  Returning a bool from flush() enforces a clear contract; ensure __tracer_wrapper.flush() always returns a bool.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

4. src/lmnr/sdk/browser/playwright_otel.py:137

• Draft comment:
  Listeners on 'page' event added on context in sync and async wrappers; consider including error handling in lambdas for robustness.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  The suggestion has merit since error handling is important for event listeners to prevent crashes. However, the handle_navigation_sync/async functions may already have their own error handling. Without seeing those functions, we can't be certain that additional error handling is needed. The comment is somewhat speculative without evidence that errors are actually occurring or being improperly handled.
  I don't have visibility into the handle_navigation functions to know if error handling is already implemented there. The comment may be suggesting redundant error handling.
  While error handling is generally good practice, without seeing the implementation of handle_navigation functions, we can't be confident that additional error handling would improve anything.
  The comment should be removed since it makes a speculative suggestion without clear evidence that there's an actual problem to solve.

5. src/lmnr/sdk/browser/pw_utils.py:39

• Draft comment:
  Switching from an array to a Set for lmnrRrwebEventsBatch may affect ordering; ensure deduplication is intended.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

6. src/lmnr/sdk/client/asynchronous/resources/agent.py:185

• Draft comment:
  Remove or convert debug print(line) in streaming non-streaming handler if not intended for production.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. src/lmnr/sdk/client/synchronous/resources/agent.py:140

• Draft comment:
  Ensure consistent handling of the 'return_screenshots' parameter across sync and async agent implementations.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

8. src/lmnr/sdk/evaluations.py:36

• Draft comment:
  Changed evaluation URL logic: uses regex to trim trailing slash; appears correct but verify port assignment for localhost matches intended behavior.
• Reason this comment was not posted:
  Confidence changes required: 20% <= threshold 50%
  None

9. src/lmnr/version.py:6

• Draft comment:
  Version string updated to '0.5.1'; ensure this is coherent with changelog and tagging.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

10. src/lmnr/openllmetry_sdk/__init__.py:70

• Draft comment:
  flush() now returns a bool. Ensure all consumers expect and handle its return value.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to ensure that all consumers of the flush() function handle its new return value. This is a request for confirmation and testing, which violates the rules.

11. src/lmnr/sdk/browser/pw_utils.py:39

• Draft comment:
  Switching lmnrRrwebEventsBatch from an array to a Set may remove duplicates and lose ordering. Verify that this behavior is intentional.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The comment raises concerns about duplicates and ordering, but examining the code shows: 1) Duplicate removal may actually be beneficial for events 2) When ordering matters (batch size limiting), the code properly handles it with Array.from().slice() 3) The comment is asking for verification rather than pointing out a clear issue 4) The implementation seems intentional and well thought out
  I could be wrong about whether event ordering is important in other contexts I can't see. There might be downstream consumers that expect strict event ordering.
  The code preserves ordering when needed through explicit conversions, and if strict ordering was critical, the original implementation would likely have enforced it more rigorously.
  Delete this comment as it asks for verification rather than pointing out a clear issue, and the implementation appears intentionally designed to handle ordering appropriately where needed.

12. src/lmnr/sdk/browser/playwright_otel.py:144

• Draft comment:
  Inline lambda for page event listener; consider refactoring to a named helper for clarity and easier debugging.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  The lambda is quite simple and just calls handle_navigation_sync with some closed-over variables. The async version uses a named function because it needs the async keyword. The sync version is simple enough that a named function wouldn't meaningfully improve debugging - you can still set breakpoints in lambdas. The code is consistent in using lambdas for sync and named functions for async throughout.
  The comment has a point that named functions can be easier to debug in some cases. And consistency between sync/async versions could be valuable.
  However, the lambda is very simple, just forwarding parameters. A named function would add more complexity than clarity here. The async/sync difference is justified by language constraints.
  The comment should be deleted as the suggested change would not meaningfully improve the code. The current approach is clear and debuggable.

13. src/lmnr/sdk/laminar.py:303

• Draft comment:
  Avoid using 'input' as a parameter name since it shadows the built-in function. Consider renaming it.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

14. README.md:38

• Draft comment:
  Typo: In the sentence 'You can also skip passing the project_api_key, in which case it will be looked in the environment (or local .env file) by the key LMNR_PROJECT_API_KEY.', consider changing 'looked' to 'looked up' to improve clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

15. src/lmnr/openllmetry_sdk/tracing/tracing.py:1

• Draft comment:
  The module/folder name 'openllmetry_sdk' appears to have an extra 'l' (i.e. 'openllmetry' instead of 'opentelemetry'). If this is not intentional, please correct the spelling to avoid confusion.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

16. src/lmnr/sdk/evaluations.py:44

• Draft comment:
  Consider rephrasing the comment on line 44 that currently reads 'We best effort assume that the frontend is running on port 5667'. For example, you could change it to 'We assume on a best-effort basis that the frontend is running on port 5667' for improved clarity.
• Reason this comment was not posted:
  Comment was on unchanged code.

17. src/lmnr/sdk/evaluations.py:113

• Draft comment:
  The docstring starting on line 113 mentions 'Initializes an instance of the Evaluations class', but the class is actually named 'Evaluation'. Update this to accurately reflect the class name.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

18. src/lmnr/sdk/laminar.py:683

• Draft comment:
  Typographical error: In the set_metadata docstring, "Willl be sent as attributes" should be "Will be sent as attributes" (remove the extra 'l').
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

19. src/lmnr/sdk/laminar.py:240

• Draft comment:
  Typographical issue: In the example code snippet of start_as_current_span, there's a stray backtick (`) at the end of the code block. It should be removed so that the code snippet is properly formatted.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

20. src/lmnr/sdk/laminar.py:700

• Draft comment:
  Typographical error: In the clear_session docstring, the phrase 'from the context' contains a double space between 'from' and 'the'. Please reduce it to a single space.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_Tw8io2yEkmAVQ7ir

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

Found a debug print (print(line)). Consider using a proper logging mechanism instead.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 404ad3c in 52 seconds

More details

• Looked at 12 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. src/lmnr/sdk/client/asynchronous/resources/agent.py:188

• Draft comment:
  Remove debug print; use proper logging if needed.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. src/lmnr/sdk/client/asynchronous/resources/agent.py:188

• Draft comment:
  Removed debug print. Use structured logging if you need to debug in production.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

Workflow ID: wflow_gAhw1aNg6rPsHY3m

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.