Skip to content

chore(deps): Address dependabot and pip-audit findings#111

Merged
m1so merged 3 commits into
mainfrom
mb/update-dependencies-20260624
Jun 25, 2026
Merged

chore(deps): Address dependabot and pip-audit findings#111
m1so merged 3 commits into
mainfrom
mb/update-dependencies-20260624

Conversation

@m1so

@m1so m1so commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved chart serialization for Polars and pandas DataFrames by sanitizing UUID-containing columns (including correct null handling) to keep chart output JSON serializable.
    • Added dedicated sanitization for polars-eager to ensure consistent chart compilation.
  • Tests
    • Added/expanded unit tests for Polars and pandas sanitization, plus end-to-end chart JSON serialization checks.
  • Chores
    • Updated dependency version constraints and bumped the pinned server notebook package to a newer release.
    • Refreshed the CI coverage uploader action version.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 94c435e8-895c-4974-a49a-b831936be9cd

📥 Commits

Reviewing files that changed from the base of the PR and between 6cd4552 and 6442003.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

📝 Walkthrough

Walkthrough

pyproject.toml updates several dependency bounds, including pyarrow, bleach, cryptography, tornado, ujson, and jupyter-server. Chart sanitization now converts UUID values to strings in pandas dataframes, adds Polars dataframe sanitization for object columns, and routes polars-eager charts through that sanitizer. Tests cover UUID handling, null preservation, Polars support, and non-mutation. The CI workflow also updates the Codecov action pin.

Suggested reviewers

  • tkislan

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Updates Docs ⚠️ Warning Visible docs don't cover the new chart/polars UUID sanitization; only generic README/config docs changed. Add feature docs in the OSS repo and update the deepnote-internal roadmap page too; I can’t verify that private repo here.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the dependency and security-fix focus of the PR, though it does not mention the chart sanitization changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

📦 Python package built successfully!

  • Version: 2.3.1.dev5+84fe12d
  • Wheel: deepnote_toolkit-2.3.1.dev5+84fe12d-py3-none-any.whl
  • Install:
    pip install "deepnote-toolkit @ https://deepnote-staging-runtime-artifactory.s3.amazonaws.com/deepnote-toolkit-packages/2.3.1.dev5%2B84fe12d/deepnote_toolkit-2.3.1.dev5%2B84fe12d-py3-none-any.whl"

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 24, 2026
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.18%. Comparing base (3967dd5) to head (6442003).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
deepnote_toolkit/chart/deepnote_chart.py 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
+ Coverage   74.09%   74.18%   +0.09%     
==========================================
  Files          95       95              
  Lines        5678     5698      +20     
  Branches      843      848       +5     
==========================================
+ Hits         4207     4227      +20     
  Misses       1195     1195              
  Partials      276      276              
Flag Coverage Δ
combined 74.18% <95.45%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@deepnote-bot

deepnote-bot commented Jun 24, 2026

Copy link
Copy Markdown

🚀 Review App Deployment Started

📝 Description 🌐 Link / Info
🌍 Review application ra-111
🔑 Sign-in URL Click to sign-in
Envoy review application ra-111 Envoy
Envoy sign-in URL Click to sign-in
📊 Application logs View logs
🔄 Actions Click to redeploy
🚀 ArgoCD deployment View deployment
Last deployed 2026-06-24 16:27:52 (UTC)
📜 Deployed commit f19244e49174eea553d25c4dfd3858926ef83fed
🛠️ Toolkit version 84fe12d

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
deepnote_toolkit/chart/utils.py (1)

12-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add explicit return annotations to the changed sanitizer helpers.

sanitize_dataframe_for_chart and _convert_uuid_columns_to_string are changed here, but they still omit return types; sanitize_dataframe_for_chart also lacks a docstring.

Suggested cleanup
-def sanitize_dataframe_for_chart(pd_df: pd.DataFrame):
+def sanitize_dataframe_for_chart(pd_df: pd.DataFrame) -> pd.DataFrame:
+    """Return a chart-safe copy of a pandas DataFrame."""
     sanitized_dataframe = pd_df.copy()
@@
-def _convert_uuid_columns_to_string(pd_df: pd.DataFrame):
+def _convert_uuid_columns_to_string(pd_df: pd.DataFrame) -> None:

As per coding guidelines, "Use type hints consistently", "Use docstrings for all functions/classes", and "Use explicit type hints for function parameters and return values".

Also applies to: 57-70

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deepnote_toolkit/chart/utils.py` around lines 12 - 20, The sanitizer helpers
in chart/utils.py still rely on implicit returns, so update
sanitize_dataframe_for_chart and _convert_uuid_columns_to_string to include
explicit return type annotations consistent with the existing type-hint style.
Also add a docstring for sanitize_dataframe_for_chart, and make sure the related
helper functions in the same block follow the same parameter/return typing
conventions so the sanitizer API is documented and typed consistently.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deepnote_toolkit/chart/utils.py`:
- Around line 75-79: The UUID sanitization in the column-processing logic only
checks the first non-null value, so it can miss columns where a later entry is a
uuid.UUID; update the detection in the utility that iterates columns in
deepnote_toolkit/chart/utils.py to scan the full non-null slice (for example, by
checking whether any non-null value is a uuid.UUID) before applying the existing
string conversion map. Add or update a regression test covering a column with a
string first and a UUID later to ensure conversion no longer depends on row
order.

---

Nitpick comments:
In `@deepnote_toolkit/chart/utils.py`:
- Around line 12-20: The sanitizer helpers in chart/utils.py still rely on
implicit returns, so update sanitize_dataframe_for_chart and
_convert_uuid_columns_to_string to include explicit return type annotations
consistent with the existing type-hint style. Also add a docstring for
sanitize_dataframe_for_chart, and make sure the related helper functions in the
same block follow the same parameter/return typing conventions so the sanitizer
API is documented and typed consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0e98fccf-9152-4497-8ee7-796914ed2d81

📥 Commits

Reviewing files that changed from the base of the PR and between f442fd9 and 6cd4552.

📒 Files selected for processing (3)
  • deepnote_toolkit/chart/deepnote_chart.py
  • deepnote_toolkit/chart/utils.py
  • tests/unit/test_chart.py

Comment thread deepnote_toolkit/chart/utils.py
@m1so m1so marked this pull request as ready for review June 24, 2026 16:21
@m1so m1so requested a review from a team as a code owner June 24, 2026 16:21
@m1so m1so requested review from mfranczel and tkislan June 24, 2026 16:21
@m1so m1so merged commit 4c34211 into main Jun 25, 2026
33 checks passed
@m1so m1so deleted the mb/update-dependencies-20260624 branch June 25, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants