fix(notebooks): update model versions, API key handling, and pip install quiet flag#333
Closed
PedramNavid wants to merge 1 commit into
Closed
fix(notebooks): update model versions, API key handling, and pip install quiet flag#333PedramNavid wants to merge 1 commit into
PedramNavid wants to merge 1 commit into
Conversation
Notebook ChangesThis PR modifies the following notebooks: 📓
|
Summary
Errors per inputErrors in temp_md/00_The_one_liner_research_agent.md
Errors in temp_md/01_skills_introduction.md
Errors in temp_md/01_The_chief_of_staff_agent.md
Errors in temp_md/02_skills_financial_applications.md
Errors in temp_md/02_The_observability_agent.md
Errors in temp_md/03_skills_custom_development.md
Errors in temp_md/automatic-context-compaction.md
Errors in temp_md/claude_3_rag_agent.md
Errors in temp_md/extended_thinking.md
Errors in temp_md/extended_thinking_with_tool_use.md
Errors in temp_md/extracting_structured_json.md
Errors in temp_md/finetuning_on_bedrock.md
Errors in temp_md/getting_started_with_vision.md
Errors in temp_md/guide.md
Errors in temp_md/low_latency_stt_claude_tts.md
Errors in temp_md/memory_cookbook.md
Errors in temp_md/orchestrator_workers.md
Errors in temp_md/pdf_upload_summarization.md
Errors in temp_md/prerecorded_audio.md
Errors in temp_md/programmatic_tool_calling_ptc.md
Errors in temp_md/rag_using_mongodb.md
Errors in temp_md/rag_using_pinecone.md
Errors in temp_md/tool_choice.md
Errors in temp_md/usage_cost_api.md
Errors in temp_md/using_citations.md
Errors in temp_md/using_llm_api.md
Errors in temp_md/wikipedia-search-cookbook.md
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Recommendation: REQUEST_CHANGES
Summary
This PR systematically updates model versions to date-suffixed formats, removes hardcoded API keys, and replaces %%capture with pip install -q across 45 notebooks. The core changes are correct, but several critical issues need to be fixed before merging.
Actionable Feedback (4 items)
Critical Issues:
- tool_use/memory_cookbook.ipynb:157 - Remove extra indentation (8 leading spaces) from the line with Install required packages comment to prevent IndentationError when cell executes
- Remove unintended file notebook_review_summary.csv (64-line CSV that appears to be a review artifact)
- Add trailing newlines to all notebook files that are missing them (POSIX compliance)
Important Issue:
- General: ~45 notebooks have cell formatting changed from multi-line array format to single-line with escaped newlines. This makes git diffs harder to read and notebooks harder to maintain. Consider reverting to original multi-line format while preserving the content changes.
Code Quality
Strengths:
- Comprehensive scope: All 45 notebooks systematically updated
- Consistent patterns: Model versions uniform across all files
- Documentation updated: All .claude/ docs and CLAUDE.md properly updated
- Validation scripts updated: validate_notebook.py correctly checks for new patterns
- Python files included: Not just notebooks - also updated agent files
Model Version Updates (Verified Correct):
- Sonnet: claude-sonnet-4-5-20250929
- Haiku: claude-haiku-4-5-20251001
- Opus: claude-opus-4-5-20251101
API Key Handling (Correct):
- Properly replaced hardcoded placeholders with Anthropic() that auto-reads env vars
- Added helpful comments about environment variable setup
- No security issues
Pip Install Pattern (Correct):
- %%capture to -q flag consistently applied
- Multiple commands consolidated where appropriate
- Validation script properly updated
Issues Found
Critical:
- Indentation Error in tool_use/memory_cookbook.ipynb:157 - Extra 8 spaces will cause Python IndentationError
- Unintended File: notebook_review_summary.csv appears to be development artifact
- Missing Newlines: Many notebooks missing POSIX-required trailing newline
Important:
4. Cell Format Changes: 225+ instances where cells converted from readable multi-line arrays to escaped newlines. This makes diffs show entire cells as changed instead of just modified lines.
Suggested Fixes
- Fix indentation - manually edit tool_use/memory_cookbook.ipynb:157
- Remove CSV: git rm notebook_review_summary.csv
- Fix trailing newlines with appropriate tool
- (Optional but recommended) Revert cell formatting while keeping content changes
Security
No security concerns. API key handling improvements are correct and follow best practices.
Positive Notes
- Excellent thoroughness in covering all notebooks
- Good user experience improvement with quieter pip install output
- Security improvement removing hardcoded API key placeholders
- Maintenance improvement with date-suffixed model versions making it clear which versions are in use
- Documentation-first approach updating guidelines alongside code
…all -q flag - Update all model references to use date-suffixed versions: - claude-sonnet-4-5-20250929 - claude-haiku-4-5-20251001 - claude-opus-4-5-20251101 - Replace hardcoded API key placeholders with os.environ.get() pattern - Add -q flag to pip install commands for quiet output - Update .claude documentation to reflect new pip install -q guideline - Update validate_notebook.py to check for -q flag 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
PedramNavid
force-pushed
the
pedram/fix-notebook-standards
branch
from
3af911f to
ce22da2
Compare
Notebook ChangesThis PR modifies the following notebooks: 📓
|
There was a problem hiding this comment.
PR Review
Recommendation: REQUEST_CHANGES
Summary
This PR systematically updates model versions to date-suffixed formats, improves API key security, and migrates from %%capture to -q flag across 67 files. The changes are comprehensive and well-executed, but one notebook still uses both %%capture AND -q together, which is inconsistent with the migration goal.
Actionable Feedback (1 item)
-
claude_agent_sdk/00_The_one_liner_research_agent.ipynb(cell with%pip install -q -U claude-agent-sdk python-dotenv) - Remove the%%capturemagic at the start of this cell. Using both%%captureand-qis redundant - the-qflag alone suppresses pip output as intended.
Detailed Review
Code Quality
Strengths:
- Systematic approach: All 45 notebooks and 4 Python files updated consistently
- Correct model mappings: All date suffixes match CLAUDE.md specifications
- Sonnet:
claude-sonnet-4-5-20250929✓ - Haiku:
claude-haiku-4-5-20251001✓ - Opus:
claude-opus-4-5-20251101✓
- Sonnet:
- Documentation alignment: All
.claude/documentation and CLAUDE.md properly updated - Pattern consistency: Changes follow clear, predictable patterns across all files
Issues:
- One notebook (
claude_agent_sdk/00_The_one_liner_research_agent.ipynb) uses both%%captureand-qflag together, contradicting the migration from%%captureTO-q
Security
Excellent improvements:
- Removed all hardcoded API key examples (
os.environ["ANTHROPIC_API_KEY"] = "...") - Changed to comment-only guidance about environment variables
- No security concerns identified
Python Script Updates
Agent scripts (claude_agent_sdk/*/agent.py):
- All three agent scripts correctly updated to use
claude-opus-4-5-20251101 - DEFAULT_MODEL constants properly updated
Validation script (validate_notebook.py):
- Updated to check for
-qflag instead of%%capture - Valid model list correctly updated with date suffixes
- Note: Current logic won't catch notebooks with BOTH
%%captureand-q. Consider adding explicit check:
if has_pip_install and has_capture:
self.warnings.append(
f"Cell {i}: Using deprecated %%capture with pip install. Use -q flag instead."
)Model Version Strategy
Legacy model migration:
claude-opus-4-1references changed toclaude-sonnet-4-5-20250929(not Opus 4.5)- This is a reasonable choice for educational content (cost-effective, still powerful)
- Applies to:
misc/building_evals.ipynb,third_party/LlamaIndex/*.ipynb
Suggestions
-
Notebook outputs: Some agent SDK notebook output cells contain old model names (e.g., HTML timeline displays). These are cosmetic and don't affect functionality, but could be refreshed by re-running notebooks.
-
Files outside PR scope: These files still need updating (suggest follow-up PR):
capabilities/text_to_sql/evaluation/promptfooconfig.yaml- still uses models without date suffixesthird_party/ElevenLabs/stream_voice_assistant_websocket.py- still usesclaude-haiku-4-5
-
User guidance: Consider adding a markdown cell in notebooks explaining how to set environment variables for users unfamiliar with the pattern.
Positive Notes
- Comprehensive coverage - 67 files updated with no missed patterns
- Security improvement - Prevents accidental API key exposure
- Validation alignment - Scripts and documentation updated to match new standards
- Grouped pip installs - Multiple packages combined in single commands where appropriate
- Python script consistency - Not just notebooks, but supporting Python files also updated
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This PR fixes multiple issues across all notebooks identified in the cookbook audit:
Model versions: Updated all model references to use date-suffixed versions per project standards
claude-sonnet-4-5→claude-sonnet-4-5-20250929claude-haiku-4-5→claude-haiku-4-5-20251001claude-opus-4-5→claude-opus-4-5-20251101claude-opus-4-1and legacyclaude-2.xAPI key handling: Replaced hardcoded API key placeholders with proper
os.environ.get()patternpip install quiet: Replaced
%%capturemagic withpip install -qflag for cleaner output-qflag.claude/documentation to reflect new guidelinevalidate_notebook.pyto check for-qflag instead of%%captureFiles Changed
Categories Updated
Test plan
🤖 Generated with Claude Code