Skip to content

Conversation

@umago
Copy link
Contributor

@umago umago commented Sep 11, 2025

Description

This patch updates the dependencies of this repository. The primary reason is the openai version 1.99.7 has the following problem:

from llama_stack.providers.utils.inference.openai_compat import (
File
"/rag-content/.venv/lib64/python3.12/site-packages/llama_stack/providers/utils/inference/openai_compat.py",
line 37, in
from openai.types.chat import (
ImportError: cannot import name
'ChatCompletionMessageToolCall' from 'openai.types.chat' (/rag-content/.venv/lib64/python3.12/site-packages/openai/types/chat/init.py). Did you mean: 'ChatCompletionMessageToolCallParam'?

And this has been fixed on later versions:
openai/openai-python@ba54e03

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Style

    • Standardized string quoting and added trailing commas for consistent formatting.
    • Harmonized logging format strings and streamlined configuration layout.
    • Minor whitespace and comment cleanups for readability.
  • Chores

    • Added/updated typing annotations to clarify intent without altering behavior.

No user-facing changes. Functionality, inputs/outputs, and error handling remain unchanged.

This patch updates the dependencies of this repository. The primary
reason is the openai version 1.99.7 has the following problem:

from llama_stack.providers.utils.inference.openai_compat import (
  File
  "/rag-content/.venv/lib64/python3.12/site-packages/llama_stack/providers/utils/inference/openai_compat.py",
  line 37, in <module>
    from openai.types.chat import (
ImportError: cannot import name
'ChatCompletionMessageToolCall' from 'openai.types.chat'
(/rag-content/.venv/lib64/python3.12/site-packages/openai/types/chat/__init__.py).
Did you mean: 'ChatCompletionMessageToolCallParam'?

And this has been fixed on later versions:
openai/openai-python@ba54e03

Signed-off-by: Lucas Alvares Gomes <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Sep 11, 2025

Walkthrough

Purely stylistic and typing updates in scripts/query_rag.py: quote style normalization, trailing commas, comment/annotation tweaks, and minor logging configuration formatting. No functional or control-flow changes.

Changes

Cohort / File(s) Summary of changes
Stylistic and typing adjustments
scripts/query_rag.py
Switched single to double quotes; added trailing commas in dicts; added inline type ignore in node loop; minor JSON result formatting tweaks; adjusted hasattr usage with double quotes; logging configuration reformatted (including non-JSON path); inserted blank line before the main guard; no logic changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • tisnik
  • lpiwowar

Pre-merge checks (1 passed, 2 warnings)

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current title claims "Update the dependencies and fix gate issues (black syntax)" but the provided raw_summary shows only stylistic/typing changes in scripts/query_rag.py with no dependency or CI/gate file modifications; the pr_objectives mention dependency intent, but that intent is not reflected in the changeset. Therefore the title is misleading and does not accurately summarize the main change. Rename the PR to accurately describe the actual changes (for example: "Format scripts/query_rag.py — style and typing tweaks") or include the missing dependency/CI changes in the diff and update the description to match; once the title and changeset align, re-run the pre-merge checks.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

Poem

I twitch my whiskers at tidy delight,
Quotes aligned, commas tucked tight.
No logic hops—just smoother trails,
Typing hints trim their cotton tails.
In quiet fields where linters roam,
I stamp approval: formatting home. 🐇

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Lucas Alvares Gomes <[email protected]>
@umago umago changed the title Update the dependencies Update the dependencies and fix gate issues (black syntax) Sep 11, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
scripts/query_rag.py (5)

45-46: Use getattr for cleaner, safer metadata access

Slight readability/type improvement; avoids hasattr and works the same.

-                "metadata": node.metadata if hasattr(node, "metadata") else {},
+                "metadata": getattr(node, "metadata", {}),

And similarly below:

-                "metadata": node.metadata if hasattr(node, "metadata") else {},
+                "metadata": getattr(node, "metadata", {}),

Also applies to: 95-96


90-90: Replace type: ignore with explicit typing

Prefer tightening types at assignment instead of suppressing the checker.

-        for node in nodes:  # type: ignore
+        for node in nodes:

To support this, annotate/cast at retrieval (outside this hunk):

from typing import Sequence, cast
from llama_index.core.schema import NodeWithScore

nodes = cast(Sequence[NodeWithScore], retriever.retrieve(args.query))

Please confirm the project uses a LlamaIndex version where NodeWithScore is available/named as above; adjust import if it’s moved/renamed.


204-204: Minor: prefer getattr for score extraction

Equivalent behavior with slightly cleaner syntax.

-            "score": score.score if hasattr(score, "score") else score,
+            "score": getattr(score, "score", score),

206-207: Optionally propagate chunk metadata when available

If res.metadata provides per-chunk metadata (often under "metadatas"), include it to aid callers.

Example (conceptual):

metadatas = md.get("metadatas") or [{}] * len(md["chunks"])
for (i, (_id, chunk, score)) in enumerate(zip(md["document_ids"], md["chunks"], md["scores"])):
    node_data = {
        "id": _id,
        "score": getattr(score, "score", score),
        "text": chunk,
        "metadata": metadatas[i],
    }

267-269: Logging setup: good JSON/STDERR separation; consider force=True

Current change correctly keeps stdout clean in JSON mode. If this script can be invoked from environments that pre-configure logging, add force=True to avoid duplicate handlers/formatters.

-        logging.basicConfig(
+        logging.basicConfig(
             level=logging.ERROR,
             format="%(levelname)s: %(message)s",
-            stream=sys.stderr,  # Send logs to stderr to keep stdout clean for JSON
+            stream=sys.stderr,  # Send logs to stderr to keep stdout clean for JSON
+            force=True,
         )
...
-        logging.basicConfig(level=logging.INFO, format="%(message)s")
+        logging.basicConfig(level=logging.INFO, format="%(message)s", force=True)

Verify your minimum Python version is 3.8+ (force was added in 3.8).

Also applies to: 273-273

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 275a28c and 5134ec7.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • scripts/query_rag.py (9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Pylinter
  • GitHub Check: build-and-push-dev
  • GitHub Check: mypy
🔇 Additional comments (1)
scripts/query_rag.py (1)

63-64: LGTM: JSON shape consistency

Trailing-commas/empty list normalization improves diff hygiene and keeps the response schema stable. No behavior change.

Also applies to: 78-79, 88-89, 172-173, 188-189, 198-199

Copy link
Collaborator

@tisnik tisnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tisnik tisnik merged commit 218898f into lightspeed-core:main Sep 11, 2025
14 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 28, 2025
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants