run-llama/notebookllama
1
0
Fork 0
mirror of https://github.com/run-llama/notebookllama.git synced 2026-06-30 22:17:57 -04:00
Code Issues 9 Packages Projects Releases 10 Wiki Activity

[PR #20] [MERGED] refactor: Restructure embedding CLI #28

New Issue
Closed
opened 2026-02-16 03:17:23 -05:00 by yindo · 0 comments
yindo commented 2026-02-16 03:17:23 -05:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/run-llama/notebookllama/pull/20
Author: @nick-galluzzo
Created: 7/10/2025
Status: ✅ Merged
Merged: 7/10/2025
Merged by: @AstraBert

Base: main ← Head: refactor/custom-embeddings-cli

📝 Commits (10+)

  • 5e6ec53 refactor(cli): introduce BaseEmbeddingApp to reduce code duplication
  • 5316489 refactor(cli): consolidate CLI styles into single base stylesheet for DRY
  • 27a7fa7 refactor(cli): improve styling and consistency
  • 840ca05 refactor: migrate embedding config to screen based navigation
  • 4b55abe refactor: reorganize embedding config into modular structure
  • 9df958f fix: theme toggle
  • 7e0ce17 refactor: remove unused default() function, add proper type hints and dodcumentation
  • 40b944d refactor: restructure embedding cli from single to multi-provider architecture
  • e760d29 refactor: remove reference to "other" embedding since we arent currently supporting that
  • 7730a58 fix: add base screen bindings back to configuration screen

📊 Changes

21 files changed (+650 additions, -476 deletions)

View changed files

📝 pyproject.toml (+1 -1)
➕ tools/cli/config/__init__.py (+3 -0)
➕ tools/cli/config/models.py (+12 -0)
➕ tools/cli/embedding_app.py (+40 -0)
➕ tools/cli/screens/__init__.py (+5 -0)
➕ tools/cli/screens/base.py (+60 -0)
➕ tools/cli/screens/embedding_provider.py (+62 -0)
➕ tools/cli/screens/embedding_providers/__init__.py (+15 -0)
➕ tools/cli/screens/embedding_providers/azure.py (+44 -0)
➕ tools/cli/screens/embedding_providers/bedrock.py (+79 -0)
➕ tools/cli/screens/embedding_providers/cohere.py (+49 -0)
➕ tools/cli/screens/embedding_providers/gemini.py (+53 -0)
➕ tools/cli/screens/embedding_providers/huggingface.py (+56 -0)
➕ tools/cli/screens/embedding_providers/openai.py (+55 -0)
➕ tools/cli/screens/initial.py (+44 -0)
➕ tools/cli/stylesheets/base.tcss (+24 -0)
➖ tools/cli/stylesheets/input.tcss (+0 -19)
➖ tools/cli/stylesheets/select.tcss (+0 -19)
📝 tools/cli/utils.py (+9 -274)
📝 tools/create_llama_cloud_index.py (+38 -162)

...and 1 more files

📄 Description

Hi! 👋

Thanks for implementing the embedding provider CLI from #12! I took a shot at refactoring the structure to improve the maintainability and extensibility.

I think this is an important refactor to allow for easier embedding model extensions so that notebookllama can eventually support any embedding model.

What this refactor accomplishes

Architecture Configuration

  • Introduced BaseScreen and ConfigurationScreen base classes
  • All providers now inherit common functionality
  • Eliminated code duplication across provider implementations
  • Utilize one unified app within Textual instead of quitting and re-building between each page.

Unified UX

  • Consistent keyboard shortcuts and styling across all providers
  • replace ctrl+q binding with a more standardized shift+enter when submitting
  • Proper error handling with user-friendly notifications
  • Password masking and input validation

Maintainability

  • Adding new providers now just requires extending the base class
  • Simplified the main pipeline creation logic significantly
  • Clear separation of concerns between UI and business logic

Enhanced Features

  • Error handling with try/catch blocks
  • Environment variable fallbacks (OpenAI API key detection)
  • Dynamic model fetching for Bedrock (will continue to add more model fetching for others)
  • Consolidated CSS styling system

The functionality remains almost identical, but the code is now more maintainable and extensible. Thanks again for the solid foundation! 🚀

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/run-llama/notebookllama/pull/20 **Author:** [@nick-galluzzo](https://github.com/nick-galluzzo) **Created:** 7/10/2025 **Status:** ✅ Merged **Merged:** 7/10/2025 **Merged by:** [@AstraBert](https://github.com/AstraBert) **Base:** `main` ← **Head:** `refactor/custom-embeddings-cli` --- ### 📝 Commits (10+) - [`5e6ec53`](https://github.com/run-llama/notebookllama/commit/5e6ec53e1c9f9b5cbd07d3badbad601fbfd108d7) refactor(cli): introduce BaseEmbeddingApp to reduce code duplication - [`5316489`](https://github.com/run-llama/notebookllama/commit/53164897cbbdd4dd27e4741e67598eca7397c0cd) refactor(cli): consolidate CLI styles into single base stylesheet for DRY - [`27a7fa7`](https://github.com/run-llama/notebookllama/commit/27a7fa7ce6d337154df18172b78f25a262f1550c) refactor(cli): improve styling and consistency - [`840ca05`](https://github.com/run-llama/notebookllama/commit/840ca05331e473dc09fb59fc8bfe09fbfa643ad0) refactor: migrate embedding config to screen based navigation - [`4b55abe`](https://github.com/run-llama/notebookllama/commit/4b55abeefa9151cd65995aa3a32627e4030f306f) refactor: reorganize embedding config into modular structure - [`9df958f`](https://github.com/run-llama/notebookllama/commit/9df958ff53ba9212ba77b7f9edb0050bf7d7e175) fix: theme toggle - [`7e0ce17`](https://github.com/run-llama/notebookllama/commit/7e0ce171fc99e6fe49556b39649c822beeca5355) refactor: remove unused default() function, add proper type hints and dodcumentation - [`40b944d`](https://github.com/run-llama/notebookllama/commit/40b944d3fd96a0578d7428193a9add950befdd65) refactor: restructure embedding cli from single to multi-provider architecture - [`e760d29`](https://github.com/run-llama/notebookllama/commit/e760d295752751d1b2c9949ef963e695e3fec6ac) refactor: remove reference to "other" embedding since we arent currently supporting that - [`7730a58`](https://github.com/run-llama/notebookllama/commit/7730a5892cde73938c3832500670dbfa96c0689b) fix: add base screen bindings back to configuration screen ### 📊 Changes **21 files changed** (+650 additions, -476 deletions) <details> <summary>View changed files</summary> 📝 `pyproject.toml` (+1 -1) ➕ `tools/cli/config/__init__.py` (+3 -0) ➕ `tools/cli/config/models.py` (+12 -0) ➕ `tools/cli/embedding_app.py` (+40 -0) ➕ `tools/cli/screens/__init__.py` (+5 -0) ➕ `tools/cli/screens/base.py` (+60 -0) ➕ `tools/cli/screens/embedding_provider.py` (+62 -0) ➕ `tools/cli/screens/embedding_providers/__init__.py` (+15 -0) ➕ `tools/cli/screens/embedding_providers/azure.py` (+44 -0) ➕ `tools/cli/screens/embedding_providers/bedrock.py` (+79 -0) ➕ `tools/cli/screens/embedding_providers/cohere.py` (+49 -0) ➕ `tools/cli/screens/embedding_providers/gemini.py` (+53 -0) ➕ `tools/cli/screens/embedding_providers/huggingface.py` (+56 -0) ➕ `tools/cli/screens/embedding_providers/openai.py` (+55 -0) ➕ `tools/cli/screens/initial.py` (+44 -0) ➕ `tools/cli/stylesheets/base.tcss` (+24 -0) ➖ `tools/cli/stylesheets/input.tcss` (+0 -19) ➖ `tools/cli/stylesheets/select.tcss` (+0 -19) 📝 `tools/cli/utils.py` (+9 -274) 📝 `tools/create_llama_cloud_index.py` (+38 -162) _...and 1 more files_ </details> ### 📄 Description Hi! 👋 Thanks for implementing the embedding provider CLI from #12! I took a shot at refactoring the structure to improve the maintainability and extensibility. I think this is an important refactor to allow for easier embedding model extensions so that `notebookllama` can eventually support any embedding model. ## What this refactor accomplishes **Architecture Configuration** - Introduced `BaseScreen` and `ConfigurationScreen` base classes - All providers now inherit common functionality - Eliminated code duplication across provider implementations - Utilize one unified app within Textual instead of quitting and re-building between each page. **Unified UX** - Consistent keyboard shortcuts and styling across all providers - replace `ctrl+q` binding with a more standardized `shift+enter` when submitting - Proper error handling with user-friendly notifications - Password masking and input validation **Maintainability** - Adding new providers now just requires extending the base class - Simplified the main pipeline creation logic significantly - Clear separation of concerns between UI and business logic **Enhanced Features** - Error handling with try/catch blocks - Environment variable fallbacks (OpenAI API key detection) - Dynamic model fetching for Bedrock (will continue to add more model fetching for others) - Consolidated CSS styling system The functionality remains almost identical, but the code is now more maintainable and extensible. Thanks again for the solid foundation! 🚀 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-02-16 03:17:23 -05:00
yindo closed this issue 2026-02-16 03:17:23 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
bug
enhancement
help wanted
help wanted
pull-request
Mirrored from GitHub Pull Request
 triage
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: run-llama/notebookllama#28
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?