Skip to content

Conversation

@are-ces
Copy link
Contributor

@are-ces are-ces commented Sep 25, 2025

Description

Substituted all fixed references to the name of the rag tool "knowledge_search" with the constant.

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

NA

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

NA

Summary by CodeRabbit

  • New Features

    • None
  • Bug Fixes

    • Improved reliability of referenced documents and streaming responses by consistently recognizing the configured retrieval tool, reducing missed results and edge-case failures across environments.
  • Refactor

    • Standardized tool identification across query and streaming endpoints for consistent behavior and easier configuration management.
  • Chores

    • Minor import reorganization with no impact on functionality or public APIs.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Replaces hard-coded tool name checks ("knowledge_search") with a centralized constant (DEFAULT_RAG_TOOL) in query and streaming_query endpoints. Adjusts and reorganizes imports accordingly. No public API changes.

Changes

Cohort / File(s) Summary
RAG tool name normalization
src/app/endpoints/query.py, src/app/endpoints/streaming_query.py
Replaced string literal "knowledge_search" comparisons with constants.DEFAULT_RAG_TOOL across tool-response handling and parsing logic; updated/imported constants accordingly; minor import reordering without behavior change otherwise.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Client
    participant Q as Query Endpoint
    participant T as Tool Executor
    participant S as Stream Handler

    C->>Q: Submit query
    Q->>T: Execute tools
    T-->>Q: Tool responses
    alt Response tool_name == DEFAULT_RAG_TOOL
        Q->>Q: Parse referenced documents
        Q->>S: Stream RAG content (docs/snippets)
    else Other tools
        Q->>S: Stream non-RAG tool output
    end
    S-->>C: Incremental results
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • tisnik
  • asamal4
  • VladimirKadlec

Poem

A nibble of code, a hop through the pool,
I swapped a hard string for a constant so cool.
Now tools align, no guesswork or duel—
DEFAULT_RAG_TOOL leads the rule!
Thump-thump, I stream with glee,
Little bunny commits, bug-free. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely describes the primary change in the pull request, namely replacing hard-coded references to the RAG tool name with a constant, which matches the modifications in both endpoint modules. It accurately reflects the main functional update without extraneous detail or ambiguity.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6529c37 and d8f617e.

📒 Files selected for processing (2)
  • src/app/endpoints/query.py (2 hunks)
  • src/app/endpoints/streaming_query.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports for internal modules (e.g., from auth import get_auth_dependency)

Files:

  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/query.py
src/app/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use standard FastAPI imports (from fastapi import APIRouter, HTTPException, Request, status, Depends) in FastAPI app code

Files:

  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/query.py
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: All modules start with descriptive module-level docstrings explaining purpose
Use logger = logging.getLogger(name) for module logging after import logging
Define type aliases at module level for clarity
All functions require docstrings with brief descriptions
Provide complete type annotations for all function parameters and return types
Use typing_extensions.Self in model validators where appropriate
Use modern union syntax (str | int) and Optional[T] or T | None consistently
Function names use snake_case with descriptive, action-oriented prefixes (get_, validate_, check_)
Avoid in-place parameter modification; return new data structures instead of mutating arguments
Use appropriate logging levels: debug, info, warning, error with clear messages
All classes require descriptive docstrings explaining purpose
Class names use PascalCase with conventional suffixes (Configuration, Error/Exception, Resolver, Interface)
Abstract base classes should use abc.ABC and @AbstractMethod for interfaces
Provide complete type annotations for all class attributes
Follow Google Python docstring style for modules, classes, and functions, including Args, Returns, Raises, Attributes sections as needed

Files:

  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/query.py
src/{app/**/*.py,client.py}

📄 CodeRabbit inference engine (CLAUDE.md)

Use async def for I/O-bound operations and external API calls

Files:

  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/query.py
src/app/endpoints/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

In API endpoints, raise FastAPI HTTPException with appropriate status codes for error handling

Files:

  • src/app/endpoints/streaming_query.py
  • src/app/endpoints/query.py
🧬 Code graph analysis (1)
src/app/endpoints/streaming_query.py (2)
src/app/endpoints/query.py (1)
  • get_rag_toolgroups (709-736)
src/utils/types.py (1)
  • TurnSummary (61-135)
⏰ 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). (2)
  • GitHub Check: e2e_tests
  • GitHub Check: build-pr
🔇 Additional comments (2)
src/app/endpoints/query.py (1)

516-517: Nice cleanup on the RAG tool check.

Switching to constants.DEFAULT_RAG_TOOL keeps the endpoint aligned with the shared definition and prevents future regressions when the tool name changes.

src/app/endpoints/streaming_query.py (1)

489-517: Good consistency update for streaming RAG responses.

The constant-based comparison mirrors the non-streaming path and ensures both flows stay in lockstep when the default tool identifier evolves.

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.


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.

Copy link
Contributor

@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 32c6fbf into lightspeed-core:main Sep 25, 2025
18 of 19 checks passed
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