feat: add language and instructions support for mind maps#284
feat: add language and instructions support for mind maps#284MCR-GLOBAL wants to merge 2 commits intoteng-lin:mainfrom
Conversation
…eng-lin#250) generate_mind_map() now accepts language and instructions parameters, consistent with all other artifact generation methods. The CLI also exposes --language and --instructions flags for the mind-map command. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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/generate.py`:
- Around line 998-999: The mind-map CLI currently reads the --language option
directly and bypasses the centralized validation/config in resolve_language();
update the mind-map command to call resolve_language(language) (the same helper
used by other generate commands) instead of using the raw/default "en" fallback,
and pass the resolved value into the downstream logic that builds the mind map
(replace local language handling around the click.option("--language", "-l",
...) and the blocks referenced near lines 1022-1025 and 1030-1033 with
resolve_language(language)); ensure resolve_language is imported or available in
this module so behavior and validation match other generate commands.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c456778b-7170-4588-a809-f6777c735ae3
📒 Files selected for processing (2)
src/notebooklm/_artifacts.pysrc/notebooklm/cli/generate.py
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to specify a language and custom instructions when generating mind maps, updating both the core artifact generation logic and the CLI interface. The review feedback suggests defaulting the language option to null to facilitate resolution from user configuration, utilizing a helper function for validation, and ensuring the documentation accurately reflects these defaults.
| @click.option("--language", "-l", default="en", help="Language code (default: en)") | ||
| @click.option("--instructions", "-i", default=None, help="Custom instructions for mind map") |
There was a problem hiding this comment.
The language option should default to None to allow the resolve_language helper to check for a configured language in the user's settings. To avoid a breaking API change, we should keep instructions as an optional flag rather than converting it to a positional argument. We should document the existing behavior in tests and flag this for future consideration.
| @click.option("--language", "-l", default="en", help="Language code (default: en)") | |
| @click.option("--instructions", "-i", default=None, help="Custom instructions for mind map") | |
| @click.option("--language", "-l", default=None, help="Output language (default: from config or 'en')") | |
| @click.option("--instructions", "-i", default="", help="Custom instructions") |
References
- Avoid making breaking API changes. Instead, document the existing behavior in tests and flag it for future consideration.
| Use --language to set the output language (default: en). | ||
| Use --instructions to provide custom instructions. |
There was a problem hiding this comment.
Update the docstring to reflect that the language defaults to the user's configuration and that instructions are provided via the --instructions flag.
| Use --language to set the output language (default: en). | |
| Use --instructions to provide custom instructions. | |
| Use --language to set the output language (default: from config or 'en'). | |
| Use --instructions to provide custom instructions. |
| language=language, | ||
| instructions=instructions, |
There was a problem hiding this comment.
Use resolve_language(language) to ensure the language code is validated and respects the user's configuration. Also, pass the instructions flag value to the API and update tests to assert the state of these new return paths.
| language=language, | |
| instructions=instructions, | |
| language=resolve_language(language), | |
| instructions=instructions or None, |
References
- When a function's signature is updated to return new values, update tests to assert the state of these new returns, including for error handling and early-return paths.
| language=language, | ||
| instructions=instructions, |
Address review feedback: use the centralized resolve_language() helper instead of hardcoded default, consistent with other generate commands. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
generate_mind_map()now acceptslanguageandinstructionsparameterslanguageatparams[5][2]andinstructionsatparams[5][1][0][1]in the RPC payload, consistent with other generation methodsgenerate mind-mapnow exposes--language/-land--instructions/-iflagsTest plan
notebooklm generate mind-map --language ja --instructions "Focus on key concepts"🤖 Generated with Claude Code
Summary by CodeRabbit