feat: Convert Deepnote block types to .ipynb format #2
Conversation
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
Signed-off-by: Andy Jakubowski <[email protected]>
Contributor
📝 WalkthroughWalkthroughAdds three notebooks demonstrating Deepnote block types (two .deepnote and one .ipynb) that build a sample pandas DataFrame, compute salary_max/salary_min, run SQL against df, render a Vega-Lite “Salary by employee” chart, and expose many input widgets and buttons. Introduces a Module function _deepnote_run_notebook_function(export_table_states_json). Refactors jupyterlab_deepnote to centralize block→cell translation via a new jupyterlab_deepnote/convert_format.py module and replaces per-block cell construction in contents.py with convert_blocks_to_cells. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Frontend as JupyterLab / UI
participant Contents as jupyterlab_deepnote/contents.py
participant Converter as jupyterlab_deepnote/convert_format.py
participant nbformat as nbformat Cells
participant Kernel as Python Kernel
participant _dntk as Deepnote SDK (_dntk)
participant ModuleFn as _deepnote_run_notebook_function
User->>Frontend: Open notebook (all-block-types)
Frontend->>Contents: Request notebook cells
Contents->>Converter: convert_blocks_to_cells(nb_blocks)
Converter-->>Contents: list of nbformat cells
Contents->>Frontend: Render notebook
User->>Frontend: Run notebook / run cells
Frontend->>Kernel: Execute generated nbformat cells
Kernel->>Kernel: run Python setup (df, salary_max, salary_min)
Kernel->>_dntk: execute_sql(...) %% SQL blocks call
_dntk-->>Kernel: query result (df)
Kernel->>Kernel: render Vega-Lite spec -> visualization
Kernel->>Kernel: execute big-number rendering (Jinja2 -> JSON)
User->>Frontend: Interact with inputs / click buttons
Frontend->>Kernel: input variable updates / button actions
Kernel->>ModuleFn: _deepnote_run_notebook_function(export_table_states_json)
ModuleFn->>_dntk: run_notebook_function(...)
_dntk-->>ModuleFn: function result
ModuleFn-->>Kernel: return / side-effects
Kernel-->>Frontend: Updated outputs / state
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.12.2)jupyterlab_deepnote/contents.py�[1;31mruff failed�[0m jupyterlab_deepnote/convert_format.py�[1;31mruff failed�[0m Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
all-block-types-no-outputs.deepnote(1 hunks)all-block-types-no-outputs.ipynb(1 hunks)all-block-types-with-outputs.deepnote(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/** : Deepnote integration code resides under src/notebooks/deepnote/
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check_release
- GitHub Check: build
🔇 Additional comments (3)
all-block-types-no-outputs.deepnote (1)
8-570: Block coverage looks comprehensive.Good end-to-end representation of Deepnote block types for conversion tests.
all-block-types-with-outputs.deepnote (1)
1-1624: Outputs-included variant LGTM.Keeping a with-outputs artifact for demos is fine. Consider adding a note in README explaining the two variants’ intent.
all-block-types-no-outputs.ipynb (1)
1-796: Make the .ipynb valid — notebook missing: notebooks/all-block-types-no-outputs.ipynb.Cannot validate (file absent). Apply the original fixes and re-run validation or attach the notebook for me to re-check.
- Move non‑schema cell keys into cell.metadata (e.g., block_group → metadata.block_group).
- Remove non‑nbformat/ephemeral cell fields: outputs_reference, content_dependencies, source_hash, execution_start, execution_millis, and any deepnote_* keys.
- Clear cell outputs and set execution_count = null for all code cells.
Comment on lines
+575
to
+738
| settings: | ||
| requirements: | ||
| - Please | ||
| - don't | ||
| - install | ||
| - "`jupyter`" | ||
| - or | ||
| - "`jedi`" | ||
| - packages, | ||
| - they | ||
| - would | ||
| - break | ||
| - your | ||
| - Deepnote | ||
| - environment. | ||
| - Also, | ||
| - no | ||
| - need | ||
| - to | ||
| - put | ||
| - "`!pip" | ||
| - install`s | ||
| - here, | ||
| - we | ||
| - already | ||
| - save | ||
| - those | ||
| - automatically! | ||
| - then | ||
| - ./requirements.txt | ||
| - fi | ||
| - unidecode | ||
| - cairosvg | ||
| - wordcloud | ||
| - pandas | ||
| - requests | ||
| - nltk | ||
| - vaderSentiment | ||
| - matplotlib | ||
| - seaborn | ||
| - transformers | ||
| - '"pandas>=2.0"' | ||
| - ipython | ||
| - requirements.txt | ||
| - python | ||
| - pip | ||
| - wheel | ||
| - plotly | ||
| - Finally, | ||
| - will | ||
| - sort | ||
| - the | ||
| - dataframe | ||
| - based | ||
| - on | ||
| - percentage | ||
| - change | ||
| - in | ||
| - descending | ||
| - order | ||
| - and | ||
| - select | ||
| - top | ||
| - "5." | ||
| - If | ||
| - "`yfinance`" | ||
| - is | ||
| - not | ||
| - installed | ||
| - Python | ||
| - environment, | ||
| - it | ||
| - using | ||
| - "`!yfinance`." | ||
| - pytrends | ||
| - pandas==1.1.0 | ||
| - /work/testmc-0.1.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl | ||
| - else | ||
| - Plotly | ||
| - a | ||
| - powerful | ||
| - library | ||
| - for | ||
| - creating | ||
| - interactive | ||
| - visualizations. | ||
| - In | ||
| - this | ||
| - notebook, | ||
| - used | ||
| - generate | ||
| - bar | ||
| - charts, | ||
| - line | ||
| - histograms. | ||
| - The | ||
| - "`!plotly`" | ||
| - command | ||
| - ensures | ||
| - that | ||
| - available | ||
| - use. | ||
| - note | ||
| - symbols | ||
| - provided | ||
| - are | ||
| - just | ||
| - reference. | ||
| - You | ||
| - can | ||
| - replace | ||
| - them | ||
| - according | ||
| - interests. | ||
| - code | ||
| - snippet | ||
| - assumes | ||
| - you | ||
| - have | ||
| - (`pd`) | ||
| - installed. | ||
| - not, | ||
| - "`!pandas`." | ||
| - jinja2 | ||
| - Jinja2==3.0.0 | ||
| - altair | ||
| - vega_datasets | ||
| - sentry-sdk | ||
| - responses | ||
| - parameterized | ||
| - python-socketio | ||
| - eventlet | ||
| - vllm | ||
| - . | ||
| - running | ||
| - after | ||
| - executing | ||
| - "!fastapi" | ||
| - '"sideloading' | ||
| - packages" | ||
| - notebook | ||
| - ipywidgets | ||
| - jupyterlab | ||
| - psutil | ||
| - Absolutely, | ||
| - here | ||
| - few | ||
| - examples | ||
| - of | ||
| - how | ||
| - Notion | ||
| - API | ||
| - be | ||
| - Python. | ||
| - It | ||
| - uses | ||
| - "`notion`" | ||
| - package. | ||
| - installed, | ||
| - via | ||
| - "`!notion`." | ||
| - "%fastapi" | ||
| - "%-q" | ||
| version: 1.0.0 |
Contributor
There was a problem hiding this comment.
Requirements list will break environments; replace with a minimal, correct set.
The list mixes prose, shell snippets, paths, conflicting pins (pandas==1.1.0 and "pandas>=2.0"), and heavy deps. This will cause failed installs and slow cold starts.
Apply:
- settings:
- requirements:
- - Please
- - don't
- - install
- ...
- - "%-q"
+ settings:
+ requirements:
+ - pandas>=2.0
+ - numpy>=1.24
+ - python-dateutil>=2.8.2
+ - jinja2>=3.1If more are truly needed for examples, add them explicitly with sane, modern pins.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| settings: | |
| requirements: | |
| - Please | |
| - don't | |
| - install | |
| - "`jupyter`" | |
| - or | |
| - "`jedi`" | |
| - packages, | |
| - they | |
| - would | |
| - break | |
| - your | |
| - Deepnote | |
| - environment. | |
| - Also, | |
| - no | |
| - need | |
| - to | |
| - put | |
| - "`!pip" | |
| - install`s | |
| - here, | |
| - we | |
| - already | |
| - save | |
| - those | |
| - automatically! | |
| - then | |
| - ./requirements.txt | |
| - fi | |
| - unidecode | |
| - cairosvg | |
| - wordcloud | |
| - pandas | |
| - requests | |
| - nltk | |
| - vaderSentiment | |
| - matplotlib | |
| - seaborn | |
| - transformers | |
| - '"pandas>=2.0"' | |
| - ipython | |
| - requirements.txt | |
| - python | |
| - pip | |
| - wheel | |
| - plotly | |
| - Finally, | |
| - will | |
| - sort | |
| - the | |
| - dataframe | |
| - based | |
| - on | |
| - percentage | |
| - change | |
| - in | |
| - descending | |
| - order | |
| - and | |
| - select | |
| - top | |
| - "5." | |
| - If | |
| - "`yfinance`" | |
| - is | |
| - not | |
| - installed | |
| - Python | |
| - environment, | |
| - it | |
| - using | |
| - "`!yfinance`." | |
| - pytrends | |
| - pandas==1.1.0 | |
| - /work/testmc-0.1.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl | |
| - else | |
| - Plotly | |
| - a | |
| - powerful | |
| - library | |
| - for | |
| - creating | |
| - interactive | |
| - visualizations. | |
| - In | |
| - this | |
| - notebook, | |
| - used | |
| - generate | |
| - bar | |
| - charts, | |
| - line | |
| - histograms. | |
| - The | |
| - "`!plotly`" | |
| - command | |
| - ensures | |
| - that | |
| - available | |
| - use. | |
| - note | |
| - symbols | |
| - provided | |
| - are | |
| - just | |
| - reference. | |
| - You | |
| - can | |
| - replace | |
| - them | |
| - according | |
| - interests. | |
| - code | |
| - snippet | |
| - assumes | |
| - you | |
| - have | |
| - (`pd`) | |
| - installed. | |
| - not, | |
| - "`!pandas`." | |
| - jinja2 | |
| - Jinja2==3.0.0 | |
| - altair | |
| - vega_datasets | |
| - sentry-sdk | |
| - responses | |
| - parameterized | |
| - python-socketio | |
| - eventlet | |
| - vllm | |
| - . | |
| - running | |
| - after | |
| - executing | |
| - "!fastapi" | |
| - '"sideloading' | |
| - packages" | |
| - notebook | |
| - ipywidgets | |
| - jupyterlab | |
| - psutil | |
| - Absolutely, | |
| - here | |
| - few | |
| - examples | |
| - of | |
| - how | |
| - Notion | |
| - API | |
| - be | |
| - Python. | |
| - It | |
| - uses | |
| - "`notion`" | |
| - package. | |
| - installed, | |
| - via | |
| - "`!notion`." | |
| - "%fastapi" | |
| - "%-q" | |
| version: 1.0.0 | |
| settings: | |
| requirements: | |
| - pandas>=2.0 | |
| - numpy>=1.24 | |
| - python-dateutil>=2.8.2 | |
| - jinja2>=3.1 | |
| version: 1.0.0 |
🤖 Prompt for AI Agents
In all-block-types-no-outputs.deepnote around lines 575 to 738, the requirements
block contains prose, shell snippets, duplicate and conflicting pins (e.g.
pandas==1.1.0 vs "pandas>=2.0"), file paths and other noise that will break
environment installation; replace this blob with a minimal, valid YAML list of
only the needed Python packages (one package per list item), remove any
shell/command text, paths and comments, deduplicate packages and resolve version
conflicts using modern, compatible pins (e.g. choose "pandas>=2.0" or a specific
stable pin), and only add optional heavy deps (transformers, vllm, yfinance,
etc.) explicitly if examples require them.
Comment on lines
+23
to
+29
| "source": "import pandas as pd\nimport numpy as np\n\n# Create sample data\ndata = {\n 'Name': ['John', 'Emma', 'Michael', 'Sarah', 'David'],\n 'Age': [28, 34, 42, 25, 31],\n 'Salary': [65000, 78000, 95000, 62000, 70000],\n 'Department': ['IT', 'HR', 'Finance', 'Marketing', 'IT'],\n 'Experience': [3, 7, 12, 2, 5]\n}\n\n# Create DataFrame\ndf = pd.DataFrame(data)\n\n# Create salary_max variable\nsalary_max = df['Salary'].max()\n\n# Create salary_min variable\nsalary_min = df['Salary'].min()\n\n# Display the DataFrame\ndf", | ||
| "block_group": "2a432c3f8c6a4602a4299b5094cd1b35", | ||
| "execution_count": 1, | ||
| "outputs": [], | ||
| "outputs_reference": null, | ||
| "content_dependencies": null | ||
| }, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Drop unused NumPy import.
np isn’t used.
-import numpy as np📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "source": "import pandas as pd\nimport numpy as np\n\n# Create sample data\ndata = {\n 'Name': ['John', 'Emma', 'Michael', 'Sarah', 'David'],\n 'Age': [28, 34, 42, 25, 31],\n 'Salary': [65000, 78000, 95000, 62000, 70000],\n 'Department': ['IT', 'HR', 'Finance', 'Marketing', 'IT'],\n 'Experience': [3, 7, 12, 2, 5]\n}\n\n# Create DataFrame\ndf = pd.DataFrame(data)\n\n# Create salary_max variable\nsalary_max = df['Salary'].max()\n\n# Create salary_min variable\nsalary_min = df['Salary'].min()\n\n# Display the DataFrame\ndf", | |
| "block_group": "2a432c3f8c6a4602a4299b5094cd1b35", | |
| "execution_count": 1, | |
| "outputs": [], | |
| "outputs_reference": null, | |
| "content_dependencies": null | |
| }, | |
| import pandas as pd | |
| # Create sample data | |
| data = { | |
| 'Name': ['John', 'Emma', 'Michael', 'Sarah', 'David'], | |
| 'Age': [28, 34, 42, 25, 31], | |
| 'Salary': [65000, 78000, 95000, 62000, 70000], | |
| 'Department': ['IT', 'HR', 'Finance', 'Marketing', 'IT'], | |
| 'Experience': [3, 7, 12, 2, 5] | |
| } | |
| # Create DataFrame | |
| df = pd.DataFrame(data) | |
| # Create salary_max variable | |
| salary_max = df['Salary'].max() | |
| # Create salary_min variable | |
| salary_min = df['Salary'].min() | |
| # Display the DataFrame | |
| df |
🤖 Prompt for AI Agents
In all-block-types-no-outputs.ipynb around lines 23 to 29, the file imports
NumPy with "import numpy as np" but never uses it; remove the unused import line
(or if NumPy is intended, use it where needed) so the notebook has only
necessary imports and no unused dependencies.
Comment on lines
+261
to
+279
| "cell_type": "code", | ||
| "metadata": { | ||
| "source_hash": "a3523be3", | ||
| "execution_start": 1758017229128, | ||
| "execution_millis": 371, | ||
| "sql_integration_id": "deepnote-dataframe-sql", | ||
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | ||
| "deepnote_variable_name": "df", | ||
| "cell_id": "3cbce657e51247f2b01b730661260cb3", | ||
| "deepnote_cell_type": "sql", | ||
| "deepnote_sql_source": "SELECT * FROM df;" | ||
| }, | ||
| "source": "df = _dntk.execute_sql(\n 'SELECT * FROM df;',\n 'SQL_DEEPNOTE_DATAFRAME_SQL',\n audit_sql_comment='',\n sql_cache_mode='cache_disabled',\n return_variable_type='dataframe'\n)\ndf", | ||
| "block_group": "19c9f74d74b741f2b76c2bbb19b9e072", | ||
| "execution_count": 2, | ||
| "outputs": [], | ||
| "outputs_reference": null, | ||
| "content_dependencies": null | ||
| }, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Guard Deepnote-only APIs for portability.
_sql and chart cells assume _dntk exists; in plain Jupyter this will NameError. Add a small guard or a helpful error.
-df = _dntk.execute_sql(
+if '_dntk' not in globals():
+ raise RuntimeError("This cell requires the Deepnote runtime (_dntk).")
+df = _dntk.execute_sql(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "cell_type": "code", | |
| "metadata": { | |
| "source_hash": "a3523be3", | |
| "execution_start": 1758017229128, | |
| "execution_millis": 371, | |
| "sql_integration_id": "deepnote-dataframe-sql", | |
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | |
| "deepnote_variable_name": "df", | |
| "cell_id": "3cbce657e51247f2b01b730661260cb3", | |
| "deepnote_cell_type": "sql", | |
| "deepnote_sql_source": "SELECT * FROM df;" | |
| }, | |
| "source": "df = _dntk.execute_sql(\n 'SELECT * FROM df;',\n 'SQL_DEEPNOTE_DATAFRAME_SQL',\n audit_sql_comment='',\n sql_cache_mode='cache_disabled',\n return_variable_type='dataframe'\n)\ndf", | |
| "block_group": "19c9f74d74b741f2b76c2bbb19b9e072", | |
| "execution_count": 2, | |
| "outputs": [], | |
| "outputs_reference": null, | |
| "content_dependencies": null | |
| }, | |
| if '_dntk' not in globals(): | |
| raise RuntimeError("This cell requires the Deepnote runtime (_dntk).") | |
| df = _dntk.execute_sql( | |
| 'SELECT * FROM df;', | |
| 'SQL_DEEPNOTE_DATAFRAME_SQL', | |
| audit_sql_comment='', | |
| sql_cache_mode='cache_disabled', | |
| return_variable_type='dataframe' | |
| ) | |
| df |
🤖 Prompt for AI Agents
In all-block-types-no-outputs.ipynb around lines 261 to 279, the SQL cell calls
Deepnote runtime API _dntk directly which will raise NameError in plain Jupyter;
add a small guard that detects whether _dntk is defined and if not either (a)
raise a clear, actionable error telling the user this cell requires Deepnote and
how to adapt it, or (b) provide a fallback path that uses pandas.read_sql or
executes a no-op returning the existing df; implement the check at the top of
the cell and use the fallback or informative error so the notebook runs portably
outside Deepnote.
Comment on lines
+411
to
+433
| "cell_type": "code", | ||
| "metadata": { | ||
| "source_hash": "ea65eb57", | ||
| "execution_start": 1758017230260, | ||
| "execution_millis": 1, | ||
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | ||
| "deepnote_big_number_title": "Max salary", | ||
| "deepnote_big_number_value": "salary_max", | ||
| "deepnote_big_number_format": "currency", | ||
| "deepnote_big_number_comparison_type": "", | ||
| "deepnote_big_number_comparison_title": "Difference with min", | ||
| "deepnote_big_number_comparison_value": "salary_min", | ||
| "deepnote_big_number_comparison_format": "", | ||
| "deepnote_big_number_comparison_enabled": true, | ||
| "cell_id": "63bd93dd0c7b4940bb7e2f281ef527ea", | ||
| "deepnote_cell_type": "big-number" | ||
| }, | ||
| "source": "\ndef __deepnote_big_number__():\n import json\n import jinja2\n from jinja2 import meta\n\n def render_template(template):\n parsed_content = jinja2.Environment().parse(template)\n\n required_variables = meta.find_undeclared_variables(parsed_content)\n\n context = {\n variable_name: globals().get(variable_name)\n for variable_name in required_variables\n }\n\n result = jinja2.Environment().from_string(template).render(context)\n\n return result\n\n rendered_title = render_template(\"Max salary\")\n rendered_comparison_title = render_template(\"Difference with min\")\n\n return json.dumps({\n \"comparisonTitle\": rendered_comparison_title,\n \"comparisonValue\": f\"{salary_min}\",\n \"title\": rendered_title,\n \"value\": f\"{salary_max}\"\n })\n\n__deepnote_big_number__()\n", | ||
| "block_group": "69a7615d8fec4e018ad29df4f7c9c8a6", | ||
| "execution_count": 4, | ||
| "outputs": [], | ||
| "outputs_reference": null | ||
| }, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Big number comparison should be a difference, not the min.
Title says “Difference with min” but comparisonValue is salary_min.
- "comparisonValue": f"{salary_min}",
+ "comparisonValue": f"{salary_max - salary_min}",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "cell_type": "code", | |
| "metadata": { | |
| "source_hash": "ea65eb57", | |
| "execution_start": 1758017230260, | |
| "execution_millis": 1, | |
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | |
| "deepnote_big_number_title": "Max salary", | |
| "deepnote_big_number_value": "salary_max", | |
| "deepnote_big_number_format": "currency", | |
| "deepnote_big_number_comparison_type": "", | |
| "deepnote_big_number_comparison_title": "Difference with min", | |
| "deepnote_big_number_comparison_value": "salary_min", | |
| "deepnote_big_number_comparison_format": "", | |
| "deepnote_big_number_comparison_enabled": true, | |
| "cell_id": "63bd93dd0c7b4940bb7e2f281ef527ea", | |
| "deepnote_cell_type": "big-number" | |
| }, | |
| "source": "\ndef __deepnote_big_number__():\n import json\n import jinja2\n from jinja2 import meta\n\n def render_template(template):\n parsed_content = jinja2.Environment().parse(template)\n\n required_variables = meta.find_undeclared_variables(parsed_content)\n\n context = {\n variable_name: globals().get(variable_name)\n for variable_name in required_variables\n }\n\n result = jinja2.Environment().from_string(template).render(context)\n\n return result\n\n rendered_title = render_template(\"Max salary\")\n rendered_comparison_title = render_template(\"Difference with min\")\n\n return json.dumps({\n \"comparisonTitle\": rendered_comparison_title,\n \"comparisonValue\": f\"{salary_min}\",\n \"title\": rendered_title,\n \"value\": f\"{salary_max}\"\n })\n\n__deepnote_big_number__()\n", | |
| "block_group": "69a7615d8fec4e018ad29df4f7c9c8a6", | |
| "execution_count": 4, | |
| "outputs": [], | |
| "outputs_reference": null | |
| }, | |
| def __deepnote_big_number__(): | |
| import json | |
| import jinja2 | |
| from jinja2 import meta | |
| def render_template(template): | |
| parsed_content = jinja2.Environment().parse(template) | |
| required_variables = meta.find_undeclared_variables(parsed_content) | |
| context = { | |
| variable_name: globals().get(variable_name) | |
| for variable_name in required_variables | |
| } | |
| result = jinja2.Environment().from_string(template).render(context) | |
| return result | |
| rendered_title = render_template("Max salary") | |
| rendered_comparison_title = render_template("Difference with min") | |
| return json.dumps({ | |
| "comparisonTitle": rendered_comparison_title, | |
| "comparisonValue": f"{salary_max - salary_min}", | |
| "title": rendered_title, | |
| "value": f"{salary_max}" | |
| }) | |
| __deepnote_big_number__() |
🤖 Prompt for AI Agents
In all-block-types-no-outputs.ipynb around lines 411 to 433 the big-number cell
returns comparisonValue as salary_min while the comparisonTitle is "Difference
with min"; change comparisonValue to compute the difference (salary_max -
salary_min) and return that (e.g., formatted as a string or number consistent
with value), ensuring salary_max and salary_min are referenced correctly from
the context used to render the template.
Comment on lines
+545
to
+566
| "cell_type": "code", | ||
| "metadata": { | ||
| "source_hash": "903ebe60", | ||
| "execution_start": 1758017230460, | ||
| "execution_millis": 0, | ||
| "deepnote_input_label": "Slider input display name", | ||
| "deepnote_slider_step": 1, | ||
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | ||
| "deepnote_variable_name": "slider_input", | ||
| "deepnote_variable_value": "5", | ||
| "deepnote_slider_max_value": 10, | ||
| "deepnote_slider_min_value": 0, | ||
| "deepnote_variable_default_value": "7", | ||
| "cell_id": "f5bde14ac91b4c619d4721119f6dc097", | ||
| "deepnote_cell_type": "input-slider" | ||
| }, | ||
| "source": "slider_input = 5", | ||
| "block_group": "ce9d87717518498a8e258cd7ca4c530c", | ||
| "execution_count": 8, | ||
| "outputs": [], | ||
| "outputs_reference": null | ||
| }, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Type mismatch in slider metadata vs code.
deepnote_variable_value is a string "5", but the code sets slider_input = 5 (int). Align to avoid downstream surprises.
- "deepnote_variable_value": "5",
+ "deepnote_variable_value": 5,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "cell_type": "code", | |
| "metadata": { | |
| "source_hash": "903ebe60", | |
| "execution_start": 1758017230460, | |
| "execution_millis": 0, | |
| "deepnote_input_label": "Slider input display name", | |
| "deepnote_slider_step": 1, | |
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | |
| "deepnote_variable_name": "slider_input", | |
| "deepnote_variable_value": "5", | |
| "deepnote_slider_max_value": 10, | |
| "deepnote_slider_min_value": 0, | |
| "deepnote_variable_default_value": "7", | |
| "cell_id": "f5bde14ac91b4c619d4721119f6dc097", | |
| "deepnote_cell_type": "input-slider" | |
| }, | |
| "source": "slider_input = 5", | |
| "block_group": "ce9d87717518498a8e258cd7ca4c530c", | |
| "execution_count": 8, | |
| "outputs": [], | |
| "outputs_reference": null | |
| }, | |
| "cell_type": "code", | |
| "metadata": { | |
| "source_hash": "903ebe60", | |
| "execution_start": 1758017230460, | |
| "execution_millis": 0, | |
| "deepnote_input_label": "Slider input display name", | |
| "deepnote_slider_step": 1, | |
| "execution_context_id": "43d850b5-85c3-4bc8-b2d5-b50f2529e42c", | |
| "deepnote_variable_name": "slider_input", | |
| "deepnote_variable_value": 5, | |
| "deepnote_slider_max_value": 10, | |
| "deepnote_slider_min_value": 0, | |
| "deepnote_variable_default_value": "7", | |
| "cell_id": "f5bde14ac91b4c619d4721119f6dc097", | |
| "deepnote_cell_type": "input-slider" | |
| }, | |
| "source": "slider_input = 5", | |
| "block_group": "ce9d87717518498a8e258cd7ca4c530c", | |
| "execution_count": 8, | |
| "outputs": [], | |
| "outputs_reference": null | |
| }, |
🤖 Prompt for AI Agents
In all-block-types-no-outputs.ipynb around lines 545 to 566, the Deepnote
metadata fields deepnote_variable_value and deepnote_variable_default_value are
strings ("5", "7") while the code sets slider_input = 5 (int); update the
metadata to use numeric values (remove quotes so values are numbers) for
deepnote_variable_value and deepnote_variable_default_value to match the integer
slider_input, or alternatively change the code to parse the string to int (e.g.,
int(slider_input)) if you must keep metadata as strings—make one of these two
changes so types are consistent.
Comment on lines
+753
to
+773
| "cell_type": "code", | ||
| "metadata": { | ||
| "source_hash": "b63b0b2e", | ||
| "execution_start": 1758017517949, | ||
| "execution_millis": 145407, | ||
| "execution_context_id": "f5da58fe-0dad-4318-867a-0ab9925d7c28", | ||
| "function_notebook_id": "e960b15d27074d38a6b07ff87eb42c89", | ||
| "function_notebook_inputs": {}, | ||
| "last_function_run_started_at": 1758017655789, | ||
| "function_notebook_export_mappings": { | ||
| "Monthly_signups": { | ||
| "enabled": true, | ||
| "variable_name": "Monthly_signups" | ||
| } | ||
| }, | ||
| "last_executed_function_notebook_id": "e960b15d27074d38a6b07ff87eb42c89", | ||
| "cell_id": "8e613308b24f419eb550573eda2602ab", | ||
| "deepnote_cell_type": "notebook-function" | ||
| }, | ||
| "source": "def _deepnote_run_notebook_function(export_table_states_json):\n globals()['_deepnote_run_notebook_function'] = None\n\n return _dntk.run_notebook_function(\n scope=globals(),\n function_notebook_id='e960b15d27074d38a6b07ff87eb42c89',\n inputs={},\n export_mappings={'Monthly_signups': {'enabled': True,'variable_name': 'Monthly_signups'}},\n export_table_states_json=export_table_states_json,\n notebook_function_api_token='',\n parent_notebook_function_run_id=None,\n debug=False\n )", | ||
| "block_group": "15846815e8f743f582176f114aaf799f", |
Contributor
There was a problem hiding this comment.
Don’t self-nuke the notebook-function proxy.
The function sets its own global name to None, preventing subsequent calls in the same kernel.
Apply:
-def _deepnote_run_notebook_function(export_table_states_json):
- globals()['_deepnote_run_notebook_function'] = None
+def _deepnote_run_notebook_function(export_table_states_json):
+ """Proxy to Deepnote notebook-function. Idempotent; safe for repeated calls."""
+ if '_dntk' not in globals():
+ raise RuntimeError("Deepnote runtime (_dntk) not available")
return _dntk.run_notebook_function(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "cell_type": "code", | |
| "metadata": { | |
| "source_hash": "b63b0b2e", | |
| "execution_start": 1758017517949, | |
| "execution_millis": 145407, | |
| "execution_context_id": "f5da58fe-0dad-4318-867a-0ab9925d7c28", | |
| "function_notebook_id": "e960b15d27074d38a6b07ff87eb42c89", | |
| "function_notebook_inputs": {}, | |
| "last_function_run_started_at": 1758017655789, | |
| "function_notebook_export_mappings": { | |
| "Monthly_signups": { | |
| "enabled": true, | |
| "variable_name": "Monthly_signups" | |
| } | |
| }, | |
| "last_executed_function_notebook_id": "e960b15d27074d38a6b07ff87eb42c89", | |
| "cell_id": "8e613308b24f419eb550573eda2602ab", | |
| "deepnote_cell_type": "notebook-function" | |
| }, | |
| "source": "def _deepnote_run_notebook_function(export_table_states_json):\n globals()['_deepnote_run_notebook_function'] = None\n\n return _dntk.run_notebook_function(\n scope=globals(),\n function_notebook_id='e960b15d27074d38a6b07ff87eb42c89',\n inputs={},\n export_mappings={'Monthly_signups': {'enabled': True,'variable_name': 'Monthly_signups'}},\n export_table_states_json=export_table_states_json,\n notebook_function_api_token='',\n parent_notebook_function_run_id=None,\n debug=False\n )", | |
| "block_group": "15846815e8f743f582176f114aaf799f", | |
| def _deepnote_run_notebook_function(export_table_states_json): | |
| """Proxy to Deepnote notebook-function. Idempotent; safe for repeated calls.""" | |
| if '_dntk' not in globals(): | |
| raise RuntimeError("Deepnote runtime (_dntk) not available") | |
| return _dntk.run_notebook_function( | |
| scope=globals(), | |
| function_notebook_id='e960b15d27074d38a6b07ff87eb42c89', | |
| inputs={}, | |
| export_mappings={'Monthly_signups': {'enabled': True,'variable_name': 'Monthly_signups'}}, | |
| export_table_states_json=export_table_states_json, | |
| notebook_function_api_token='', | |
| parent_notebook_function_run_id=None, | |
| debug=False | |
| ) |
🤖 Prompt for AI Agents
In all-block-types-no-outputs.ipynb around lines 753 to 773, the
notebook-function proxy unconditionally sets
globals()['_deepnote_run_notebook_function'] = None which prevents subsequent
calls in the same kernel; remove that assignment so the function is not
self-nuking (i.e., delete the line that sets the global to None or otherwise
avoid overwriting the global name) so the proxy remains callable for later
invocations.
Comment on lines
+787
to
+794
| "nbformat": 4, | ||
| "nbformat_minor": 0, | ||
| "metadata": { | ||
| "deepnote_persisted_session": { | ||
| "createdAt": "2025-09-16T12:15:01.925Z" | ||
| }, | ||
| "deepnote_notebook_id": "686ad8158f6947f9b2796fb1add22277" | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Consider bumping nbformat_minor.
nbformat_minor is 0; most tools emit >=4/5. Not required, but normalizing reduces tool churn.
🤖 Prompt for AI Agents
In all-block-types-no-outputs.ipynb around lines 787 to 794, the notebook's
nbformat_minor is set to 0 which is uncommon and can cause tool churn; update
nbformat_minor to a more typical value (e.g., 2 or 5) in the notebook metadata,
ensure the notebook remains compatible with nbformat 4, and run a quick
validation (or open/save in your editor) to confirm the updated metadata is
correctly persisted.
Comment on lines
+1606
to
+1616
| function_notebook_id: e960b15d27074d38a6b07ff87eb42c89 | ||
| function_notebook_inputs: {} | ||
| last_function_run_started_at: 1758017230910 | ||
| function_notebook_export_mappings: | ||
| Monthly_signups: | ||
| enabled: true | ||
| variable_name: Monthly_signups | ||
| last_executed_function_notebook_id: 704967b0680a4c15a30ba70ff8730844 | ||
| outputs: [] | ||
| sortingKey: yyyyyyyyr | ||
| type: notebook-function |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Align last_executed_function_notebook_id with function_notebook_id.
The IDs differ; this can confuse reproducibility/debugging.
- last_executed_function_notebook_id: 704967b0680a4c15a30ba70ff8730844
+ last_executed_function_notebook_id: e960b15d27074d38a6b07ff87eb42c89📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function_notebook_id: e960b15d27074d38a6b07ff87eb42c89 | |
| function_notebook_inputs: {} | |
| last_function_run_started_at: 1758017230910 | |
| function_notebook_export_mappings: | |
| Monthly_signups: | |
| enabled: true | |
| variable_name: Monthly_signups | |
| last_executed_function_notebook_id: 704967b0680a4c15a30ba70ff8730844 | |
| outputs: [] | |
| sortingKey: yyyyyyyyr | |
| type: notebook-function | |
| function_notebook_id: e960b15d27074d38a6b07ff87eb42c89 | |
| function_notebook_inputs: {} | |
| last_function_run_started_at: 1758017230910 | |
| function_notebook_export_mappings: | |
| Monthly_signups: | |
| enabled: true | |
| variable_name: Monthly_signups | |
| last_executed_function_notebook_id: e960b15d27074d38a6b07ff87eb42c89 | |
| outputs: [] | |
| sortingKey: yyyyyyyyr | |
| type: notebook-function |
🤖 Prompt for AI Agents
In all-block-types-with-outputs.deepnote around lines 1606 to 1616, the value of
last_executed_function_notebook_id does not match function_notebook_id; update
last_executed_function_notebook_id to exactly match function_notebook_id
(e960b15d27074d38a6b07ff87eb42c89) so the two IDs are identical and maintain
consistency for reproducibility; ensure there are no trailing characters or
formatting changes and commit the modified file.
Signed-off-by: Andy Jakubowski <[email protected]>
andyjakubowski
changed the title
Artmann
approved these changes
Sep 17, 2025
| "big-number", | ||
| "code", | ||
| "sql", | ||
| # "notebook-function", # notebook-function is no longer a code type |
| "input-slider", | ||
| "visualization", | ||
| } | ||
| markdown_types = { |
Member
There was a problem hiding this comment.
Do we need to specify markdown types if that's the default?
| # Handle all text-cell-* types and separator, else return content unchanged | ||
| if btype == "text-cell-h1": | ||
| return "# " + content | ||
| elif btype == "text-cell-h2": |
Member
There was a problem hiding this comment.
Nitpick: we don't need an else when we are returning values.
Artmann
reviewed
Sep 17, 2025
|
|
||
|
|
||
| # Helper: Map Deepnote block type to Jupyter cell type | ||
| def map_block_type_to_jupyter(btype: str) -> str: |
Member
There was a problem hiding this comment.
Can we add a few tests for these functions? 🙏
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jupyterlab_deepnote/contents.py (1)
41-45: Embedding full notebooks in metadata may bloat models.Large .deepnote files could inflate
model["content"]. Consider size gating or a flag.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
jupyterlab_deepnote/contents.py(2 hunks)jupyterlab_deepnote/convert_format.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/** : Deepnote integration code resides under src/notebooks/deepnote/
🧬 Code graph analysis (2)
jupyterlab_deepnote/convert_format.py (1)
jupyterlab_deepnote/contents.py (1)
get(52-85)
jupyterlab_deepnote/contents.py (1)
jupyterlab_deepnote/convert_format.py (1)
convert_blocks_to_cells(400-404)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check_release
🔇 Additional comments (2)
jupyterlab_deepnote/contents.py (1)
31-40: Good delegation to converter.Centralizing block→cell logic simplifies contents and keeps YAML parsing clean.
jupyterlab_deepnote/convert_format.py (1)
117-147: Remove duplicate escape helper & ensure _dntk is imported
- Remove the inner escape_python_string and call the module-level escape_python_string instead.
- Keep this minimal diff:
def execute_sql_query( @@ - # Escape query and audit comment for Python string literal - def escape_python_string(value: str) -> str: - # Escape backslashes, single quotes, and newlines, then wrap in single quotes - escaped = value.replace("\\", "\\\\").replace("'", "\\'").replace("\n", "\\n") - return f"'{escaped}'" - - escaped_query = escape_python_string(query) + escaped_query = escape_python_string(query) escaped_audit_comment = "''" # No audit comment in this context
- Generated code calls _dntk.execute_sql but no import is provided — either add a safe import (e.g.
from jupyterlab_deepnote import _dntk # raises ImportError if unsupported) or document/runtime-inject_dntk.- Confirm the intended import path for _dntk.
Comment on lines
+21
to
+36
| "visualization", | ||
| } | ||
| markdown_types = { | ||
| "markdown", | ||
| "text-cell-h1", | ||
| "text-cell-h2", | ||
| "text-cell-h3", | ||
| "text-cell-p", | ||
| "text-cell-bullet", | ||
| "text-cell-todo", | ||
| "text-cell-callout", | ||
| "image", | ||
| "button", | ||
| "separator", | ||
| "notebook-function", # treat notebook-function as markdown | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Coverage gaps: visualization, image, input-file not explicitly mapped.
- map_block_type_to_jupyter marks visualization as code but map_block_source lacks a handler; if content is a spec dict, a raw dict in a code cell will execute as a bare literal (OKish for display but not rendering).
- image/button/input-file likely need markdown or Python renderers.
Proposed next steps:
- Add explicit handlers: visualization -> Python that renders (e.g., altair/vega-lite if spec present); image -> Markdown
; input-file -> path(s)/bytes handle or docs stub. - If out-of-scope, gate with a clear markdown notice like you did for notebook-function.
Also applies to: 254-383
Comment on lines
+71
to
+114
| def execute_big_number( | ||
| title_template: str, | ||
| value_variable_name: str, | ||
| comparison_title_template: str = "", | ||
| comparison_variable_name: str = "", | ||
| ) -> str: | ||
| value_part = f'f"{{{value_variable_name}}}"' if value_variable_name else '""' | ||
| comparison_value_part = ( | ||
| f'f"{{{comparison_variable_name}}}"' if comparison_variable_name else '""' | ||
| ) | ||
| # Use triple-quoted string for multiline Python code | ||
| return f""" | ||
| def __deepnote_big_number__(): | ||
| import json | ||
| import jinja2 | ||
| from jinja2 import meta | ||
|
|
||
| def render_template(template): | ||
| parsed_content = jinja2.Environment().parse(template) | ||
|
|
||
| required_variables = meta.find_undeclared_variables(parsed_content) | ||
|
|
||
| context = {{ | ||
| variable_name: globals().get(variable_name) | ||
| for variable_name in required_variables | ||
| }} | ||
|
|
||
| result = jinja2.Environment().from_string(template).render(context) | ||
|
|
||
| return result | ||
|
|
||
| rendered_title = render_template("{title_template}") | ||
| rendered_comparison_title = render_template("{comparison_title_template}") | ||
|
|
||
| return json.dumps({{ | ||
| "comparisonTitle": rendered_comparison_title, | ||
| "comparisonValue": {comparison_value_part}, | ||
| "title": rendered_title, | ||
| "value": {value_part} | ||
| }}) | ||
|
|
||
| __deepnote_big_number__() | ||
| """.lstrip() | ||
|
|
Contributor
There was a problem hiding this comment.
Big-number: unescaped templates and NameError risk on values.
- title/comparison templates are embedded without escaping; quotes/newlines will break the generated code.
- Referencing variables via f-strings will NameError if the variable is undefined.
Fix by escaping templates and reading values via globals(); also guard jinja2 import.
Apply:
def execute_big_number(
title_template: str,
value_variable_name: str,
comparison_title_template: str = "",
comparison_variable_name: str = "",
) -> str:
- value_part = f'f"{{{value_variable_name}}}"' if value_variable_name else '""'
- comparison_value_part = (
- f'f"{{{comparison_variable_name}}}"' if comparison_variable_name else '""'
- )
+ safe_title = escape_python_string(title_template)
+ safe_comp_title = escape_python_string(comparison_title_template)
+ vv = sanitize_python_variable_name(value_variable_name or "", disable_empty_fallback=True)
+ cv = sanitize_python_variable_name(comparison_variable_name or "", disable_empty_fallback=True)
+ value_part = f"str(globals().get({escape_python_string(vv)}, ''))" if vv else '""'
+ comparison_value_part = f"str(globals().get({escape_python_string(cv)}, ''))" if cv else '""'
# Use triple-quoted string for multiline Python code
return f"""
def __deepnote_big_number__():
- import json
- import jinja2
- from jinja2 import meta
+ import json
+ try:
+ import jinja2
+ from jinja2 import meta
+ except Exception:
+ jinja2 = None
+ meta = None
def render_template(template):
- parsed_content = jinja2.Environment().parse(template)
+ if jinja2 is None:
+ return template
+ parsed_content = jinja2.Environment().parse(template)
required_variables = meta.find_undeclared_variables(parsed_content)
context = {{
variable_name: globals().get(variable_name)
for variable_name in required_variables
}}
result = jinja2.Environment().from_string(template).render(context)
return result
- rendered_title = render_template("{title_template}")
- rendered_comparison_title = render_template("{comparison_title_template}")
+ rendered_title = render_template({safe_title})
+ rendered_comparison_title = render_template({safe_comp_title})
return json.dumps({{
"comparisonTitle": rendered_comparison_title,
"comparisonValue": {comparison_value_part},
"title": rendered_title,
"value": {value_part}
}})
__deepnote_big_number__()
""".lstrip()Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+160
to
+165
| # Accepts empty string or ISO date string | ||
| def is_date_or_empty(x): | ||
| return x == "" or (isinstance(x, str) and re.match(r"^\d{4}-\d{2}-\d{2}", x)) | ||
|
|
||
| return is_date_or_empty(s) and is_date_or_empty(e) | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Absolute date regex should anchor end.
Prevents partial matches like "2025-09-17T...".
- return x == "" or (isinstance(x, str) and re.match(r"^\d{4}-\d{2}-\d{2}", x))
+ return x == "" or (isinstance(x, str) and re.match(r"^\d{4}-\d{2}-\d{2}$", x))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Accepts empty string or ISO date string | |
| def is_date_or_empty(x): | |
| return x == "" or (isinstance(x, str) and re.match(r"^\d{4}-\d{2}-\d{2}", x)) | |
| return is_date_or_empty(s) and is_date_or_empty(e) | |
| # Accepts empty string or ISO date string | |
| def is_date_or_empty(x): | |
| return x == "" or (isinstance(x, str) and re.match(r"^\d{4}-\d{2}-\d{2}$", x)) | |
| return is_date_or_empty(s) and is_date_or_empty(e) |
🤖 Prompt for AI Agents
In jupyterlab_deepnote/convert_format.py around lines 160 to 165, the date regex
is not anchored at the end so strings like "2025-09-17T..." incorrectly match;
change the regex to require the end (for example use ^\d{4}-\d{2}-\d{2}$ or use
re.fullmatch) so only exact YYYY-MM-DD strings are accepted, and keep the
existing empty-string allowance logic unchanged.
Comment on lines
+174
to
+179
| def is_valid_relative_date_interval(value): | ||
| # Accepts one of the known keys | ||
| if not isinstance(value, str): | ||
| return value in [] | ||
| return value in DATE_RANGE_INPUT_RELATIVE_RANGES | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Relative date interval: non-str branch always False; be explicit.
def is_valid_relative_date_interval(value):
# Accepts one of the known keys
if not isinstance(value, str):
- return value in []
+ return False
return value in DATE_RANGE_INPUT_RELATIVE_RANGES📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def is_valid_relative_date_interval(value): | |
| # Accepts one of the known keys | |
| if not isinstance(value, str): | |
| return value in [] | |
| return value in DATE_RANGE_INPUT_RELATIVE_RANGES | |
| def is_valid_relative_date_interval(value): | |
| # Accepts one of the known keys | |
| if not isinstance(value, str): | |
| return False | |
| return value in DATE_RANGE_INPUT_RELATIVE_RANGES |
🤖 Prompt for AI Agents
In jupyterlab_deepnote/convert_format.py around lines 174 to 179, the non-string
branch of is_valid_relative_date_interval currently returns a confusing
expression (value in []) which is always False; change it to explicitly return
False when value is not an instance of str, and keep the string branch returning
whether value is in DATE_RANGE_INPUT_RELATIVE_RANGES so the function clearly and
deterministically returns a boolean.
Comment on lines
+191
to
+195
| def date_range_custom_days(name, days): | ||
| return ( | ||
| "from datetime import datetime, timedelta\n" | ||
| f"{name} = [datetime.now().date() - timedelta(days={days}), datetime.now().date()]" | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Consistent aliasing in generated code.
Other helpers use underscored aliases; align customDays for readability.
def date_range_custom_days(name, days):
return (
- "from datetime import datetime, timedelta\n"
- f"{name} = [datetime.now().date() - timedelta(days={days}), datetime.now().date()]"
+ "from datetime import datetime as _deepnote_datetime, timedelta as _deepnote_timedelta\n"
+ f"{name} = [_deepnote_datetime.now().date() - _deepnote_timedelta(days={days}), _deepnote_datetime.now().date()]"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def date_range_custom_days(name, days): | |
| return ( | |
| "from datetime import datetime, timedelta\n" | |
| f"{name} = [datetime.now().date() - timedelta(days={days}), datetime.now().date()]" | |
| ) | |
| def date_range_custom_days(name, days): | |
| return ( | |
| "from datetime import datetime as _deepnote_datetime, timedelta as _deepnote_timedelta\n" | |
| f"{name} = [_deepnote_datetime.now().date() - _deepnote_timedelta(days={days}), _deepnote_datetime.now().date()]" | |
| ) |
🤖 Prompt for AI Agents
In jupyterlab_deepnote/convert_format.py around lines 191 to 195, the generated
variable uses camelCase (e.g., customDays) while other helpers use snake_case;
update the function to generate a snake_cased alias from the provided name
(e.g., convert camelCase to snake_case using a simple regex or helper) and use
that snake_case alias in the returned code string so the produced variable names
are consistently underscored across helpers.
Comment on lines
+272
to
+306
| elif btype == "input-text" or btype == "input-textarea" or btype == "input-slider": | ||
| # Generate Python assignment: {sanitized_variable_name} = {escaped_value} | ||
| var_name = sanitize_python_variable_name( | ||
| metadata.get("deepnote_variable_name", "") | ||
| ) | ||
| value = escape_python_string(metadata.get("deepnote_variable_value", "")) | ||
| return f"{var_name} = {value}" | ||
| elif btype == "input-checkbox": | ||
| # Sanitize variable name and assign True/False based on value | ||
| var_name = sanitize_python_variable_name( | ||
| metadata.get("deepnote_variable_name", "") | ||
| ) | ||
| value = metadata.get("deepnote_variable_value") | ||
| return f"{var_name} = {'True' if value else 'False'}" | ||
| elif btype == "input-select": | ||
| var_name = sanitize_python_variable_name( | ||
| metadata.get("deepnote_variable_name", "") | ||
| ) | ||
| allow_multiple = metadata.get("deepnote_allow_multiple_values") | ||
| values = metadata.get("deepnote_variable_value", None) | ||
| # If allow_multiple is truthy or values is a list, treat as list assignment | ||
| if allow_multiple or isinstance(values, list): | ||
| # Ensure values is a list | ||
| if not isinstance(values, list): | ||
| values = [values] if values is not None else [] | ||
| return ( | ||
| f"{var_name} = [{', '.join(escape_python_string(v) for v in values)}]" | ||
| ) | ||
| # If not allow_multiple and value is falsy (None or empty string), assign None | ||
| elif not allow_multiple and (values is None or values == ""): | ||
| return f"{var_name} = None" | ||
| else: | ||
| # Explicitly convert to string for type safety | ||
| return f"{var_name} = {escape_python_string(str(values or ''))}" | ||
| elif btype == "input-date": |
Contributor
There was a problem hiding this comment.
Inputs: slider coerced to string; select drops 0 values.
- Slider assigns quoted strings; should be numeric when possible.
- For single-select,
str(values or '')loses 0.
Apply:
- elif btype == "input-text" or btype == "input-textarea" or btype == "input-slider":
+ elif btype in ("input-text", "input-textarea"):
# Generate Python assignment: {sanitized_variable_name} = {escaped_value}
var_name = sanitize_python_variable_name(
metadata.get("deepnote_variable_name", "")
)
value = escape_python_string(metadata.get("deepnote_variable_value", ""))
return f"{var_name} = {value}"
+ elif btype == "input-slider":
+ var_name = sanitize_python_variable_name(
+ metadata.get("deepnote_variable_name", "")
+ )
+ raw = metadata.get("deepnote_variable_value", None)
+ if isinstance(raw, (int, float)):
+ return f"{var_name} = {raw}"
+ # tolerate numeric strings like "3" or "3.14"
+ elif isinstance(raw, str) and raw.replace('.', '', 1).isdigit():
+ return f"{var_name} = {raw}"
+ else:
+ return f"{var_name} = {escape_python_string('' if raw is None else raw)}"
@@
- elif not allow_multiple and (values is None or values == ""):
+ elif not allow_multiple and (values is None or values == ""):
return f"{var_name} = None"
else:
# Explicitly convert to string for type safety
- return f"{var_name} = {escape_python_string(str(values or ''))}"
+ return f"{var_name} = {escape_python_string(str(values))}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| elif btype == "input-text" or btype == "input-textarea" or btype == "input-slider": | |
| # Generate Python assignment: {sanitized_variable_name} = {escaped_value} | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| value = escape_python_string(metadata.get("deepnote_variable_value", "")) | |
| return f"{var_name} = {value}" | |
| elif btype == "input-checkbox": | |
| # Sanitize variable name and assign True/False based on value | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| value = metadata.get("deepnote_variable_value") | |
| return f"{var_name} = {'True' if value else 'False'}" | |
| elif btype == "input-select": | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| allow_multiple = metadata.get("deepnote_allow_multiple_values") | |
| values = metadata.get("deepnote_variable_value", None) | |
| # If allow_multiple is truthy or values is a list, treat as list assignment | |
| if allow_multiple or isinstance(values, list): | |
| # Ensure values is a list | |
| if not isinstance(values, list): | |
| values = [values] if values is not None else [] | |
| return ( | |
| f"{var_name} = [{', '.join(escape_python_string(v) for v in values)}]" | |
| ) | |
| # If not allow_multiple and value is falsy (None or empty string), assign None | |
| elif not allow_multiple and (values is None or values == ""): | |
| return f"{var_name} = None" | |
| else: | |
| # Explicitly convert to string for type safety | |
| return f"{var_name} = {escape_python_string(str(values or ''))}" | |
| elif btype == "input-date": | |
| elif btype in ("input-text", "input-textarea"): | |
| # Generate Python assignment: {sanitized_variable_name} = {escaped_value} | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| value = escape_python_string(metadata.get("deepnote_variable_value", "")) | |
| return f"{var_name} = {value}" | |
| elif btype == "input-slider": | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| raw = metadata.get("deepnote_variable_value", None) | |
| if isinstance(raw, (int, float)): | |
| return f"{var_name} = {raw}" | |
| # tolerate numeric strings like "3" or "3.14" | |
| elif isinstance(raw, str) and raw.replace('.', '', 1).isdigit(): | |
| return f"{var_name} = {raw}" | |
| else: | |
| return f"{var_name} = {escape_python_string('' if raw is None else raw)}" | |
| elif btype == "input-checkbox": | |
| # Sanitize variable name and assign True/False based on value | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| value = metadata.get("deepnote_variable_value") | |
| return f"{var_name} = {'True' if value else 'False'}" | |
| elif btype == "input-select": | |
| var_name = sanitize_python_variable_name( | |
| metadata.get("deepnote_variable_name", "") | |
| ) | |
| allow_multiple = metadata.get("deepnote_allow_multiple_values") | |
| values = metadata.get("deepnote_variable_value", None) | |
| # If allow_multiple is truthy or values is a list, treat as list assignment | |
| if allow_multiple or isinstance(values, list): | |
| # Ensure values is a list | |
| if not isinstance(values, list): | |
| values = [values] if values is not None else [] | |
| return ( | |
| f"{var_name} = [{', '.join(escape_python_string(v) for v in values)}]" | |
| ) | |
| # If not allow_multiple and value is falsy (None or empty string), assign None | |
| elif not allow_multiple and (values is None or values == ""): | |
| return f"{var_name} = None" | |
| else: | |
| # Explicitly convert to string for type safety | |
| return f"{var_name} = {escape_python_string(str(values))}" | |
| elif btype == "input-date": |
Comment on lines
+310
to
+325
| escaped_value = escape_python_string( | ||
| metadata.get("deepnote_variable_value", "") | ||
| ) | ||
| if not metadata.get("deepnote_variable_value"): | ||
| return f"{var_name} = None" | ||
| elif metadata.get("deepnote_input_date_version") == 2: | ||
| return ( | ||
| f"from dateutil.parser import parse as _deepnote_parse\n" | ||
| f"{var_name} = _deepnote_parse({escaped_value}).date()" | ||
| ) | ||
| else: | ||
| return ( | ||
| f"from datetime import datetime as _deepnote_datetime\n" | ||
| f'{var_name} = _deepnote_datetime.strptime({escaped_value}, "%Y-%m-%dT%H:%M:%S.%fZ")' | ||
| ) | ||
| elif btype == "input-date-range": |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Input date v1 returns datetime, v2 returns date; normalize.
Append .date() in the v1 path for consistency.
else:
return (
f"from datetime import datetime as _deepnote_datetime\n"
- f'{var_name} = _deepnote_datetime.strptime({escaped_value}, "%Y-%m-%dT%H:%M:%S.%fZ")'
+ f'{var_name} = _deepnote_datetime.strptime({escaped_value}, "%Y-%m-%dT%H:%M:%S.%fZ").date()'
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| escaped_value = escape_python_string( | |
| metadata.get("deepnote_variable_value", "") | |
| ) | |
| if not metadata.get("deepnote_variable_value"): | |
| return f"{var_name} = None" | |
| elif metadata.get("deepnote_input_date_version") == 2: | |
| return ( | |
| f"from dateutil.parser import parse as _deepnote_parse\n" | |
| f"{var_name} = _deepnote_parse({escaped_value}).date()" | |
| ) | |
| else: | |
| return ( | |
| f"from datetime import datetime as _deepnote_datetime\n" | |
| f'{var_name} = _deepnote_datetime.strptime({escaped_value}, "%Y-%m-%dT%H:%M:%S.%fZ")' | |
| ) | |
| elif btype == "input-date-range": | |
| escaped_value = escape_python_string( | |
| metadata.get("deepnote_variable_value", "") | |
| ) | |
| if not metadata.get("deepnote_variable_value"): | |
| return f"{var_name} = None" | |
| elif metadata.get("deepnote_input_date_version") == 2: | |
| return ( | |
| f"from dateutil.parser import parse as _deepnote_parse\n" | |
| f"{var_name} = _deepnote_parse({escaped_value}).date()" | |
| ) | |
| else: | |
| return ( | |
| f"from datetime import datetime as _deepnote_datetime\n" | |
| f'{var_name} = _deepnote_datetime.strptime({escaped_value}, "%Y-%m-%dT%H:%M:%S.%fZ").date()' | |
| ) | |
| elif btype == "input-date-range": |
🤖 Prompt for AI Agents
In jupyterlab_deepnote/convert_format.py around lines 310 to 325, the branch
handling input-date v1 returns a datetime while v2 returns a date; modify the v1
return so it calls .date() on the parsed datetime to normalize both versions to
date. Specifically, keep the existing from datetime import datetime as
_deepnote_datetime import and change the returned expression to call .date() on
the result of _deepnote_datetime.strptime(...), ensuring escaped_value is used
as before.
Comment on lines
+386
to
+396
| def convert_deepnote_block_to_jupyter_cell(block: dict): | ||
| btype = block.get("type", "markdown") | ||
| content = block.get("content", "") | ||
| source = map_block_source(btype, content, block.get("metadata", {})) | ||
|
|
||
| jupyter_type = map_block_type_to_jupyter(btype) | ||
|
|
||
| if jupyter_type == "code": | ||
| return new_code_cell(source) | ||
| else: | ||
| return new_markdown_cell(source) |
Contributor
There was a problem hiding this comment.
Coerce non-string sources before cell creation.
Some blocks may produce dict/list sources (e.g., visualization specs); ensure valid cell source.
def convert_deepnote_block_to_jupyter_cell(block: dict):
btype = block.get("type", "markdown")
content = block.get("content", "")
source = map_block_source(btype, content, block.get("metadata", {}))
jupyter_type = map_block_type_to_jupyter(btype)
- if jupyter_type == "code":
- return new_code_cell(source)
- else:
- return new_markdown_cell(source)
+ if not isinstance(source, str):
+ source = repr(source) if jupyter_type == "code" else str(source)
+ return new_code_cell(source) if jupyter_type == "code" else new_markdown_cell(source)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def convert_deepnote_block_to_jupyter_cell(block: dict): | |
| btype = block.get("type", "markdown") | |
| content = block.get("content", "") | |
| source = map_block_source(btype, content, block.get("metadata", {})) | |
| jupyter_type = map_block_type_to_jupyter(btype) | |
| if jupyter_type == "code": | |
| return new_code_cell(source) | |
| else: | |
| return new_markdown_cell(source) | |
| def convert_deepnote_block_to_jupyter_cell(block: dict): | |
| btype = block.get("type", "markdown") | |
| content = block.get("content", "") | |
| source = map_block_source(btype, content, block.get("metadata", {})) | |
| jupyter_type = map_block_type_to_jupyter(btype) | |
| if not isinstance(source, str): | |
| source = repr(source) if jupyter_type == "code" else str(source) | |
| return new_code_cell(source) if jupyter_type == "code" else new_markdown_cell(source) |
🤖 Prompt for AI Agents
In jupyterlab_deepnote/convert_format.py around lines 386 to 396, the function
may pass dicts/lists as the cell source which is invalid for Jupyter cells;
coerce the mapped source into an appropriate type before creating a cell: if
source is a dict, serialize it to a compact JSON string; if it is a list,
convert each element to string and join with "\n" (or if elements are already
strings you may keep as a list per nbformat convention); otherwise call
str(source) for other non-string types; then pass the resulting string/list into
new_code_cell/new_markdown_cell so all created cells have valid source types.
Comment on lines
+400
to
+404
| def convert_blocks_to_cells(blocks: list[dict]): | ||
| return [ | ||
| convert_deepnote_block_to_jupyter_cell(block) | ||
| for block in sorted(blocks, key=lambda b: b.get("sortingKey", "")) | ||
| ] |
Contributor
There was a problem hiding this comment.
Sorting key may be int; cast to str to avoid TypeError.
def convert_blocks_to_cells(blocks: list[dict]):
return [
convert_deepnote_block_to_jupyter_cell(block)
- for block in sorted(blocks, key=lambda b: b.get("sortingKey", ""))
+ for block in sorted(blocks, key=lambda b: str(b.get("sortingKey", "")))
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def convert_blocks_to_cells(blocks: list[dict]): | |
| return [ | |
| convert_deepnote_block_to_jupyter_cell(block) | |
| for block in sorted(blocks, key=lambda b: b.get("sortingKey", "")) | |
| ] | |
| def convert_blocks_to_cells(blocks: list[dict]): | |
| return [ | |
| convert_deepnote_block_to_jupyter_cell(block) | |
| for block in sorted(blocks, key=lambda b: str(b.get("sortingKey", ""))) | |
| ] |
🤖 Prompt for AI Agents
In jupyterlab_deepnote/convert_format.py around lines 400 to 404, the sortingKey
may be an int which can raise a TypeError when mixed with strings during
sorting; update the key function to cast the sortingKey to str (and fall back to
empty string when missing) so sorting always compares strings, e.g. replace the
current key=lambda b: b.get("sortingKey", "") with a key that converts the value
to str.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ports our TypeScript code from the
deepnoterepo to Python.There are 3 missing converters for block types: image, button, visualization. They will be added to this PR later, or they’ll come in a separate PR.
Signed-off-by: Andy Jakubowski [email protected]
Summary by CodeRabbit
New Features
Documentation
Refactor