fix: auto-set context to newly created notebook#220
fix: auto-set context to newly created notebook#220netzwichte1 wants to merge 1 commit intoteng-lin:mainfrom
Conversation
After `notebooklm create`, subsequent commands like `source add` would target the previously active notebook instead of the newly created one, because `create_cmd()` did not call `set_current_notebook()`. This adds a `set_current_notebook()` call after notebook creation, matching the behavior of the `use` command. This ensures that `source add`, `ask`, and other context-dependent commands target the correct notebook without requiring a manual `notebooklm use` step.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a user experience issue where creating a new notebook via the CLI did not automatically switch the active context to that new notebook. This led to confusion and incorrect operations when subsequent commands were executed. The change ensures that after a notebook is created, the CLI's context is immediately updated, streamlining the workflow and making the tool more intuitive to use. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful usability improvement by automatically setting the CLI context to a newly created notebook. The implementation is mostly correct, but there is a potential for a runtime error. I've identified a case where accessing nb.is_owner could fail if the API response for creating a notebook doesn't include this attribute. My suggestion is to assume ownership is True for a newly created notebook, which makes the implementation more robust. It's also worth noting that the associated unit tests were not updated to reflect these changes, which is something to address.
|
|
||
| # Auto-switch context to the newly created notebook | ||
| created_str = nb.created_at.strftime("%Y-%m-%d") if nb.created_at else None | ||
| set_current_notebook(nb.id, nb.title, nb.is_owner, created_str) |
There was a problem hiding this comment.
When a notebook is created, the current user is always the owner. It's safer to explicitly pass True for is_owner rather than relying on nb.is_owner. The create API endpoint might not return the is_owner field, which could cause an AttributeError. The existing unit tests for this command also mock a Notebook object without this attribute, suggesting it might not be present.
| set_current_notebook(nb.id, nb.title, nb.is_owner, created_str) | |
| set_current_notebook(nb.id, nb.title, True, created_str) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/notebooklm/cli/notebook.py`:
- Line 102: The console.print call in notebook.py uses an unnecessary f-string:
locate the console.print(f"[dim]Context set to new notebook[/dim]") invocation
(e.g., in the function or block that prints the context change) and remove the
leading "f" so the literal string is a normal string:
console.print("[dim]Context set to new notebook[/dim]"). This eliminates the
unused f-string and resolves the Ruff F541 warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b92e416-1bf6-4cc5-b5b8-7352e8ec9a6c
📒 Files selected for processing (1)
src/notebooklm/cli/notebook.py
| return | ||
|
|
||
| console.print(f"[green]Created notebook:[/green] {nb.id} - {nb.title}") | ||
| console.print(f"[dim]Context set to new notebook[/dim]") |
There was a problem hiding this comment.
Remove unnecessary f-string prefix.
This string has no placeholders, so the f prefix is extraneous. Ruff flags this as F541.
🔧 Proposed fix
- console.print(f"[dim]Context set to new notebook[/dim]")
+ console.print("[dim]Context set to new notebook[/dim]")🧰 Tools
🪛 Ruff (0.15.6)
[error] 102-102: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/notebooklm/cli/notebook.py` at line 102, The console.print call in
notebook.py uses an unnecessary f-string: locate the
console.print(f"[dim]Context set to new notebook[/dim]") invocation (e.g., in
the function or block that prints the context change) and remove the leading "f"
so the literal string is a normal string: console.print("[dim]Context set to new
notebook[/dim]"). This eliminates the unused f-string and resolves the Ruff F541
warning.
|
Behavior is reasonable. Two things blocking merge: (1) |
3 participants
Summary
notebooklm createnow automatically sets the CLI context to the newly created notebookcreate, subsequent commands likesource addwould target the previously active notebook — not the one just createdset_current_notebook()call after notebook creation, matching the behavior of theusecommandProblem
Fix
Changes
src/notebooklm/cli/notebook.py: Importset_current_notebookand call it afterclient.notebooks.create()increate_cmd()Test plan
notebooklm create "Test" --json→ verify context switches (check withnotebooklm status)notebooklm create "Test"(non-JSON) → verify "Context set to new notebook" messagenotebooklm source add <url>after create → verify source lands in new notebookSummary by CodeRabbit
Release Notes