fix: add language and instructions parameters to generate_mind_map()#268
fix: add language and instructions parameters to generate_mind_map()#268octo-patch wants to merge 2 commits intoteng-lin:mainfrom
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded optional 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.
Code Review
This pull request introduces the ability to specify a language and custom instructions when generating mind maps. The generate_mind_map function in the artifacts module and the CLI command have been updated to support these parameters, and a new integration test verifies the RPC payload. Feedback suggests refactoring the CLI logic to eliminate duplicate calls to the generation method by using a helper closure.
I am having trouble creating individual review comments. Click here to see my feedback.
src/notebooklm/cli/generate.py (1017-1025)
The call to client.artifacts.generate_mind_map is duplicated inside and outside the console.status block. This redundancy makes the code harder to maintain and deviates from the pattern used in other generation commands. It is better to define the call once using a closure or by preparing the arguments.
async def _generate():
return await client.artifacts.generate_mind_map(
nb_id_resolved,
source_ids=sources,
language=resolved_language,
instructions=instructions
)
if json_output:
result = await _generate()
else:
with console.status("Generating mind map..."):
result = await _generate()
Fixes #250
Problem
generate_mind_map()does not acceptlanguageorinstructionsparameters, unlike every other artifact generation method (audio, video, report, slides, infographic, etc.). The payload contained hardcoded static values:This means mind maps were always generated in English with no custom instructions, ignoring the configured language or any user-provided instructions.
Solution
Added
languageandinstructionsparameters togenerate_mind_map()in_artifacts.py, matching the signature of other generation methods:language: str = "en"— passed toparams[5][2]instructions: str | None = None— passed toparams[5][1][0][1]Updated the CLI
generate mind-mapcommand incli/generate.pyto expose--languageand--instructionsflags, using the sameresolve_language()helper as other generate commands.Testing
tests/integration/test_artifacts.py:test_generate_mind_map_language_and_instructions— verifies that thelanguageandinstructionsvalues are correctly placed in the RPC payloadSummary by CodeRabbit
New Features
CLI
Tests