Skip to content

LCORE-1931: Fixed issues in test_vector_store_request.py#1542

Merged
tisnik merged 1 commit into
lightspeed-core:mainfrom
tisnik:lcore-1931-fixed-issues-in-test-vector-store-request
Apr 20, 2026
Merged

LCORE-1931: Fixed issues in test_vector_store_request.py#1542
tisnik merged 1 commit into
lightspeed-core:mainfrom
tisnik:lcore-1931-fixed-issues-in-test-vector-store-request

Conversation

@tisnik
Copy link
Copy Markdown
Contributor

@tisnik tisnik commented Apr 20, 2026

Description

LCORE-1931: Fixed issues in test_vector_store_request.py

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
  • Benchmarks improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1931

Summary by CodeRabbit

  • Tests
    • Updated unit tests for vector store request handlers with enhanced type annotations and parameter validation.

Note: This release contains internal testing updates with no user-facing changes.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 298650d2-2a36-4a20-98be-b214fca6bf69

📥 Commits

Reviewing files that changed from the base of the PR and between b58c114 and 5781b26.

📒 Files selected for processing (1)
  • tests/unit/models/requests/test_vector_store_requests.py
📜 Recent review details
⏰ 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). (15)
  • GitHub Check: Pylinter
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: mypy
  • GitHub Check: build-pr
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Import FastAPI dependencies with: from fastapi import APIRouter, HTTPException, Request, status, Depends
Import Llama Stack client with: from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
Type aliases defined at module level for clarity
Use Final[type] as type hint for all constants
All functions require docstrings with brief descriptions
Complete type annotations for parameters and return types in functions
Use typing_extensions.Self for model validators in Pydantic models
Use modern union type syntax str | int instead of Union[str, int]
Use Optional[Type] for optional type hints
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead
Use async def for I/O operations and external API calls
Handle APIConnectionError from Llama Stack in error handling
Use standard log levels with clear purposes: debug, info, warning, error
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use ABC for abstract base classes with @abstractmethod decorators
Use @model_validator and @field_validator for Pydantic model validation
Complete type annotations for all class attributes; use specific types, not Any
Follow Google Python docstring conventions with Parameters, Returns, Raises, and Attributes sections

Files:

  • tests/unit/models/requests/test_vector_store_requests.py
tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests
Do not use unittest; pytest is the standard for this project
Use pytest-mock for AsyncMock objects in tests
Use marker pytest.mark.asyncio for async tests
Unit tests require 60% coverage, integration tests 10%

Files:

  • tests/unit/models/requests/test_vector_store_requests.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T15:40:25.624Z
Learning: Write unit tests covering new functionality
🔇 Additional comments (2)
tests/unit/models/requests/test_vector_store_requests.py (2)

14-57: LGTM on the pyright: ignore[reportCallIssue] additions.

Since all fields on VectorStoreUpdateRequest are Optional with default None, the call sites are semantically valid; the pyright suppressions are reasonable to silence the reportCallIssue noise introduced by the @model_validator(mode="after") and Pydantic's typing. No functional concerns.


65-180: Explicit attributes=None, chunking_strategy=None keeps tests aligned with the model.

Passing the optional fields explicitly matches the expanded constructor call surface and the dict[str, str | float | bool] annotations on the local attributes variables mirror the field type on VectorStoreFileCreateRequest. All existing validation assertions (16-pair cap, 64-char keys, 512-char string values, None handling, required file_id) are preserved.

One minor observation: test_attributes_non_string_values_allowed and test_attributes_mixed_value_types assign int literals (12345, 42) to a dict annotated dict[str, str | float | bool]. This is intentionally consistent with the field definition in src/models/requests.py, and Pydantic coerces int to float here, so behavior is unchanged — just worth noting if stricter typing is ever enforced.


Walkthrough

Unit tests updated to align with an expanded VectorStoreFileCreateRequest constructor signature. Test cases now explicitly pass attributes=None and chunking_strategy=None parameters, include type-checking ignore directives, and apply more specific type annotations for attribute test data while preserving all validation logic.

Changes

Cohort / File(s) Summary
Test Constructor Signature Updates
tests/unit/models/requests/test_vector_store_requests.py
Updated test invocations to explicitly pass attributes=None and chunking_strategy=None to VectorStoreFileCreateRequest(...). Added # pyright: ignore[reportCallIssue] comments on constructor calls and refined type annotations for attribute test data to dict[str, str | float | bool]. Existing validation assertions for counts, constraints, and None handling remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ 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 directly refers to fixing issues in test_vector_store_request.py, which matches the changeset that updates unit tests with corrected constructor signatures and type annotations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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.

@tisnik
Copy link
Copy Markdown
Contributor Author

tisnik commented Apr 20, 2026

/retest

@tisnik tisnik merged commit f1f210a into lightspeed-core:main Apr 20, 2026
29 of 30 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.

1 participant