feat: add llamacloud to llama_index_server #530
Conversation
|
WalkthroughThis pull request introduces the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (46)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (15)
llama-index-server/llama_index/server/services/llamacloud/generate.py (3)
23-27: Consider more flexible default directory handling.The function uses
data_dir or "data"which assumes a "data" directory exists in the current working directory.- data_dir or "data", + data_dir or "./data",You might also want to check if the directory exists before attempting to read from it:
import os if data_dir is None: data_dir = "./data" if not os.path.exists(data_dir): logger.warning(f"Default directory {data_dir} does not exist")
41-48: Consider adding more context to the error log.When calling
add_file_to_pipeline, the error handling could benefit from more context about the file that failed.LlamaCloudFileService.add_file_to_pipeline( index.project.id, index.pipeline.id, f, custom_metadata={}, wait_for_processing=False, ) except Exception as e: error_files.append(input_file) - logger.error(f"Error adding file {input_file}: {e}") + logger.error(f"Error adding file {input_file} to pipeline {index.pipeline.id}: {str(e)}")
30-36: Use file counts in progress bar.The progress bar could be enhanced by showing the total number of files being processed.
# add each file to the LlamaCloud pipeline error_files = [] for input_file in tqdm( files_to_process, desc="Processing files", unit="file", + total=len(files_to_process), ):llama-index-server/llama_index/server/api/routers/chat.py (1)
27-30: Addbackground_tasksargument tochat
Includingbackground_taskshere enables asynchronous processing without blocking the main request. Consider updating the docstring ofchatto reflect this parameter’s usage.llama-index-server/llama_index/server/api/callbacks/llamacloud.py (4)
1-11: Unused logger
The top-level imports look fine. However, theloggerdefined at line 10 is never used. You might consider removing it or using it within method implementations for better error logging.
13-17: Class docstring
Documentation provides an overview of this callback’s purpose. Consider expanding it to mention howbackground_tasksis utilized for asynchronous file download processing.
22-28: Event handling logic inrun
The approach of checkingevent.to_response()and filtering for"type": "sources"is straightforward. Logging or debugging statements could be beneficial if errors occur while processing events.
29-36: Usecontextlib.suppress(ImportError)
Catching and passingImportErroris valid, but consider usingcontextlib.suppress(ImportError)to make the code cleaner and more expressive:- try: - LlamaCloudFileService.download_files_from_nodes( - source_nodes, self.background_tasks - ) - except ImportError: - pass + from contextlib import suppress + with suppress(ImportError): + LlamaCloudFileService.download_files_from_nodes( + source_nodes, self.background_tasks + )🧰 Tools
🪛 Ruff (0.8.2)
30-35: Use
contextlib.suppress(ImportError)instead oftry-except-passReplace with
contextlib.suppress(ImportError)(SIM105)
llama-index-server/llama_index/server/services/llamacloud/index.py (3)
71-89: Definition ofIndexConfig
Having a dedicated class to bundleLlamaCloudConfigand an optionalCallbackManageris clear. Consider clarifying the aliasllamaCloudPipelineif it might cause confusion to users.
91-108:get_indexfunction
Catching a broadValueErrorto detect a missing index is functional but may mask other validation issues (e.g., invalid credentials). Narrowing or differentiating error handling could improve diagnostics.
115-151:_create_indexfunction
Logic for creating or updating the pipeline is clear. You might consider adding error handling or logging aroundupsert_pipelineto detect potential failures.llama-index-server/llama_index/server/services/llamacloud/file.py (4)
1-19: Use a more specific logger name.Using
"uvicorn"as the logger name could cause confusion since it's not tied to your custom services. Consider using a more self-documenting logger name such as__name__or"llama_index.server.services.llamacloud.file".
35-59: Consider returning a structured model instead of List[Dict].Returning a
List[Dict[str, Any]]can hurt maintainability and type safety. Defining a dedicated pydantic model (e.g.,ProjectWithPipelines) may make the code easier to understand and validate.
60-99: Make the wait and retry mechanism more flexible.Right now, the code waits for up to 2 seconds total (20 attempts × 0.1s) before raising an exception for incomplete file processing. For larger files or slower ingestion, this may be insufficient. Consider making the maximum wait time or the retry frequency configurable, possibly using a backoff strategy to handle slower uploads gracefully.
Do you want help adding a backoff approach or making the retry parameters configurable?
168-179: Add request timeouts or size checks for robust downloads.Using
requests.get(url, stream=True)without a timeout or validations might risk hanging indefinitely or writing partial data if the server stalls. Setting a timeout and validating the downloaded content size or integrity can strengthen reliability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
llama-index-server/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
llama-index-server/llama_index/server/api/callbacks/__init__.py(2 hunks)llama-index-server/llama_index/server/api/callbacks/llamacloud.py(1 hunks)llama-index-server/llama_index/server/api/routers/chat.py(3 hunks)llama-index-server/llama_index/server/server.py(2 hunks)llama-index-server/llama_index/server/services/llamacloud/__init__.py(1 hunks)llama-index-server/llama_index/server/services/llamacloud/file.py(1 hunks)llama-index-server/llama_index/server/services/llamacloud/generate.py(1 hunks)llama-index-server/llama_index/server/services/llamacloud/index.py(1 hunks)llama-index-server/pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
llama-index-server/llama_index/server/api/callbacks/__init__.py (1)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py (1)
LlamaCloudFileDownload(13-41)
llama-index-server/llama_index/server/services/llamacloud/generate.py (1)
llama-index-server/llama_index/server/services/llamacloud/file.py (2)
LlamaCloudFileService(35-178)add_file_to_pipeline(61-99)
llama-index-server/llama_index/server/services/llamacloud/file.py (1)
llama-index-server/llama_index/server/services/llamacloud/index.py (1)
get_client(110-112)
🪛 Ruff (0.8.2)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py
30-35: Use contextlib.suppress(ImportError) instead of try-except-pass
Replace with contextlib.suppress(ImportError)
(SIM105)
⏰ Context from checks skipped due to timeout of 90000ms (46)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (19)
llama-index-server/pyproject.toml (1)
38-38: Dependency addition looks good.The addition of
llama-index-indices-managed-llama-cloud = "^0.6.3"aligns with the PR's objective of adding LlamaCloud functionality to the server.llama-index-server/llama_index/server/api/callbacks/__init__.py (2)
2-2: Import statement looks good.The import statement for
LlamaCloudFileDownloadfrom the llamacloud module is correctly added.
12-12: Addition to all is correct.The
LlamaCloudFileDownloadclass is properly added to the__all__list, making it available through the public API.llama-index-server/llama_index/server/services/llamacloud/generate.py (3)
11-16: Function signature looks good.The
load_to_llamacloudfunction has appropriate parameters with sensible defaults and proper type annotations.
17-19: Logger initialization is handled properly.The function correctly creates a default logger if none is provided.
53-54: Good error summary logging.Providing a summary of all files that failed to process is a helpful feature.
llama-index-server/llama_index/server/services/llamacloud/__init__.py (1)
1-11: Module exports look good.The
__init__.pyfile correctly imports and exports all necessary components for the LlamaCloud functionality.llama-index-server/llama_index/server/api/routers/chat.py (3)
5-5: New import for background tasks
ImportingBackgroundTasksfrom FastAPI is appropriate here and aligns well with the asynchronous file handling needs in this module.
14-14: Introduced LlamaCloudFileDownload import
This import is consistent with the new callback approach for handling file downloads in background tasks.
44-44: Callback usage
AddingLlamaCloudFileDownload(background_tasks)effectively integrates asynchronous file download handling into the chat workflow. Implementation looks correct and consistent.llama-index-server/llama_index/server/server.py (1)
26-26: Changed default foruse_default_routers
Defaultinguse_default_routerstoTruechanges the server’s initialization behavior to include default routers automatically, which may affect existing configurations. Verify that this update won't introduce unintended routing for legacy setups.llama-index-server/llama_index/server/api/callbacks/llamacloud.py (2)
19-21: Constructor forLlamaCloudFileDownload
The constructor correctly storesbackground_tasksfor subsequent use. No issues spotted here.
37-41: Convenience constructor
The class methodfrom_defaultis a neat shortcut for creating an instance with aBackgroundTasksobject. No issues found.llama-index-server/llama_index/server/services/llamacloud/index.py (2)
21-63:LlamaCloudConfiginitialization and validation
This configuration class reads environment variables and validates required fields. The approach is sound, ensuring necessary credentials are set before runtime.
110-113:get_clientfunction
Straightforward use ofllama_cloud_get_clientwith environment-based config. No issues observed.llama-index-server/llama_index/server/services/llamacloud/file.py (4)
20-33: Data model implementation looks solid.The
LlamaCloudFileclass is well-defined, with clear equality and hashing support. This should simplify handling deduplicated file objects within sets.
102-133: Raise an exception on missing files to avoid silent failures.If no matching file name is found when iterating over pipeline files, the function simply exits without raising an error, leading to potential silent failures. Consider adding an exception if the file is not located or logging at
logger.errorlevel instead oflogger.infofor the caught exception.
134-141: Evaluate concurrent downloads and resource usage.Adding a background task to download each file is convenient, but if many files share the same name or if there's a high volume of downloads, concurrency might cause issues (e.g., race conditions over file writes or resource exhaustion). Verify that concurrency limits or checks are in place according to your system’s capacity.
143-158: Logical approach to collecting files for download.The
_get_files_to_downloadmethod cleanly filters out nodes missing pipeline or file metadata, returns a deduplicated set ofLlamaCloudFileobjects, and integrates well with the Pydantic model. Great job!
There was a problem hiding this comment.
The idea is moving the LlamaCloud code to the server, then we can simplify the logic in the generated code. Also, it's always included in the server package, we can also eliminate the ugly code that checks if the LlamaCloud service is importable in the old code.
leehuwuj
force-pushed
the
lee/add-llamacloud-support
branch
from
db0065f to
982b388
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
♻️ Duplicate comments (2)
llama-index-server/llama_index/server/server.py (1)
53-53:⚠️ Potential issueLogic always sets
use_default_routersto TrueThe expression
use_default_routers or Truewill always evaluate toTrue, ignoring the user-provided argument. This is likely a bug.Apply this fix:
- self.use_default_routers = use_default_routers or True + self.use_default_routers = True if use_default_routers is None else use_default_routersllama-index-server/llama_index/server/services/llamacloud/file.py (1)
159-165:⚠️ Potential issuePrevent possible directory traversal attacks
Currently, file names containing path separators (e.g.,
"../../etc/passwd") or other special characters could bypass the intended download path. Consider sanitizingfile_namebefore formatting it into the storage path.Apply this fix:
@classmethod def _get_file_name(cls, name: str, pipeline_id: str) -> str: + # Sanitize the filename to prevent directory traversal + safe_name = os.path.basename(name) - return cls.DOWNLOAD_FILE_NAME_TPL.format(pipeline_id=pipeline_id, filename=name) + return cls.DOWNLOAD_FILE_NAME_TPL.format(pipeline_id=pipeline_id, filename=safe_name) @classmethod def _get_file_path(cls, name: str, pipeline_id: str) -> str: return os.path.join(cls.LOCAL_STORE_PATH, cls._get_file_name(name, pipeline_id))
🧹 Nitpick comments (6)
llama-index-server/llama_index/server/api/routers/chat.py (2)
65-68: Add error handling for LlamaCloud API key retrievalInstead of directly accessing the environment variable, consider using a more robust method or providing a more descriptive error message.
- if not os.getenv("LLAMA_CLOUD_API_KEY"): + llama_cloud_api_key = os.getenv("LLAMA_CLOUD_API_KEY") + if not llama_cloud_api_key: raise HTTPException( - status_code=500, detail="LlamaCloud API KEY is not configured" + status_code=500, detail="LlamaCloud API KEY is not configured in environment variables" )
65-81: Consider adding response validation and error handlingThe function should validate the response from
get_all_projects_with_pipelines()and handle potential exceptions for a more robust implementation.if not os.getenv("LLAMA_CLOUD_API_KEY"): raise HTTPException( status_code=500, detail="LlamaCloud API KEY is not configured" ) - projects = LlamaCloudFileService.get_all_projects_with_pipelines() - pipeline = os.getenv("LLAMA_CLOUD_INDEX_NAME") - project = os.getenv("LLAMA_CLOUD_PROJECT_NAME") - pipeline_config = None - if pipeline and project: - pipeline_config = { - "pipeline": pipeline, - "project": project, - } - return { - "projects": projects, - "pipeline": pipeline_config, - } + try: + projects = LlamaCloudFileService.get_all_projects_with_pipelines() + pipeline = os.getenv("LLAMA_CLOUD_INDEX_NAME") + project = os.getenv("LLAMA_CLOUD_PROJECT_NAME") + pipeline_config = None + if pipeline and project: + pipeline_config = { + "pipeline": pipeline, + "project": project, + } + return { + "projects": projects, + "pipeline": pipeline_config, + } + except Exception as e: + logger.error(f"Error fetching LlamaCloud configuration: {e}") + raise HTTPException( + status_code=500, detail=f"Error fetching LlamaCloud configuration: {str(e)}" + )llama-index-server/llama_index/server/services/llamacloud/file.py (4)
39-57: Add pagination support for large numbers of projects and pipelinesThe current implementation might not handle large numbers of projects and pipelines efficiently. Consider adding pagination support.
@classmethod def get_all_projects_with_pipelines(cls) -> List[Dict[str, Any]]: try: client = get_client() - projects = client.projects.list_projects() - pipelines = client.pipelines.search_pipelines() + # Get projects with pagination if supported + projects = client.projects.list_projects() + + # Get pipelines with pagination if supported + pipelines = client.pipelines.search_pipelines() + + # Consider adding limit/offset or pagination if the API supports it return [ { **(project.dict()), "pipelines": [ {"id": p.id, "name": p.name} for p in pipelines if p.project_id == project.id ], } for project in projects ] except Exception as error: logger.error(f"Error listing projects and pipelines: {error}") return []
83-98: Improve polling mechanism for file processing statusThe current polling mechanism uses a fixed sleep time. Consider implementing an exponential backoff strategy for more efficient polling.
# Wait 2s for the file to be processed max_attempts = 20 attempt = 0 + base_sleep_time = 0.1 while attempt < max_attempts: result = client.pipelines.get_pipeline_file_status( file_id=file_id, pipeline_id=pipeline_id ) if result.status == ManagedIngestionStatus.ERROR: raise Exception(f"File processing failed: {str(result)}") if result.status == ManagedIngestionStatus.SUCCESS: # File is ingested - return the file id return file_id attempt += 1 - time.sleep(0.1) # Sleep for 100ms + # Exponential backoff with jitter + sleep_time = min(base_sleep_time * (2 ** attempt), 1.0) + time.sleep(sleep_time) raise Exception( f"File processing did not complete after {max_attempts} attempts." )
167-177: Add request timeout and additional error handling for file downloadsThe current implementation doesn't have a timeout for the HTTP request, which could cause the process to hang if the server is slow to respond.
@classmethod def _download_file(cls, url: str, local_file_path: str) -> None: logger.info(f"Saving file to {local_file_path}") # Create directory if it doesn't exist os.makedirs(cls.LOCAL_STORE_PATH, exist_ok=True) # Download the file - with requests.get(url, stream=True) as r: - r.raise_for_status() - with open(local_file_path, "wb") as f: - for chunk in r.iter_content(chunk_size=8192): - f.write(chunk) - logger.info("File downloaded successfully") + try: + with requests.get(url, stream=True, timeout=30) as r: + r.raise_for_status() + with open(local_file_path, "wb") as f: + for chunk in r.iter_content(chunk_size=8192): + f.write(chunk) + logger.info("File downloaded successfully") + except requests.exceptions.Timeout: + logger.error(f"Timeout while downloading file from {url}") + raise + except requests.exceptions.RequestException as e: + logger.error(f"Error downloading file: {e}") + raise
179-184: Add logging for configuration statusConsider adding debug logging to help with troubleshooting configuration issues.
@classmethod def is_configured(cls) -> bool: try: - return os.environ.get("LLAMA_CLOUD_API_KEY") is not None + api_key = os.environ.get("LLAMA_CLOUD_API_KEY") + is_configured = api_key is not None + if is_configured: + logger.debug("LlamaCloud API key is configured") + else: + logger.debug("LlamaCloud API key is not configured") + return is_configured except Exception: + logger.exception("Error checking LlamaCloud configuration") return False
🛑 Comments failed to post (1)
llama-index-server/llama_index/server/api/routers/chat.py (1)
62-81: 💡 Verification agent
🧩 Analysis chain
Fix formatting issues flagged by the linter
The pipeline is failing with "Ruff formatting check failed". While the logic is correct, you need to run the formatter to fix the style issues.
🏁 Script executed:
#!/bin/bash # Description: Check the failing formatting issue ruff format --diff llama-index-server/llama_index/server/api/routers/chat.pyLength of output: 544
Fix Ruff Formatting Issues in the Chat Router File
The linter diff confirms that
llama-index-server/llama_index/server/api/routers/chat.pyis not fully formatted. Ruff reports that one file would be reformatted (an extra newline is being added). Although the code logic is correct, please run the auto-formatter (e.g., withruff format) to apply the proper style updates and resolve the "Ruff formatting check failed" error.
- Ensure the file is updated by applying the suggested formatting changes.
- Re-run the pipeline checks after formatting to confirm that the linter issues are resolved.
leehuwuj
force-pushed
the
lee/add-llamacloud-support
branch
from
982b388 to
5350c0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
llama-index-server/llama_index/server/api/routers/chat.py (3)
62-83: New LlamaCloud configuration endpoint follows good practicesThe new endpoint is properly conditionally registered only when LlamaCloud is configured, and includes appropriate error handling for missing API keys. The endpoint returns a well-structured response with project and pipeline information.
A few suggestions to enhance the implementation:
@router.get("/config/llamacloud") async def chat_llama_cloud_config() -> dict: - if not os.getenv("LLAMA_CLOUD_API_KEY"): + if not LlamaCloudFileService.is_configured(): raise HTTPException( status_code=500, detail="LlamaCloud API KEY is not configured" ) projects = LlamaCloudFileService.get_all_projects_with_pipelines() pipeline = os.getenv("LLAMA_CLOUD_INDEX_NAME") project = os.getenv("LLAMA_CLOUD_PROJECT_NAME") pipeline_config = None if pipeline and project: pipeline_config = { "pipeline": pipeline, "project": project, } return { "projects": projects, "pipeline": pipeline_config, }This reuses the existing
is_configured()method for consistent configuration checking.
44-47: Consider adding a conditional check for LlamaCloud callbackCurrently, the
LlamaCloudFileDownloadcallback is always added to the callbacks list, but it might be better to only add it when LlamaCloud is properly configured.callbacks: list[EventCallback] = [ SourceNodesFromToolCall(), - LlamaCloudFileDownload(background_tasks), ] +if LlamaCloudFileService.is_configured(): + callbacks.append(LlamaCloudFileDownload(background_tasks))This would prevent unnecessary callback processing when LlamaCloud is not configured.
66-69: Consider returning 403 for API key errors instead of 500Using a 500 status code for a missing API key implies a server error, but this is more of an authorization/configuration issue. A 403 Forbidden status would be more appropriate.
if not os.getenv("LLAMA_CLOUD_API_KEY"): raise HTTPException( - status_code=500, detail="LlamaCloud API KEY is not configured" + status_code=403, detail="LlamaCloud API KEY is not configured" )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
llama-index-server/llama_index/server/api/models.py(1 hunks)llama-index-server/llama_index/server/api/routers/chat.py(4 hunks)llama-index-server/llama_index/server/server.py(3 hunks)llama-index-server/llama_index/server/services/llamacloud/file.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- llama-index-server/llama_index/server/api/models.py
🚧 Files skipped from review as they are similar to previous changes (2)
- llama-index-server/llama_index/server/server.py
- llama-index-server/llama_index/server/services/llamacloud/file.py
🧰 Additional context used
🧬 Code Definitions (1)
llama-index-server/llama_index/server/api/routers/chat.py (3)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py (1)
LlamaCloudFileDownload(13-41)llama-index-server/llama_index/server/api/models.py (1)
ChatRequest(29-37)llama-index-server/llama_index/server/services/llamacloud/file.py (3)
LlamaCloudFileService(34-184)is_configured(180-184)get_all_projects_with_pipelines(39-57)
⏰ Context from checks skipped due to timeout of 90000ms (46)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (3)
llama-index-server/llama_index/server/api/routers/chat.py (3)
3-3: Import additions look good for LlamaCloud integrationThe new imports include required dependencies for LlamaCloud functionality:
osfor environment variables,BackgroundTasksfrom FastAPI, and the LlamaCloud-specific imports for file handling.Also applies to: 6-6, 15-16, 19-19
29-32: Well-structured function signature update for background task supportThe addition of the
background_tasksparameter to the chat function is a good implementation that follows FastAPI's pattern for handling background operations. This allows file downloads to happen asynchronously without blocking the main response.
46-46: LlamaCloud file download callback properly integratedThe
LlamaCloudFileDownloadcallback is correctly instantiated with thebackground_tasksparameter and added to the callbacks list. This enables asynchronous file download capabilities from LlamaCloud source nodes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py (2)
9-9: Logger initialization check.Creating a logger with
"uvicorn"is a common practice within FastAPI services. Make sure log statements are used consistently throughout to troubleshoot issues with file downloads effectively.
27-34: Consider usingcontextlib.suppress(ImportError).The try-except block that silently passes on
ImportErrorcould be replaced with a context manager for clarity:+ from contextlib import suppress async def _process_response_nodes(self, source_nodes: List[NodeWithScore]) -> None: - try: - LlamaCloudFileService.download_files_from_nodes( - source_nodes, self.background_tasks - ) - except ImportError: - pass + with suppress(ImportError): + LlamaCloudFileService.download_files_from_nodes(source_nodes, self.background_tasks)This change boosts readability without altering functionality.
🧰 Tools
🪛 Ruff (0.8.2)
28-33: Use
contextlib.suppress(ImportError)instead oftry-except-passReplace with
contextlib.suppress(ImportError)(SIM105)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py
28-33: Use contextlib.suppress(ImportError) instead of try-except-pass
Replace with contextlib.suppress(ImportError)
(SIM105)
⏰ Context from checks skipped due to timeout of 90000ms (46)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (6)
llama-index-server/llama_index/server/api/callbacks/llamacloud.py (6)
1-2: Imports look fine.The usage of
loggingand typing-related imports (Any,List) is correct and necessary for the functionality of this module.
4-7: Good use of dependencies.Importing
BackgroundTasks,NodeWithScore, and the baseEventCallbacklooks appropriate. TheLlamaCloudFileServiceimport clarifies where file download logic lives.
12-16: Clear class documentation.The docstring clearly states its purpose as a processor for file downloads from LlamaCloud source nodes. This clarifies the class responsibilities.
17-19: Dependency injection practice.Injecting
BackgroundTasksensures that the file download jobs can be dispatched asynchronously. This helps keep the main request flow unblocked.
20-25: Overall logic for handling events.The check for a “sources” type response and the presence of
.nodesis a concise approach to conditionally triggering the download logic. This fits well for handling the specialized scenario of source retrieval.
35-39: Clear factory method.The
from_defaultclassmethod offers a convenient way to instantiate the callback by simply passingBackgroundTasks. This is helpful for places where a lightweight construction pattern is desired.
leehuwuj
force-pushed
the
lee/add-llamacloud-support
branch
from
c42327e to
e19fa71
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
llama-index-server/README.md (5)
3-4: Introduction Clarity and Punctuation Improvement
The introductory text clearly outlines that LlamaIndexServer is a FastAPI-based application with an optional chat UI. For enhanced readability, consider adding a comma after “API endpoints” (e.g., “...with both API endpoints, and a user interface for interaction”). Also, if LlamaCloud features (such as asynchronous file downloads) are part of this integration, you might want to briefly reference them here for a more complete overview.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...(AI_HYDRA_LEO_MISSING_COMMA)
5-10: Enhance Consistency in the Features List
The new "Features" section is well organized; however, to improve consistency you might consider hyphenating compound adjectives. For example, “high performance” could be written as “high-performance” API development, which aids clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...(EN_COMPOUND_ADJECTIVE_INTERNAL)
69-81: Comprehensive Configuration Options Section
This section provides a detailed list of configuration parameters for the server. For consistency and improved readability, review the punctuation in the bullet list (e.g., consider whether to end each bullet with a period or no punctuation). The clarity here is excellent, but aligning the style with other documentation parts might further improve the reader’s experience.🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: -workflow_factory: A callable that creates a workflow inst...(UNLIKELY_OPENING_PUNCTUATION)
82-101: Detailed Default Routers and Features Documentation
The new “Default Routers and Features” section is very comprehensive. It covers chat routing, static file serving, and the chat UI neatly. Since this PR introduces LlamaCloud functionality, you might consider adding a subsection or a note here that briefly describes LlamaCloud’s file download capabilities and any related endpoints if they affect the server’s behavior.
109-115: API Endpoints Section is Well Outlined
The section on API endpoints provides a concise overview of the available endpoints. If LlamaCloud introduces any new endpoints (for example, for file downloads), remember to update this section to include them.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
llama-index-server/README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
llama-index-server/README.md
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: - workflow_factory: A callable that creates a workflow inst...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~116-~116: Loose punctuation mark.
Context: ...lowing default endpoints: - /api/chat: Chat interaction endpoint - `/api/files...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (42)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (8)
llama-index-server/README.md (8)
18-18: Renaming 'Usage' to 'Quick Start' Improves Clarity
The section header “## Quick Start” clearly sets the tone for a fast onboarding process. This renaming from “Usage” makes the documentation more accessible and immediately action-oriented.
42-48: API Server Initialization with New Parameters
The example instantiation ofLlamaIndexServernow includes the new parameters (workflow_factory,env,include_ui,starter_questions, andverbose), which is clearly commented. This provides a good reference for users. If the new LlamaCloud file download feature is part of this PR, consider adding a brief note or reference here (or in a dedicated section) to highlight its integration.
52-59: Streamlined 'Running the Server' Instructions
The “Running the Server” section now presents concise instructions, which makes it easy for users to quickly start the server. The command example and layout are clear.
60-64: Clear API Request Example Provided
The curl command example is straightforward and useful for users to verify the chat endpoint. Just ensure that if any new authentication or parameters (possibly related to LlamaCloud) are introduced in future iterations, this example is updated accordingly.
67-68: Informative Chat UI Access Note
The note on accessing the chat UI is clear and provides important information regarding the configuration prerequisites (env="dev"orinclude_ui=True).
102-108: Clear Overview of Development Mode Enhancements
The “Development Mode” section effectively details the benefits available during development (CORS enablement, auto-inclusion of the chat UI, and verbose logging). This is clear and useful for troubleshooting during development.
116-123: Good Practices Section Provides Valuable Guidance
The “Best Practices” section offers practical tips for using the server. One suggestion is to consider adding guidance related to any new features (like LlamaCloud callbacks) if they require special configuration or usage considerations. Also, ensure consistent punctuation in the list as noted by static analysis.🧰 Tools
🪛 LanguageTool
[uncategorized] ~116-~116: Loose punctuation mark.
Context: ...lowing default endpoints: -/api/chat: Chat interaction endpoint - `/api/files...(UNLIKELY_OPENING_PUNCTUATION)
124-127: Helpful 'Getting Started' Section
The “Getting Started with a New Project” section is a useful addition—it points users to the create-llama tool for rapid project initiation. The external link is a good touch; just verify periodically that the link remains active.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
llama-index-server/llama_index/server/services/llamacloud/index.py (2)
123-159:⚠️ Potential issue_create_index has potential issues with embedding model validation and environment variables.
This function has several concerns:
- It imports OpenAIEmbedding inside the function, which should be moved to the top level
- It directly accesses environment variables which may not be set
- The embedding model check may not be sufficient if Settings.embed_model is None
+from llama_index.embeddings.openai import OpenAIEmbedding + def _create_index( config: IndexConfig, ) -> None: client = get_client() pipeline_name = config.llama_cloud_pipeline_config.pipeline pipelines = client.pipelines.search_pipelines( pipeline_name=pipeline_name, pipeline_type=PipelineType.MANAGED.value, ) if len(pipelines) == 0: - from llama_index.embeddings.openai import OpenAIEmbedding - - if not isinstance(Settings.embed_model, OpenAIEmbedding): + embed_model = Settings.embed_model + if embed_model is None or not isinstance(embed_model, OpenAIEmbedding): raise ValueError( "Creating a new pipeline with a non-OpenAI embedding model is not supported." ) + + # Get environment variables with appropriate fallbacks + openai_api_key = os.getenv("OPENAI_API_KEY") + embedding_model = os.getenv("EMBEDDING_MODEL", "text-embedding-ada-002") + + if not openai_api_key: + raise ValueError("OPENAI_API_KEY environment variable is required to create a pipeline") + client.pipelines.upsert_pipeline( request={ "name": pipeline_name, "embedding_config": { "type": "OPENAI_EMBEDDING", "component": { - "api_key": os.getenv("OPENAI_API_KEY"), # editable - "model_name": os.getenv("EMBEDDING_MODEL"), + "api_key": openai_api_key, # editable + "model_name": embedding_model, }, },
1-159: 💡 Verification agent🧩 Analysis chain
Code formatting issue needs to be fixed.
The pipeline failure indicates that the file needs formatting with 'ruff format'. Please run this tool to address the style issues.
🏁 Script executed:
#!/bin/bash # Check what formatting issues exist in the file ruff format --check llama-index-server/llama_index/server/services/llamacloud/index.py --diffLength of output: 1242
Action Required: Correct Code Formatting with Ruff
The file still shows formatting discrepancies as detected by ruff. Specifically, lines in the
from_chat_requestmethod should be reformatted as follows:
- Before:
default_config.llama_cloud_pipeline_config.pipeline = llamacloud_config["pipeline"] default_config.llama_cloud_pipeline_config.project = llamacloud_config["project"]- After:
default_config.llama_cloud_pipeline_config.pipeline = llamacloud_config[ "pipeline" ] default_config.llama_cloud_pipeline_config.project = llamacloud_config[ "project" ]Please run
ruff formatto apply these changes and ensure consistent formatting across the codebase.🧰 Tools
🪛 GitHub Actions: Lint on push or pull request
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'ruff format' to fix code style issues in this file.
🧹 Nitpick comments (4)
llama-index-server/llama_index/server/services/llamacloud/index.py (4)
1-19: Import structure looks good, but could be organized better.The imports are comprehensive and include all necessary dependencies for LlamaCloud integration. However, they could be organized into standard library, third-party, and local imports groups for better readability.
- import logging - import os - from typing import TYPE_CHECKING, Any, Optional - - from llama_cloud import PipelineType - from llama_index.core.callbacks import CallbackManager - from llama_index.core.ingestion.api_utils import ( - get_client as llama_cloud_get_client, - ) - from llama_index.core.settings import Settings - from llama_index.indices.managed.llama_cloud import LlamaCloudIndex - from llama_index.server.api.models import ChatRequest - from pydantic import BaseModel, Field, field_validator + # Standard library imports + import logging + import os + from typing import TYPE_CHECKING, Any, Optional + + # Third-party imports + from pydantic import BaseModel, Field, field_validator + from llama_cloud import PipelineType + + # LlamaIndex imports + from llama_index.core.callbacks import CallbackManager + from llama_index.core.ingestion.api_utils import ( + get_client as llama_cloud_get_client, + ) + from llama_index.core.settings import Settings + from llama_index.indices.managed.llama_cloud import LlamaCloudIndex + from llama_index.server.api.models import ChatRequest🧰 Tools
🪛 GitHub Actions: Lint on push or pull request
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'ruff format' to fix code style issues in this file.
21-69: LlamaCloudConfig is well-designed with proper environment variable fallbacks.The class handles configuration gracefully with environment variables as fallbacks and appropriate field validation. The
exclude=Truefor sensitive fields is a good security practice.However, consider adding docstrings to improve code documentation and help users understand the purpose of this class.
class LlamaCloudConfig(BaseModel): + """Configuration for LlamaCloud services. + + This class manages API key, base URL, organization ID, pipeline name, + and project name, with environment variable fallbacks. + """ # Private attributes api_key: str = Field( exclude=True, # Exclude from the model representation )
54-63: Field validation might show incorrect error message.The error message mentions only some of the required environment variables, but the validator is applied to all fields including
api_key. This might confuse users if other fields are missing.@field_validator("pipeline", "project", "api_key", mode="before") @classmethod def validate_fields(cls, value: Any) -> Any: if value is None: raise ValueError( - "Please set LLAMA_CLOUD_INDEX_NAME, LLAMA_CLOUD_PROJECT_NAME and LLAMA_CLOUD_API_KEY" - " to your environment variables or config them in .env file" + "Missing required configuration value. Please set all required " + "LlamaCloud environment variables (LLAMA_CLOUD_API_KEY, LLAMA_CLOUD_INDEX_NAME, " + "LLAMA_CLOUD_PROJECT_NAME) in your environment or .env file" ) return value
100-116: Good error handling in get_index, but needs more specific logging.The function handles index retrieval and creation with appropriate error handling. However, the error logging is vague and doesn't specify which index wasn't found or what error occurred.
def get_index( chat_request: Optional[ChatRequest] = None, create_if_missing: bool = False, ) -> Optional[LlamaCloudIndex]: config = IndexConfig.from_chat_request(chat_request) # Check whether the index exists try: index = LlamaCloudIndex(**config.to_index_kwargs()) return index - except ValueError: - logger.warning("Index not found") + except ValueError as e: + pipeline_name = config.llama_cloud_pipeline_config.pipeline + project_name = config.llama_cloud_pipeline_config.project + logger.warning(f"Index not found: pipeline={pipeline_name}, project={project_name}. Error: {str(e)}") if create_if_missing: - logger.info("Creating index") + logger.info(f"Creating index: pipeline={pipeline_name}, project={project_name}") _create_index(config) return LlamaCloudIndex(**config.to_index_kwargs()) return None
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
llama-index-server/llama_index/server/api/routers/chat.py(3 hunks)llama-index-server/llama_index/server/services/llamacloud/index.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- llama-index-server/llama_index/server/api/routers/chat.py
🧰 Additional context used
🪛 GitHub Actions: Lint on push or pull request
llama-index-server/llama_index/server/services/llamacloud/index.py
[error] 1-1: Ruff formatting check failed. 1 file would be reformatted. Please run 'ruff format' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 90000ms (46)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (1)
llama-index-server/llama_index/server/services/llamacloud/index.py (1)
118-121: get_client function is minimal and effective.This function provides a clean interface to get a LlamaCloud client with the configured settings.
llama-index-server/llama_index/server/services/llamacloud/index.py
Outdated
Show resolved
Hide resolved
leehuwuj
force-pushed
the
lee/add-llamacloud-support
branch
from
7963e12 to
d2c27cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
llama-index-server/llama_index/server/services/llamacloud/index.py (1)
91-102:⚠️ Potential issueIndexConfig.from_chat_request lacks error handling.
The method assumes that
chat_request.data["llamaCloudPipeline"]and its nested fields exist without proper error checking, which could lead to KeyError or AttributeError.@classmethod def from_chat_request(cls, chat_request: ChatRequest) -> "IndexConfig": default_config = cls() if chat_request is not None: - llamacloud_config = chat_request.data["llamaCloudPipeline"] - if llamacloud_config is not None: - default_config.llama_cloud_pipeline_config.pipeline = llamacloud_config[ - "pipeline" - ] - default_config.llama_cloud_pipeline_config.project = llamacloud_config[ - "project" - ] + llamacloud_config = chat_request.data.get("llamaCloudPipeline") + if llamacloud_config is not None: + pipeline = llamacloud_config.get("pipeline") + project = llamacloud_config.get("project") + if pipeline: + default_config.llama_cloud_pipeline_config.pipeline = pipeline + if project: + default_config.llama_cloud_pipeline_config.project = project return default_config
🧹 Nitpick comments (5)
llama-index-server/llama_index/server/services/llamacloud/index.py (5)
54-62: Improve field validation flexibility.The field validator checks only for
Nonevalues but doesn't validate empty strings, which could lead to invalid configurations.@field_validator("pipeline", "project", "api_key", mode="before") @classmethod def validate_fields(cls, value: Any) -> Any: - if value is None: + if not value: # This will catch None, empty strings, and other falsy values raise ValueError( "Please set LLAMA_CLOUD_INDEX_NAME, LLAMA_CLOUD_PROJECT_NAME and LLAMA_CLOUD_API_KEY" " to your environment variables or config them in .env file" ) return value
111-120: Improve error handling in get_index function.The function only catches
ValueErrorexceptions, which could miss other exceptions that might occur during index creation.try: index = LlamaCloudIndex(**config.to_index_kwargs()) return index -except ValueError: +except (ValueError, Exception) as e: - logger.warning("Index not found") + logger.warning(f"Error when getting index: {str(e)}") if create_if_missing: logger.info("Creating index") _create_index(config) - return LlamaCloudIndex(**config.to_index_kwargs()) + try: + return LlamaCloudIndex(**config.to_index_kwargs()) + except Exception as e: + logger.error(f"Failed to create index: {str(e)}") + return None return None
134-137: Add error handling for client.pipelines.search_pipelines.The API call to search pipelines doesn't have error handling, which could lead to unhandled exceptions if the LlamaCloud service is unavailable.
- pipelines = client.pipelines.search_pipelines( - pipeline_name=pipeline_name, - pipeline_type=PipelineType.MANAGED.value, - ) + try: + pipelines = client.pipelines.search_pipelines( + pipeline_name=pipeline_name, + pipeline_type=PipelineType.MANAGED.value, + ) + except Exception as e: + logger.error(f"Failed to search pipelines: {str(e)}") + raise ValueError(f"Failed to access LlamaCloud service: {str(e)}")
145-163: Add error handling for pipeline creation.The client API call to create the pipeline doesn't have error handling, which could lead to unhandled exceptions if the creation fails.
- client.pipelines.upsert_pipeline( - request={ - "name": pipeline_name, - "embedding_config": { - "type": "OPENAI_EMBEDDING", - "component": { - "api_key": os.getenv("OPENAI_API_KEY"), # editable - "model_name": os.getenv("EMBEDDING_MODEL"), - }, - }, - "transform_config": { - "mode": "auto", - "config": { - "chunk_size": Settings.chunk_size, # editable - "chunk_overlap": Settings.chunk_overlap, # editable - }, - }, - }, - ) + try: + client.pipelines.upsert_pipeline( + request={ + "name": pipeline_name, + "embedding_config": { + "type": "OPENAI_EMBEDDING", + "component": { + "api_key": os.getenv("OPENAI_API_KEY"), # editable + "model_name": os.getenv("EMBEDDING_MODEL"), + }, + }, + "transform_config": { + "mode": "auto", + "config": { + "chunk_size": Settings.chunk_size, # editable + "chunk_overlap": Settings.chunk_overlap, # editable + }, + }, + }, + ) + logger.info(f"Successfully created pipeline: {pipeline_name}") + except Exception as e: + logger.error(f"Failed to create pipeline: {str(e)}") + raise ValueError(f"Failed to create pipeline: {str(e)}")
140-144: Improve error message for unsupported embedding model.The error message could be more informative by suggesting what embedding models are supported.
if not isinstance(Settings.embed_model, OpenAIEmbedding): raise ValueError( - "Creating a new pipeline with a non-OpenAI embedding model is not supported." + "Creating a new pipeline with a non-OpenAI embedding model is not supported. " + "Please configure an OpenAI embedding model in your Settings." )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
llama-index-server/llama_index/server/api/routers/chat.py(3 hunks)llama-index-server/llama_index/server/services/llamacloud/index.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- llama-index-server/llama_index/server/api/routers/chat.py
⏰ Context from checks skipped due to timeout of 90000ms (46)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
llama-index-server/llama_index/server/services/llamacloud/index.py (2)
90-102:⚠️ Potential issueIndexConfig.from_chat_request lacks error handling
The method assumes that
chat_request.data["llamaCloudPipeline"]and its nested fields exist without proper error checking, which could lead to KeyError or AttributeError.@classmethod def from_chat_request(cls, chat_request: ChatRequest) -> "IndexConfig": default_config = cls() - if chat_request.data is not None: - llamacloud_config = chat_request.data.get("llamaCloudPipeline") - if llamacloud_config is not None: - default_config.llama_cloud_pipeline_config.pipeline = llamacloud_config[ - "pipeline" - ] - default_config.llama_cloud_pipeline_config.project = llamacloud_config[ - "project" - ] + if chat_request.data is not None: + llamacloud_config = chat_request.data.get("llamaCloudPipeline") + if llamacloud_config is not None: + pipeline = llamacloud_config.get("pipeline") + project = llamacloud_config.get("project") + if pipeline: + default_config.llama_cloud_pipeline_config.pipeline = pipeline + if project: + default_config.llama_cloud_pipeline_config.project = project return default_config
154-155:⚠️ Potential issueAdd environment variable validation for OpenAI configuration
The code uses environment variables without checking if they're set, which could lead to unexpected behavior if they're missing.
"embedding_config": { "type": "OPENAI_EMBEDDING", "component": { - "api_key": os.getenv("OPENAI_API_KEY"), # editable - "model_name": os.getenv("EMBEDDING_MODEL"), + "api_key": os.getenv("OPENAI_API_KEY") or Settings.openai_api_key, # Try both sources + "model_name": os.getenv("EMBEDDING_MODEL") or "text-embedding-ada-002", # Provide a default }, },
🧹 Nitpick comments (4)
llama-index-server/llama_index/server/services/llamacloud/index.py (4)
40-51: Consider using a centralized configuration approachThe current implementation directly accesses environment variables in the constructor. Consider implementing a centralized configuration management approach (e.g., using a Settings class or dependency injection) to improve maintainability and testability.
- def __init__(self, **kwargs: Any) -> None: - if "api_key" not in kwargs: - kwargs["api_key"] = os.getenv("LLAMA_CLOUD_API_KEY") - if "base_url" not in kwargs: - kwargs["base_url"] = os.getenv("LLAMA_CLOUD_BASE_URL") - if "organization_id" not in kwargs: - kwargs["organization_id"] = os.getenv("LLAMA_CLOUD_ORGANIZATION_ID") - if "pipeline" not in kwargs: - kwargs["pipeline"] = os.getenv("LLAMA_CLOUD_INDEX_NAME") - if "project" not in kwargs: - kwargs["project"] = os.getenv("LLAMA_CLOUD_PROJECT_NAME") - super().__init__(**kwargs) + def __init__(self, **kwargs: Any) -> None: + # Get configuration from centralized settings + config = self._get_config() + # Override with provided kwargs + for key in ["api_key", "base_url", "organization_id", "pipeline", "project"]: + if key not in kwargs: + kwargs[key] = config.get(key) + super().__init__(**kwargs) + + @classmethod + def _get_config(cls) -> dict: + """Get configuration from environment variables or other sources""" + return { + "api_key": os.getenv("LLAMA_CLOUD_API_KEY"), + "base_url": os.getenv("LLAMA_CLOUD_BASE_URL"), + "organization_id": os.getenv("LLAMA_CLOUD_ORGANIZATION_ID"), + "pipeline": os.getenv("LLAMA_CLOUD_INDEX_NAME"), + "project": os.getenv("LLAMA_CLOUD_PROJECT_NAME"), + }
58-61: Improve error message specificityThe current error message doesn't indicate which specific field is missing. Consider enhancing the error message to include the name of the missing field for easier troubleshooting.
- @field_validator("pipeline", "project", "api_key", mode="before") - @classmethod - def validate_fields(cls, value: Any) -> Any: - if value is None: - raise ValueError( - "Please set LLAMA_CLOUD_INDEX_NAME, LLAMA_CLOUD_PROJECT_NAME and LLAMA_CLOUD_API_KEY" - " to your environment variables or config them in .env file" - ) - return value + @field_validator("pipeline", "project", "api_key", mode="before") + @classmethod + def validate_fields(cls, value: Any, info: ValidationInfo) -> Any: + if value is None: + field_name = info.field_name + env_var_name = { + "pipeline": "LLAMA_CLOUD_INDEX_NAME", + "project": "LLAMA_CLOUD_PROJECT_NAME", + "api_key": "LLAMA_CLOUD_API_KEY", + }.get(field_name, field_name.upper()) + raise ValueError( + f"Missing required field '{field_name}'. Please set {env_var_name}" + " in your environment variables or .env file" + ) + return value
158-164: Consider exposing transform configuration optionsThe transform configuration uses hardcoded values like
"mode": "auto"alongside settings fromSettings. Consider making all these configuration options explicit and configurable for better flexibility."transform_config": { - "mode": "auto", + "mode": os.getenv("LLAMA_CLOUD_TRANSFORM_MODE", "auto"), "config": { "chunk_size": Settings.chunk_size, # editable "chunk_overlap": Settings.chunk_overlap, # editable + # Consider adding more configurable transform options }, },
137-141: Enhance pipeline search error handlingThe pipeline search doesn't handle potential API errors or rate limiting. Consider adding try-except blocks to handle different types of errors gracefully.
- pipelines = client.pipelines.search_pipelines( - pipeline_name=pipeline_name, - pipeline_type=PipelineType.MANAGED.value, - ) - if len(pipelines) == 0: + try: + pipelines = client.pipelines.search_pipelines( + pipeline_name=pipeline_name, + pipeline_type=PipelineType.MANAGED.value, + ) + if len(pipelines) == 0: + # Continue with pipeline creation... + except Exception as e: + logger.error(f"Error searching for pipelines: {str(e)}") + raise ValueError(f"Failed to check if pipeline exists: {str(e)}")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
llama-index-server/llama_index/server/api/models.py(2 hunks)llama-index-server/llama_index/server/services/llamacloud/index.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
llama-index-server/llama_index/server/services/llamacloud/index.py (1)
llama-index-server/llama_index/server/api/models.py (1)
ChatRequest(29-38)
⏰ Context from checks skipped due to timeout of 90000ms (46)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (6)
llama-index-server/llama_index/server/api/models.py (2)
9-9: Import organization improvedThe reorganization of Pydantic imports (
BaseModel,Field,field_validator) before application-specific imports follows standard import ordering conventions, improving code organization.
31-31: New field added to support LlamaCloud integrationAdding the optional
datafield toChatRequestprovides a flexible extension point for passing LlamaCloud configuration and other arbitrary data through the API. The field is properly typed asOptional[Any]with a default ofNone, ensuring backward compatibility.llama-index-server/llama_index/server/services/llamacloud/index.py (4)
21-69: Well-structured LlamaCloudConfig class with proper environment variable fallbacksThe
LlamaCloudConfigclass appropriately handles configuration through constructor parameters with environment variable fallbacks. The field validation ensures required fields are set, preventing runtime errors later in the execution path.
114-123: Good error handling in get_index functionThe implementation properly handles potential
ValueErrorexceptions when attempting to get an index, with appropriate logging and the option to create the index if it's missing. This makes the function more robust and user-friendly.
144-147: Good validation of embedding model compatibilityThe code properly checks if the current embedding model is an instance of
OpenAIEmbeddingbefore attempting to create a pipeline, preventing potential compatibility issues with different embedding models.
126-128: Simple and effective client initializationThe
get_clientfunction provides a clean, reusable way to initialize the LlamaCloud client with the appropriate configuration, promoting code reuse.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
llama-index-server/README.md (1)
1-130: 🛠️ Refactor suggestionDocumentation Completeness – Include New LlamaCloud Feature:
While the README has been significantly updated with sections on features, quick start, configuration, and more, it does not reference the newly added LlamaCloud file download functionality. Given that this feature is a key objective of the PR, consider adding a dedicated bullet point under "## Features" (or an additional section) that briefly describes theLlamaCloudFileDownloadclass, its purpose, and its integration into the server (e.g., asynchronous file downloads via background tasks).🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: -workflow_factory: A callable that creates a workflow inst...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~93-~93: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rkflows fromcreate-llama(see below) are following this pattern. ### Chat UI When enable...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~115-~115: Loose punctuation mark.
Context: ...lowing default endpoints: -/api/chat: Chat interaction endpoint - `/api/files...(UNLIKELY_OPENING_PUNCTUATION)
🧹 Nitpick comments (9)
llama-index-server/README.md (5)
3-3: Feature Highlight Missing:
The introductory paragraph describes the server’s capabilities well, but there is no mention of the new LlamaCloud file download functionality (i.e. theLlamaCloudFileDownloadclass) introduced by this PR. Consider adding a brief note or section that highlights this new feature and its intended use.
5-10: Hyphenate Compound Adjectives in Bullet Points:
In the Features section, the bullet point
- Built on FastAPI for high performance and easy API development
would read better (and align with best practices) by hyphenating the compound adjective "high performance" to "high-performance".
For example, you could apply the following diff:- - Built on FastAPI for high performance and easy API development + - Built on FastAPI for high-performance and easy API developmentThis change improves readability and adheres to style guidelines.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...(EN_COMPOUND_ADJECTIVE_INTERNAL)
69-82: Punctuation Consistency in Configuration Options:
The list under "## Configuration Options" could benefit from consistent punctuation. For instance, the bullet forworkflow_factoryis written as:- `workflow_factory`: A callable that creates a workflow instance for each requestConsider adding terminal punctuation (such as a period) to each bullet for consistency and improved clarity. A suggested diff would be:
- - `workflow_factory`: A callable that creates a workflow instance for each request + - `workflow_factory`: A callable that creates a workflow instance for each request.Feel free to apply similar adjustments to the subsequent list items.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: -workflow_factory: A callable that creates a workflow inst...(UNLIKELY_OPENING_PUNCTUATION)
89-94: Verb Tense Adjustment for Clarity:
In the Static File Serving section, the sentence:The example workflows from `create-llama` (see below) are following this pattern.would read more succinctly as:
The example workflows from `create-llama` (see below) follow this pattern.This simple adjustment enhances clarity. A proposed diff:
- The example workflows from `create-llama` (see below) are following this pattern. + The example workflows from `create-llama` (see below) follow this pattern.🧰 Tools
🪛 LanguageTool
[uncategorized] ~93-~93: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rkflows fromcreate-llama(see below) are following this pattern. ### Chat UI When enable...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
115-118: Consistent Punctuation for API Endpoint Descriptions:
The bullet item for the API endpoints, for example:- `/api/chat`: Chat interaction endpointwould benefit from ending with a period for consistency (and to indicate completeness of the description). A sample diff is:
- - `/api/chat`: Chat interaction endpoint + - `/api/chat`: Chat interaction endpoint.Apply similar punctuation adjustments to the other bullet items if needed.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~115-~115: Loose punctuation mark.
Context: ...lowing default endpoints: -/api/chat: Chat interaction endpoint - `/api/files...(UNLIKELY_OPENING_PUNCTUATION)
llama-index-server/llama_index/server/services/llamacloud/index.py (4)
54-62: Make validation error messages more specificThe current error message assumes environment variables are the only way to configure these fields and doesn't indicate which specific field caused the validation failure. This could be confusing when configuring programmatically.
I recommend making the error messages more specific to each field:
@field_validator("pipeline", "project", "api_key", mode="before") @classmethod def validate_fields(cls, value: Any) -> Any: if value is None: - raise ValueError( - "Please set LLAMA_CLOUD_INDEX_NAME, LLAMA_CLOUD_PROJECT_NAME and LLAMA_CLOUD_API_KEY" - " to your environment variables or config them in .env file" - ) + field_name = cls.field_name # Get the current field name from context + env_var_mapping = { + "pipeline": "LLAMA_CLOUD_INDEX_NAME", + "project": "LLAMA_CLOUD_PROJECT_NAME", + "api_key": "LLAMA_CLOUD_API_KEY" + } + env_var = env_var_mapping.get(field_name, f"LLAMA_CLOUD_{field_name.upper()}") + raise ValueError( + f"Required field '{field_name}' is not set. " + f"You can set it via the {env_var} environment variable or pass it directly." + ) return value
105-121: Enhance error handling in get_index functionThe function only catches ValueError, but other exceptions could occur. Additionally, more detailed logging would be helpful for troubleshooting.
Consider catching more specific exceptions and improving logging:
def get_index( chat_request: Optional[ChatRequest] = None, create_if_missing: bool = False, ) -> Optional[LlamaCloudIndex]: config = IndexConfig.from_default(chat_request) # Check whether the index exists try: index = LlamaCloudIndex(**config.to_index_kwargs()) return index - except ValueError: - logger.warning("Index not found") + except ValueError as e: + logger.warning(f"Index not found: {str(e)}") + except Exception as e: + logger.error(f"Error retrieving index: {str(e)}") if create_if_missing: - logger.info("Creating index") + logger.info(f"Creating index with pipeline: {config.llama_cloud_pipeline_config.pipeline}") _create_index(config) - return LlamaCloudIndex(**config.to_index_kwargs()) + try: + return LlamaCloudIndex(**config.to_index_kwargs()) + except Exception as e: + logger.error(f"Failed to create index: {str(e)}") return None
139-145: Move import statement to the top of the fileImporting modules inside a function is generally discouraged unless there's a specific reason like avoiding circular imports.
import logging import os from typing import TYPE_CHECKING, Any, Optional from llama_cloud import PipelineType +from llama_index.embeddings.openai import OpenAIEmbedding from llama_index.core.callbacks import CallbackManager from llama_index.core.ingestion.api_utils import ( get_client as llama_cloud_get_client, ) from llama_index.core.settings import Settings from llama_index.indices.managed.llama_cloud import LlamaCloudIndex from llama_index.server.api.models import ChatRequest from pydantic import BaseModel, Field, field_validator # ... def _create_index( config: IndexConfig, ) -> None: client = get_client() pipeline_name = config.llama_cloud_pipeline_config.pipeline pipelines = client.pipelines.search_pipelines( pipeline_name=pipeline_name, pipeline_type=PipelineType.MANAGED.value, ) if len(pipelines) == 0: - from llama_index.embeddings.openai import OpenAIEmbedding - if not isinstance(Settings.embed_model, OpenAIEmbedding):
145-163: Add error handling for pipeline creationThe upsert_pipeline call lacks error handling, which could cause unhandled exceptions if the API call fails.
Add a try-except block around the pipeline creation API call:
if len(pipelines) == 0: from llama_index.embeddings.openai import OpenAIEmbedding if not isinstance(Settings.embed_model, OpenAIEmbedding): raise ValueError( "Creating a new pipeline with a non-OpenAI embedding model is not supported." ) - client.pipelines.upsert_pipeline( - request={ - "name": pipeline_name, - "embedding_config": { - "type": "OPENAI_EMBEDDING", - "component": { - "api_key": os.getenv("OPENAI_API_KEY"), # editable - "model_name": os.getenv("EMBEDDING_MODEL"), - }, - }, - "transform_config": { - "mode": "auto", - "config": { - "chunk_size": Settings.chunk_size, # editable - "chunk_overlap": Settings.chunk_overlap, # editable - }, - }, - }, - ) + try: + client.pipelines.upsert_pipeline( + request={ + "name": pipeline_name, + "embedding_config": { + "type": "OPENAI_EMBEDDING", + "component": { + "api_key": os.getenv("OPENAI_API_KEY") or Settings.openai_api_key, # editable + "model_name": os.getenv("EMBEDDING_MODEL") or "text-embedding-ada-002", + }, + }, + "transform_config": { + "mode": "auto", + "config": { + "chunk_size": Settings.chunk_size, # editable + "chunk_overlap": Settings.chunk_overlap, # editable + }, + }, + }, + ) + logger.info(f"Successfully created pipeline: {pipeline_name}") + except Exception as e: + logger.error(f"Failed to create pipeline: {str(e)}") + raise ValueError(f"Failed to create LlamaCloud pipeline: {str(e)}") from e
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
llama-index-server/README.md(2 hunks)llama-index-server/llama_index/server/services/llamacloud/index.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
llama-index-server/README.md
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: - workflow_factory: A callable that creates a workflow inst...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~93-~93: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rkflows from create-llama (see below) are following this pattern. ### Chat UI When enable...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~115-~115: Loose punctuation mark.
Context: ...lowing default endpoints: - /api/chat: Chat interaction endpoint - `/api/files...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (44)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (2)
llama-index-server/llama_index/server/services/llamacloud/index.py (2)
91-102: IndexConfig.from_default lacks error handlingThe method assumes that the "pipeline" and "project" keys exist in the llamacloud_config dictionary without checking, which could lead to KeyError.
I recommend using the
.get()method with default values to safely access dictionary items:@classmethod def from_default(cls, chat_request: Optional[ChatRequest] = None) -> "IndexConfig": default_config = cls() if chat_request is not None and chat_request.data is not None: llamacloud_config = chat_request.data.get("llamaCloudPipeline") if llamacloud_config is not None: - default_config.llama_cloud_pipeline_config.pipeline = llamacloud_config[ - "pipeline" - ] - default_config.llama_cloud_pipeline_config.project = llamacloud_config[ - "project" - ] + pipeline = llamacloud_config.get("pipeline") + project = llamacloud_config.get("project") + if pipeline: + default_config.llama_cloud_pipeline_config.pipeline = pipeline + if project: + default_config.llama_cloud_pipeline_config.project = project return default_config
151-152: Add environment variable validation for OpenAI configurationThe code uses environment variables without checking if they're set, which could lead to unexpected behavior if they're missing.
"embedding_config": { "type": "OPENAI_EMBEDDING", "component": { - "api_key": os.getenv("OPENAI_API_KEY"), # editable - "model_name": os.getenv("EMBEDDING_MODEL"), + "api_key": os.getenv("OPENAI_API_KEY") or Settings.openai_api_key, # Try both sources + "model_name": os.getenv("EMBEDDING_MODEL") or "text-embedding-ada-002", # Provide a default }, },
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
llama-index-server/llama_index/server/tools/document_generator.py (2)
107-161:⚠️ Potential issueMethod decorator inconsistency with actual usage.
The methods
_generate_html_content,_generate_pdf, and_generate_htmlare still decorated with@classmethodbut are now being called as instance methods from thegenerate_documentmethod. This inconsistency could lead to confusion or bugs.Consider updating these methods to be instance methods for consistency:
-@classmethod -def _generate_html_content(cls, original_content: str) -> str: +def _generate_html_content(self, original_content: str) -> str: """ Generate HTML content from the original markdown content. """ try: import markdown # type: ignore except ImportError: raise ImportError( "Failed to import required modules. Please install markdown." ) # Convert markdown to HTML with fenced code and table extensions return markdown.markdown(original_content, extensions=["fenced_code", "tables"]) -@classmethod -def _generate_pdf(cls, html_content: str) -> BytesIO: +def _generate_pdf(self, html_content: str) -> BytesIO: """ Generate a PDF from the HTML content. """ # Method implementation... -@classmethod -def _generate_html(cls, html_content: str) -> str: +def _generate_html(self, html_content: str) -> str: """ Generate a complete HTML document with the given HTML content. """ # Method implementation...🧰 Tools
🪛 Ruff (0.8.2)
115-117: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
130-132: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
203-227:⚠️ Potential issueInconsistent method type for static methods.
The methods
_write_to_fileand_validate_file_nameare still defined as static methods but are now called as instance methods. This inconsistency should be addressed.Consider updating these methods to be instance methods for consistency:
-@staticmethod -def _write_to_file(content: BytesIO, file_path: str) -> None: +def _write_to_file(self, content: BytesIO, file_path: str) -> None: """ Write the content to a file. """ # Method implementation... -@staticmethod -def _validate_file_name(file_name: str) -> str: +def _validate_file_name(self, file_name: str) -> str: """ Validate the file name. """ # Method implementation...
♻️ Duplicate comments (1)
llama-index-server/llama_index/server/server.py (1)
57-57:⚠️ Potential issueFix the assignment of
use_default_routersparameterThe expression
use_default_routers or Truewill always evaluate toTrue, ignoring the user-provided argument. This is likely a bug and would override any explicitFalsevalue passed by users.- self.use_default_routers = use_default_routers or True + self.use_default_routers = True if use_default_routers is None else use_default_routers
🧹 Nitpick comments (9)
llama-index-server/pyproject.toml (3)
36-36: New Dependency:pydantic-settingsThe addition of
pydantic-settings = "^2.8.1"supports enhanced configuration management, which is aligned with the new asynchronous file download functionality. Please verify that its integration is correctly documented in related modules and configuration files.
39-39: New Dependency:llama-index-indices-managed-llama-cloudThis new dependency (
llama-index-indices-managed-llama-cloud = "^0.6.3") is likely required to support the managed llama cloud index functionality. Ensure that its usage is consistent across the project and that relevant documentation is updated.
64-64: New Dependency:llama-cloudAdding
llama-cloud = "^0.1.17"fulfills the requirements for the new asynchronous file download capabilities via theLlamaCloudFileDownloadclass. Verify that any configuration or initialization needing this package is updated accordingly.llama-index-server/llama_index/server/services/file.py (1)
90-92: Use the new get_file_url method for URL construction.The file_url construction should use the newly added get_file_url method to avoid duplication and ensure consistent URL formatting.
- file_url = ( - f"{server_settings.file_server_url_prefix}/{save_dir}/{new_file_name}" - ) + file_url = cls.get_file_url(new_file_name, save_dir)llama-index-server/llama_index/server/tools/interpreter.py (1)
52-53: Consider removing duplicate default values.The class has default values defined both as class variables (lines 30-31) and in the constructor. This could lead to confusion if one is updated but not the other.
- output_dir = "output/tools" - uploaded_files_dir = "output/uploaded"Or update the constructor to use the class variables:
- self.output_dir = output_dir or "output/tools" - self.uploaded_files_dir = uploaded_files_dir or "output/uploaded" + self.output_dir = output_dir or self.output_dir + self.uploaded_files_dir = uploaded_files_dir or self.uploaded_files_dirllama-index-server/README.md (4)
3-3: Consider adding a comma for improved readability.The sentence is long and would benefit from a comma after "workflows" to separate the clauses.
-LlamaIndexServer is a FastAPI-based application that allows you to quickly launch your [LlamaIndex Workflows](https://docs.llamaindex.ai/en/stable/module_guides/workflow/#workflows) and [Agent Workflows](https://docs.llamaindex.ai/en/stable/understanding/agent/multi_agent/) as an API server with an optional chat UI. It provides a complete environment for running LlamaIndex workflows with both API endpoints and a user interface for interaction. +LlamaIndexServer is a FastAPI-based application that allows you to quickly launch your [LlamaIndex Workflows](https://docs.llamaindex.ai/en/stable/module_guides/workflow/#workflows) and [Agent Workflows](https://docs.llamaindex.ai/en/stable/understanding/agent/multi_agent/) as an API server with an optional chat UI. It provides a complete environment for running LlamaIndex workflows, with both API endpoints and a user interface for interaction.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...(AI_HYDRA_LEO_MISSING_COMMA)
7-10: Consider using hyphens for compound adjectives.For bullet points that contain compound adjectives modifying a noun, consider adding hyphens for better readability.
- Built on FastAPI for high performance and easy API development + Built on FastAPI for high-performance and easy API development🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...(EN_COMPOUND_ADJECTIVE_INTERNAL)
73-82: Format bullet points consistently.The bullet points in this section have extra spaces before them, unlike other bullet point sections in the document.
- - `workflow_factory`: A callable that creates a workflow instance for each request - - `logger`: Optional logger instance (defaults to uvicorn logger) - - `use_default_routers`: Whether to include default routers (chat, static file serving) - - `env`: Environment setting ('dev' enables CORS and UI by default) - - `include_ui`: Whether to include the chat UI - - `starter_questions`: List of starter questions for the chat UI - - `verbose`: Enable verbose logging - - `api_prefix`: API route prefix (default: "/api") - - `server_url`: The deployment URL of the server. - - `ui_path`: Path for downloaded UI static files (default: ".ui") +- `workflow_factory`: A callable that creates a workflow instance for each request +- `logger`: Optional logger instance (defaults to uvicorn logger) +- `use_default_routers`: Whether to include default routers (chat, static file serving) +- `env`: Environment setting ('dev' enables CORS and UI by default) +- `include_ui`: Whether to include the chat UI +- `starter_questions`: List of starter questions for the chat UI +- `verbose`: Enable verbose logging +- `api_prefix`: API route prefix (default: "/api") +- `server_url`: The deployment URL of the server. +- `ui_path`: Path for downloaded UI static files (default: ".ui")🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: -workflow_factory: A callable that creates a workflow inst...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...eployment URL of the server. -ui_path: Path for downloaded UI static files (de...(UNLIKELY_OPENING_PUNCTUATION)
110-113: Format bullet points consistently.Similar to the previous comment, these bullet points have extra spaces before them.
- - `/api/chat`: Chat interaction endpoint - - `/api/files/data/*`: Access to data directory files - - `/api/files/output/*`: Access to output directory files +- `/api/chat`: Chat interaction endpoint +- `/api/files/data/*`: Access to data directory files +- `/api/files/output/*`: Access to output directory files🧰 Tools
🪛 LanguageTool
[uncategorized] ~110-~110: Loose punctuation mark.
Context: ...lowing default endpoints: -/api/chat: Chat interaction endpoint - `/api/files...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
llama-index-server/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
llama-index-server/README.md(2 hunks)llama-index-server/llama_index/server/api/models.py(3 hunks)llama-index-server/llama_index/server/server.py(7 hunks)llama-index-server/llama_index/server/services/file.py(3 hunks)llama-index-server/llama_index/server/settings.py(1 hunks)llama-index-server/llama_index/server/tools/document_generator.py(3 hunks)llama-index-server/llama_index/server/tools/interpreter.py(2 hunks)llama-index-server/pyproject.toml(2 hunks)llama-index-server/tests/services/test_file_service.py(2 hunks)llama-index-server/tests/tools/test_document_generator.py(3 hunks)llama-index-server/tests/tools/test_interpreter.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- llama-index-server/llama_index/server/api/models.py
🧰 Additional context used
🧬 Code Definitions (4)
llama-index-server/tests/tools/test_interpreter.py (1)
llama-index-server/llama_index/server/tools/interpreter.py (1)
E2BCodeInterpreter(29-216)
llama-index-server/tests/tools/test_document_generator.py (1)
llama-index-server/llama_index/server/tools/document_generator.py (6)
_validate_file_name(216-227)_write_to_file(204-213)_generate_html_content(108-120)_generate_html(153-161)_generate_pdf(123-150)generate_document(163-201)
llama-index-server/llama_index/server/tools/document_generator.py (1)
llama-index-server/llama_index/server/settings.py (1)
file_server_url_prefix(16-17)
llama-index-server/llama_index/server/server.py (2)
llama-index-server/tests/test_llamaindex_server.py (1)
server(25-33)llama-index-server/llama_index/server/settings.py (3)
set_url(35-37)set_api_prefix(39-41)api_url(20-21)
🪛 LanguageTool
llama-index-server/README.md
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: - workflow_factory: A callable that creates a workflow inst...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...eployment URL of the server. - ui_path: Path for downloaded UI static files (de...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~110-~110: Loose punctuation mark.
Context: ...lowing default endpoints: - /api/chat: Chat interaction endpoint - `/api/files...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 GitHub Actions: Build Package
llama-index-server/tests/services/test_file_service.py
[error] 56-56: AssertionError: assert False. The result.url does not end with the expected path.
[error] 161-161: AssertionError: assert '/api/files/test_dir/test_12345678-1234-5678-1234-567812345678.txt' == '/api/files\test_dir\test_12345678-1234-5678-1234-567812345678.txt'. The URL format is inconsistent.
[warning] 1-1: PytestDeprecationWarning: The configuration option 'asyncio_default_fixture_loop_scope' is unset. Set the default fixture loop scope explicitly.
llama-index-server/llama_index/server/settings.py
[error] 37-41: Too few arguments for 'call' of '_OnlyValueValidatorClsMethod' and '_V1ValidatorWithValuesKwOnlyClsMethod'. Missing named argument 'values' for 'call' of '_V1ValidatorWithValuesKwOnlyClsMethod' and 'V1ValidatorWithValuesKwOnly'.
⏰ Context from checks skipped due to timeout of 90000ms (42)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
🔇 Additional comments (26)
llama-index-server/pyproject.toml (1)
37-37: Dependency Version Update:llama-index-coreThe dependency version has been updated from a caret version to a specific version (
"0.12.25"). Confirm that this precise version requirement does not introduce any breaking changes and that all modules depending on it remain compatible.llama-index-server/tests/tools/test_interpreter.py (1)
21-23: Good update to match the API changes.The test now directly passes the API key to the constructor rather than patching environment variables, which reflects the updated constructor signature in the implementation file.
llama-index-server/tests/services/test_file_service.py (1)
146-146: LGTM: Mock environment value updated correctly.The mock return value has been updated to match the new URL prefix pattern.
llama-index-server/llama_index/server/services/file.py (2)
8-8: Good import for centralized settings.Using server_settings is a good approach for centralized configuration.
103-111: Ensure URL path separators are consistent.The new method is good, but URLs should always use forward slashes regardless of the operating system. The current string formatting approach is correct, but be cautious of using
os.path.join()for URLs as it would use backslashes on Windows.llama-index-server/llama_index/server/tools/interpreter.py (3)
9-9: LGTM: Appropriate import.The import for BaseModel from pydantic is correctly placed.
36-39: Good API improvement requiring explicit API key.Making the API key a required parameter is a good design decision as it eliminates the implicit dependency on environment variables.
47-50: Good error message update.The error message has been appropriately updated to reflect that the API key must be passed explicitly.
llama-index-server/README.md (1)
18-124: Excellent documentation improvements!The documentation is now much more comprehensive and well-structured, making it easier for users to understand the server's features and configuration options. The addition of sections like "Features", "Configuration Options", "Best Practices", and "Getting Started" significantly improves the user experience.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: -workflow_factory: A callable that creates a workflow inst...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~82-~82: Loose punctuation mark.
Context: ...eployment URL of the server. -ui_path: Path for downloaded UI static files (de...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~110-~110: Loose punctuation mark.
Context: ...lowing default endpoints: -/api/chat: Chat interaction endpoint - `/api/files...(UNLIKELY_OPENING_PUNCTUATION)
llama-index-server/tests/tools/test_document_generator.py (6)
14-17: Clean refactoring of tests to use instance methods.The test has been properly updated to instantiate the DocumentGenerator with the new required parameter.
20-21: Appropriate error handling test update.The test for invalid filenames now correctly uses an instance of DocumentGenerator.
26-33: Correctly updated test for file writing.The test now properly calls the _write_to_file method on an instance rather than as a static method.
40-47: Well-structured HTML generation test updates.Tests for HTML content generation and full HTML generation have been updated to use instance methods consistently.
54-61: PDF generation tests correctly updated.Both success and error cases for PDF generation now use the instance method properly.
74-89: Document generation tests correctly updated.The tests for document generation have been properly updated to use the new instance method and verify the correct URL format.
llama-index-server/llama_index/server/settings.py (3)
15-22: Well-designed properties for URL construction.The properties for
file_server_url_prefixandapi_urlprovide a clean and consistent way to access these derived values throughout the application.
5-13: Good use of Pydantic settings with field descriptions.The ServerSettings class is well-structured with appropriate default values and helpful descriptions for each field.
23-33: Effective validators for URL format requirements.The validators ensure that the URL doesn't end with a slash and the API prefix starts with a slash, preventing common configuration issues.
llama-index-server/llama_index/server/tools/document_generator.py (3)
102-106: Well-designed initialization with parameter validation.The new init method properly requires and validates the file_server_url_prefix parameter, ensuring that instances are correctly configured.
163-201: Successfully converted to instance method with proper URL construction.The
generate_documentmethod has been properly converted to an instance method and now correctly constructs the URL using the instance's file_server_url_prefix.🧰 Tools
🪛 Ruff (0.8.2)
178-180: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
240-242: Properly implemented to_tool method.The
to_toolmethod correctly passes the instance method to FunctionTool.from_defaults.llama-index-server/llama_index/server/server.py (5)
59-63: LGTM: Properly handling server configuration overridesThe logic to update server settings when custom values are provided is well-implemented and follows the correct pattern of only updating settings when explicitly provided.
80-89: LGTM: LlamaCloud integration with UI configurationGood implementation of conditionally adding LlamaCloud API configuration when the API key is available. This maintains backward compatibility while enabling the new feature when properly configured.
106-106: LGTM: Consistent use of centralized API prefixUsing
server_settings.api_prefixfor router configuration ensures consistency across the application.
145-148: LGTM: Consistent path mounting for data directoryThe update to use
server_settings.api_prefixfor mounting the data directory ensures consistency with server configuration.
155-158: LGTM: Consistent path mounting for output directoryThe update to use
server_settings.api_prefixfor mounting the output directory ensures consistency with server configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
llama-index-server/README.md (5)
3-3: Punctuation Suggestion in the IntroductionThe introductory sentence on line 3 could benefit from a slight punctuation adjustment to enhance clarity. For example, adding a comma after “API server” would clearly separate the clauses.
-LlamaIndexServer is a FastAPI-based application that allows you to quickly launch your [LlamaIndex Workflows](...) and [Agent Workflows](...) as an API server with an optional chat UI. +LlamaIndexServer is a FastAPI-based application that allows you to quickly launch your [LlamaIndex Workflows](...) and [Agent Workflows](...) as an API server, with an optional chat UI.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...(AI_HYDRA_LEO_MISSING_COMMA)
8-8: Review Compound Adjective FormattingIn the Features section (line 8), the phrase “Built on FastAPI for high performance and easy API development” might be clearer if the compound adjectives are hyphenated (e.g. “high-performance” and “easy-to-use API development”). This can help avoid any ambiguity about which words modify the noun.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...(EN_COMPOUND_ADJECTIVE_INTERNAL)
42-49: Clarify Instantiation Example ConfigurationIn the instantiation example for
LlamaIndexServer(lines 42–49), you introduce several parameters (such asworkflow_factory,env,include_ui,starter_questions, andverbose). While the Configuration Options section later details additional parameters likeserver_url,api_prefix, andui_path, it might be beneficial to reference them here—either via inline comments or a note—to ensure that users are aware of all configurable options.
93-95: Verb Tense Adjustment in Default Routers SectionIn the Default Routers section, the sentence “The example workflows from
create-llama(see below) are following this pattern” could be made more concise by changing it to “follow this pattern.”- - The example workflows from `create-llama` (see below) are following this pattern. + - The example workflows from `create-llama` (see below) follow this pattern.🧰 Tools
🪛 LanguageTool
[uncategorized] ~94-~94: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rkflows fromcreate-llama(see below) are following this pattern. ### Chat UI When enable...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
115-118: Punctuation Consistency in API Endpoints ListThe API Endpoints section (lines 115–118) presents its items in a list. It would improve readability to review the punctuation and formatting so that each list entry is consistent and clear.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~116-~116: Loose punctuation mark.
Context: ...lowing default endpoints: -/api/chat: Chat interaction endpoint - `/api/files...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
llama-index-server/README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
llama-index-server/README.md
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...low as a chatbot - Built on FastAPI for high performance and easy API development - Optional bui...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: - workflow_factory: A callable that creates a workflow inst...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~94-~94: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...rkflows from create-llama (see below) are following this pattern. ### Chat UI When enable...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~116-~116: Loose punctuation mark.
Context: ...lowing default endpoints: - /api/chat: Chat interaction endpoint - `/api/files...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (44)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (2)
llama-index-server/README.md (2)
1-4: Update Documentation to Reflect New LlamaCloud FeaturesThe PR title mentions the addition of a
LlamaCloudFileDownloadfeature. However, the README currently does not reference this new functionality. Consider adding a dedicated section—or updating an existing one—to describe how LlamaCloud is integrated, provide usage examples, and explain any relevant configuration options.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...lete environment for running LlamaIndex workflows with both API endpoints and a user inte...(AI_HYDRA_LEO_MISSING_COMMA)
73-73: Revisit theuse_default_routersParameterThe configuration option for
use_default_routersis included on line 73. In previous reviews, feedback noted that folder mapping is workflow specific, suggesting the potential use of afile_serverparameter instead. Please clarify whetheruse_default_routersis still needed or update its description to better align with the folder mapping logic.🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: Loose punctuation mark.
Context: ...ration parameters: -workflow_factory: A callable that creates a workflow inst...(UNLIKELY_OPENING_PUNCTUATION)
Summary by CodeRabbit
New Features
Bug Fixes