Fix/docker frontend build#90
Open
T-DevH wants to merge 7 commits into
Open
Conversation
- Embeddings: llama-nemotron-embed-vl-1b-v2 (2048), EMBEDDING_DIMENSION, Milvus - LLM defaults: integrate.api.nvidia.com/v1, llama-3.3-nemotron-super-49b-v1.5 - Docs and .env.example; guardrails and judge fallbacks aligned - UI: ESLint 8 pin, devDependency eslint, drop react-app/jest extend - Add recreate_milvus_collections.py; CI LLM_NIM_URL https
fix(ui): regenerate package-lock.json for npm ci with React 19 and MU…
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Attribute assignment immediately overwritten with raw parameters
- Removed lines 47-48 that were overwriting the correctly resolved model_name and dimension values with raw None parameters.
- ✅ Fixed: Env var renamed in example but not in code
- Updated large_llm_judge.py to read LLAMA_49B_TIMEOUT instead of LLAMA_70B_TIMEOUT to match the .env.example configuration.
Or push these changes by commenting:
@cursor push 9ed83ba2cc
Preview (9ed83ba2cc)
diff --git a/src/api/agents/document/validation/large_llm_judge.py b/src/api/agents/document/validation/large_llm_judge.py
--- a/src/api/agents/document/validation/large_llm_judge.py
+++ b/src/api/agents/document/validation/large_llm_judge.py
@@ -64,9 +64,8 @@
# Use LLM_MODEL from .env (49B model on NVIDIA public cloud)
self.model = os.getenv("LLM_MODEL", "nvidia/llama-3.3-nemotron-super-49b-v1.5")
# Large LLM (49B) models need more time for complex evaluation prompts
- # Default: 120 seconds (2 minutes), configurable via LLAMA_70B_TIMEOUT env var
- # Note: Environment variable name kept as LLAMA_70B_TIMEOUT for backward compatibility
- self.timeout = int(os.getenv("LLAMA_70B_TIMEOUT", "120"))
+ # Default: 120 seconds (2 minutes), configurable via LLAMA_49B_TIMEOUT env var
+ self.timeout = int(os.getenv("LLAMA_49B_TIMEOUT", "120"))
async def initialize(self):
"""Initialize the Large LLM Judge."""
diff --git a/src/retrieval/vector/embedding_service.py b/src/retrieval/vector/embedding_service.py
--- a/src/retrieval/vector/embedding_service.py
+++ b/src/retrieval/vector/embedding_service.py
@@ -44,8 +44,6 @@
):
self.model_name = model_name or os.getenv("EMBEDDING_MODEL", "nvidia/llama-nemotron-embed-vl-1b-v2")
self.dimension = dimension if dimension is not None else int(os.getenv("EMBEDDING_DIMENSION", "2048"))
- self.model_name = model_name
- self.dimension = dimension
self._initialized = False
self.nim_client = NoneThis Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8c5e4f9. Configure here.
| dimension: int | None = None, | ||
| ): | ||
| self.model_name = model_name or os.getenv("EMBEDDING_MODEL", "nvidia/llama-nemotron-embed-vl-1b-v2") | ||
| self.dimension = dimension if dimension is not None else int(os.getenv("EMBEDDING_DIMENSION", "2048")) |
There was a problem hiding this comment.
Attribute assignment immediately overwritten with raw parameters
High Severity
Lines 45–46 correctly resolve model_name and dimension with env-var fallbacks, but lines 47–48 immediately overwrite self.model_name and self.dimension with the raw parameter values. When get_embedding_service() calls EmbeddingService() with no arguments, both attributes end up as None, breaking downstream calls (e.g., generate_embedding passes None as the model, and get_dimension() returns None).
Reviewed by Cursor Bugbot for commit 8c5e4f9. Configure here.
| # Default: 120 seconds (2 minutes) | ||
| # Increase for complex documents that need more processing time | ||
| LLAMA_70B_TIMEOUT=120 | ||
| LLAMA_49B_TIMEOUT=120 |
There was a problem hiding this comment.
Env var renamed in example but not in code
Medium Severity
The .env.example renames the timeout variable from LLAMA_70B_TIMEOUT to LLAMA_49B_TIMEOUT, but large_llm_judge.py still reads LLAMA_70B_TIMEOUT via os.getenv. Users following the example will set LLAMA_49B_TIMEOUT, which has no effect — the code will silently fall back to the 120-second default, making the timeout non-configurable through the documented variable name.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8c5e4f9. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR standardizes NVIDIA NIM configuration (models/endpoints) across backend, docs, and UI, migrates embeddings to a new 2048-dim model (with Milvus dimension wiring + a collection recreation tool), and improves frontend Docker/CI build determinism.
Changes:
- Switch default NIM LLM endpoint/model to NVIDIA public cloud (
integrate.api.nvidia.com/v1,nvidia/llama-3.3-nemotron-super-49b-v1.5) and update guardrails/judge/docs accordingly. - Migrate embeddings to
nvidia/llama-nemotron-embed-vl-1b-v2(2048-dim) and make Milvus dimension configurable viaEMBEDDING_DIMENSION, including a script to drop affected collections. - Improve frontend build reliability by pinning Node image version, copying
.npmrcinto Docker builds, usingnpm ci, and adjusting ESLint config.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_embedding.py | Updates embedding test to new model + asserts 2048 dimension. |
| src/ui/web/src/pages/Documentation.tsx | UI docs updated to new embedding model name. |
| src/ui/web/src/pages/DocumentExtraction.tsx | Pipeline stage label updated to new embedding model. |
| src/ui/web/package.json | Adds deps/overrides and adjusts ESLint config for build stability. |
| src/ui/web/.eslintrc.js | Simplifies ESLint extends to avoid jest/globals env error. |
| src/retrieval/vector/milvus_retriever.py | Reads Milvus vector dimension from EMBEDDING_DIMENSION. |
| src/retrieval/vector/gpu_milvus_retriever.py | Reads GPU Milvus vector dimension from EMBEDDING_DIMENSION. |
| src/retrieval/vector/embedding_service.py | Makes embedding model/dimension configurable via env/defaults. |
| src/api/services/llm/nim_client.py | Updates default LLM endpoint + LLM/embedding model defaults. |
| src/api/services/guardrails/guardrails_service.py | Updates default guardrails model and related comments. |
| src/api/agents/document/validation/large_llm_judge.py | Updates judge defaults to NVIDIA public cloud endpoint/model. |
| src/api/agents/document/processing/embedding_indexing.py | Updates embedding model references + uses EMBEDDING_DIMENSION for mock vectors. |
| src/api/agents/document/preprocessing/nemo_retriever.py | Updates fallback model identifier. |
| src/api/agents/document/mcp_document_agent.py | Updates pipeline stage label to new embedding model. |
| src/api/agents/document/document_extraction_agent.py | Updates pipeline stage label to new embedding model. |
| src/api/agents/document/action_tools.py | Updates pipeline stage label to new embedding model. |
| scripts/tools/recreate_milvus_collections.py | Adds tool to drop Milvus collections for embedding dimension migration. |
| docs/architecture/mcp-gpu-acceleration-guide.md | Updates documented embedding dimension to 2048. |
| docs/architecture/diagrams/warehouse-operational-assistant.md | Updates architecture diagram/model references + endpoints text. |
| docs/architecture/adr/002-nvidia-nims-integration.md | Updates ADR to new embeddings model/dimension and LLM model id. |
| data/postgres/002_document_schema.sql | Updates comment documenting embedding model name stored in DB. |
| data/config/guardrails/rails.yaml | Updates guardrails main/embedding model identifiers. |
| data/config/guardrails/config.yml | Updates guardrails main/embedding model identifiers. |
| README.md | Updates README feature text and governing terms model link. |
| Dockerfile.frontend | Pins Node image version and copies .npmrc before npm ci. |
| Dockerfile | Pins Node image version, copies .npmrc, disables ESLint plugin during build. |
| DEPLOYMENT.md | Updates deployment docs to new endpoints/models and embedding NIM image/model. |
| .github/workflows/main.yml | Uses https endpoint and switches frontend install to npm ci with cleanup. |
| .env.example | Updates env defaults for new endpoints/models and embedding dimension. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
47
to
48
| self.model_name = model_name | ||
| self.dimension = dimension |
| # Default: 120 seconds (2 minutes) | ||
| # Increase for complex documents that need more processing time | ||
| LLAMA_70B_TIMEOUT=120 | ||
| LLAMA_49B_TIMEOUT=120 |
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
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.
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Fixes #(issue number)
Changes Made
Testing
Screenshots (if applicable)
Add screenshots to help explain your changes.
Checklist
Deployment Notes
Any special deployment considerations or environment variable changes.
Breaking Changes
If this is a breaking change, describe the impact and migration path.
Note
Medium Risk
Medium risk because it changes the embedding model/dimension (1024→2048), which requires Milvus collection recreation and could break retrieval if not migrated, and it updates default NIM endpoints/models used across LLM/guardrails/document pipeline.
Overview
Standardizes NIM usage on NVIDIA public cloud by switching default
LLM_NIM_URLtohttps://integrate.api.nvidia.com/v1and updating default LLM/guardrails model identifiers tonvidia/llama-3.3-nemotron-super-49b-v1.5(removing Brev-specific guidance).Migrates embeddings from
nv-embedqa-e5-v5(1024-d) tonvidia/llama-nemotron-embed-vl-1b-v2(2048-d) by addingEMBEDDING_MODEL/EMBEDDING_DIMENSION, wiring Milvus retrievers and document indexing to read the dimension from env, updating guardrails configs/docs, and addingscripts/tools/recreate_milvus_collections.pyto drop/recreate affected Milvus collections for the dimension change.Improves frontend build reliability by pinning Node to
20.19.0-alpine, copying.npmrcinto Docker builds, disabling ESLint during image build, switching CI fromnpm installtonpm ciwith cache/permission cleanup, and simplifying.eslintrc.jsto avoid thejest/globalsenv error.Reviewed by Cursor Bugbot for commit 8c5e4f9. Bugbot is set up for automated code reviews on this repo. Configure here.