Update Llama Stack to 0.2.22 #43
Conversation
WalkthroughAdds mypy config and relaxes dependency pins in Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Query Script
participant Retriever as LlamaIndex Retriever
participant Formatter as Output Formatter
note over CLI,Retriever: User issues RAG query
CLI->>Retriever: retrieve()
Retriever-->>CLI: nodes[]
alt single-node
CLI->>CLI: check node type
alt TextNode
CLI->>Formatter: format TextNode -> JSON/plain
Formatter-->>CLI: output
else not TextNode
CLI-->>CLI: log warning
CLI-->>CLI: return JSON error & exit(1)
end
else multi-node
CLI->>CLI: iterate nodes
CLI->>CLI: if NodeWithScore -> append
CLI-->>CLI: skip others (debug)
CLI->>Formatter: format list -> JSON/plain
Formatter-->>CLI: output
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
pyproject.toml (1)
40-52: Consider using compatible release constraints instead of minimum version constraints.The dependency specifications have been relaxed from exact pins (
==) to minimum version constraints (>=) for most packages. While this provides flexibility, it can introduce instability by allowing breaking changes from future releases to be pulled in automatically. This is especially risky for packages likellama-index,torch, andfaiss-cpu, which can have breaking API changes between releases.Consider using compatible release constraints (
~=) instead of minimum versions:- "PyYAML>=6.0.2", + "PyYAML~=6.0.2", "huggingface_hub>=0.33.4", - "llama-index>=0.12.51", - "llama-index-vector-stores-faiss>=0.3.0", - "llama-index-embeddings-huggingface>=0.4.0", - "llama-index-readers-file>=0.4.11", - "faiss-cpu>=1.11.0.post1", + "llama-index~=0.12.51", + "llama-index-vector-stores-faiss~=0.3.0", + "llama-index-embeddings-huggingface~=0.4.0", + "llama-index-readers-file~=0.4.11", + "faiss-cpu~=1.11.0", "llama-index-vector-stores-postgres>=0.5.4", - "torch>=2.7.1", + "torch~=2.7.1", "llama-stack==0.2.22", "llama-stack-client==0.2.22", - "aiosqlite>=0.21.0", - "sqlite-vec>=0.1.6", + "aiosqlite~=0.21.0", + "sqlite-vec~=0.1.6",The
~=operator allows patch-level updates but prevents minor/major version changes that could introduce breaking changes. For example,~=0.12.51allows0.12.52but not0.13.0.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
pyproject.toml(2 hunks)scripts/query_rag.py(3 hunks)src/lightspeed_rag_content/document_processor.py(1 hunks)
🔇 Additional comments (3)
src/lightspeed_rag_content/document_processor.py (1)
283-283: LGTM: Type annotations cleaned up.The removal of
# type: ignorecomments indicates that llama-stack 0.2.22 now properly exports type information forRAGDocument, and the mypy configuration updates inpyproject.tomlenable proper type checking for these imports. This is a positive improvement for type safety.Also applies to: 288-289
pyproject.toml (1)
23-25: LGTM: Mypy configuration for namespace packages.The added mypy configuration properly enables namespace package support, which allows mypy to correctly resolve types from the
srcdirectory structure. This explains why# type: ignorecomments could be removed in other files.scripts/query_rag.py (1)
14-14: Imports are compatible; no issue exists with this code.NodeWithScore and TextNode are available in llama_index.core.schema for v0.12.51, which matches the pinned dependency (
llama-index>=0.12.51).While llama-stack 0.2.22 uses an older Node/TextNode shape incompatible with modern LlamaIndex, this incompatibility does not affect the script. The code has two separate execution paths:
- _llama_index_query() (FAISS): Uses NodeWithScore/TextNode exclusively with LlamaIndex—llama-stack is not involved.
- _llama_stack_query() (llama-stack): Uses only llama-stack APIs and does not reference the imported types.
These paths are mutually exclusive (determined at line 218-224 by
args.vector_store_type), so the incompatibility between libraries never manifests at runtime.Likely an incorrect or invalid review comment.
syedriko
force-pushed
the
syedriko-update-llama-stack-version
branch
from
d8de1b2 to
256c5d7
Compare
syedriko
force-pushed
the
syedriko-update-llama-stack-version
branch
from
256c5d7 to
c10d224
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/query_rag.py (1)
83-97: Guard threshold compare against missing/None scores.
nodes[0].scorecan be None in some retrievers;None < floatraises TypeError.- if args.threshold > 0.0 and nodes[0].score < args.threshold: + top_score = getattr(nodes[0], "score", None) + if args.threshold > 0.0 and (top_score is None or top_score < args.threshold):
🧹 Nitpick comments (4)
scripts/query_rag.py (3)
156-158: Import the submodule explicitly to avoid attribute lookup issues on the package.Top-level
import_module("llama_stack")doesn’t guaranteecoreis loaded. Import the submodule.- stack_lib = importlib.import_module("llama_stack") - client = stack_lib.core.library_client.LlamaStackAsLibraryClient(cfg_file) + lib_client_mod = importlib.import_module("llama_stack.core.library_client") + client = lib_client_mod.LlamaStackAsLibraryClient(cfg_file)
197-211: Mirror the threshold None-guard for llama-stack results.
md["scores"][0]may be a plain float or an object with.score; it may also be missing/None. Guard before comparing.- if threshold > 0.0 and md.get("scores") and md["scores"][0].score < threshold: + top = None + if md.get("scores"): + s0 = md["scores"][0] + top = s0.score if hasattr(s0, "score") else s0 + if threshold > 0.0 and (top is None or top < threshold):
66-66: Prefer sys.exit over exit for scripts.
exit()is intended for interactive shells; usesys.exit()consistently.pyproject.toml (1)
40-52: Constrain fast‑moving libs to a safe range; unify Hugging Face package name.
- The llama-index family moves fast; add an upper bound to avoid surprise breaks.
- Use the canonical PyPI name “huggingface-hub” (hyphen) for consistency.
- "PyYAML>=6.0.2", - "huggingface_hub>=0.33.4", - "llama-index>=0.12.51", - "llama-index-vector-stores-faiss>=0.3.0", - "llama-index-embeddings-huggingface>=0.4.0", - "llama-index-readers-file>=0.4.11", + "PyYAML>=6.0.2", + "huggingface-hub>=0.33.4", + "llama-index>=0.12.51,<0.13", + "llama-index-vector-stores-faiss>=0.3.0,<0.4", + "llama-index-embeddings-huggingface>=0.4.0,<0.5", + "llama-index-readers-file>=0.4.11,<0.5",Optionally consider similar upper bounds for
faiss-cputo the next minor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
pyproject.toml(2 hunks)scripts/query_rag.py(4 hunks)src/lightspeed_rag_content/document_processor.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lightspeed_rag_content/document_processor.py
🔇 Additional comments (2)
pyproject.toml (2)
23-25: mypy path/namespace tweaks look good.Enables proper namespace package resolution under src/. LGTM.
48-50: Original concern is unwarranted; torch>=2.7.1 is available and satisfiable.Latest stable PyTorch CPU wheel for Python 3.12 is 2.8.0+cpu, available at https://download.pytorch.org/whl/cpu. Since 2.8.0 exists, the pinned constraint
torch>=2.7.1is fully resolvable and will install without issue. No changes needed.
| if args.node is not None: | ||
| node = storage_context.docstore.get_node(args.node) | ||
| result = { | ||
| "query": args.query, | ||
| "type": "single_node", | ||
| "node_id": args.node, | ||
| "node": { | ||
| "id": node.node_id, | ||
| "text": node.text, | ||
| "metadata": node.metadata if hasattr(node, "metadata") else {}, | ||
| }, | ||
| } | ||
| if args.json: | ||
| print(json.dumps(result, indent=2)) | ||
| if isinstance(node, TextNode): | ||
| result = { | ||
| "query": args.query, | ||
| "type": "single_node", | ||
| "node_id": args.node, |
There was a problem hiding this comment.
Handle “node not found” explicitly and avoid ambiguous type message; also prefer sys.exit.
docstore.get_node can raise KeyError or return None. Treat that as “not found” instead of “not a TextNode”, and exit via sys.exit to be safe in non-interactive runs.
- if args.node is not None:
- node = storage_context.docstore.get_node(args.node)
+ if args.node is not None:
+ try:
+ node = storage_context.docstore.get_node(args.node)
+ except KeyError:
+ msg = f"Node {args.node} not found"
+ logging.warning(msg)
+ if args.json:
+ print(json.dumps({
+ "query": args.query,
+ "type": "single_node",
+ "node_id": args.node,
+ "error": msg,
+ }, indent=2))
+ sys.exit(1)
if isinstance(node, TextNode):
...
- else:
+ else:
logging.warning(
f"Node {args.node} is not a TextNode, type: {type(node).__name__}"
)
if args.json:
result = {
"query": args.query,
"type": "single_node",
"node_id": args.node,
"error": f"Node is not a TextNode (type: {type(node).__name__})",
}
print(json.dumps(result, indent=2))
- exit(1)
+ sys.exit(1)Also applies to: 55-66
🤖 Prompt for AI Agents
In scripts/query_rag.py around lines 37-43 (and similarly 55-66), handle the
case where storage_context.docstore.get_node may raise KeyError or return None:
wrap the get_node call in a try/except KeyError (and check for None) and if
missing print/log a clear "node not found" message and call sys.exit(1); if the
node is present but not a TextNode print/log a clear "node is not a TextNode"
message and call sys.exit(1) instead of falling through to ambiguous behavior.
Ensure sys is imported at the top if not already.
| for node in nodes: # type: ignore | ||
| node_data = { | ||
| "id": node.node_id, | ||
| "score": node.score, | ||
| "text": node.text, | ||
| "metadata": node.metadata if hasattr(node, "metadata") else {}, | ||
| } | ||
| result["nodes"].append(node_data) | ||
| if isinstance(node, NodeWithScore): | ||
| node_data = { | ||
| "id": node.node_id, | ||
| "score": node.score, | ||
| "text": node.text, | ||
| "metadata": node.metadata if hasattr(node, "metadata") else {}, | ||
| } | ||
| result["nodes"].append(node_data) | ||
| else: | ||
| logging.debug( | ||
| f"Skipping node of type {type(node).__name__}, expected NodeWithScore" | ||
| ) |
There was a problem hiding this comment.
Access fields via NodeWithScore.node to avoid AttributeError across versions; drop the loop’s type: ignore.
In llama-index 0.12, NodeWithScore holds the node in .node. Accessing .node_id, .text, .metadata directly on the wrapper may fail.
- for node in nodes: # type: ignore
- if isinstance(node, NodeWithScore):
- node_data = {
- "id": node.node_id,
- "score": node.score,
- "text": node.text,
- "metadata": node.metadata if hasattr(node, "metadata") else {},
- }
- result["nodes"].append(node_data)
+ from typing import Sequence, cast
+ retrieved = cast(Sequence[NodeWithScore], nodes)
+ for nw in retrieved:
+ if isinstance(nw, NodeWithScore):
+ base = nw.node
+ # Prefer get_content() to work for non-TextNode nodes too
+ text = getattr(base, "text", None)
+ if text is None and hasattr(base, "get_content"):
+ text = base.get_content() or ""
+ node_data = {
+ "id": getattr(base, "node_id", getattr(base, "id_", "")),
+ "score": nw.score,
+ "text": text,
+ "metadata": getattr(base, "metadata", {}) or {},
+ }
+ result["nodes"].append(node_data)
else:
logging.debug(
f"Skipping node of type {type(node).__name__}, expected NodeWithScore"
)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/query_rag.py around lines 105–117, the loop currently assumes
NodeWithScore exposes node_id/text/metadata directly and uses a "# type:
ignore"; change it to handle both wrapper and plain Node without ignoring types:
if isinstance(node, NodeWithScore) set inner = node.node else set inner = node,
then read inner.node_id, inner.score (from wrapper), inner.text and
inner.metadata; remove the "# type: ignore" comment and ensure you use the
wrapper's score when present so the code works for llama-index 0.12+ and earlier
versions.
tisnik
left a comment
tisnik
left a comment
There was a problem hiding this comment.
why they are changes made in source code? Should not it be just one line in pyproject.toml and re-generated uv.lock?
2 participants
Description
Update Llama Stack to 0.2.22 and the rest of the Python deps accordingly.
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Chores
Bug Fixes