Conversation
WalkthroughThis PR moves the vector-index API from draive.utils into draive.embedding (adds VectorIndex and new VectorIndexing/VectorSearching/VectorDeleting protocols and corresponding imports), deletes src/draive/utils/vector_index.py, refactors volatile vector index to perform embedding before acquiring locks, rewrites multimodal tokenization/parsing internals, replaces template variable parsing with a regex-driven implementation, tightens evaluator tag parsing and FORMAT_INSTRUCTION (with new tests), and applies documentation, Makefile (.env removal and uv version bump), and pyproject version/dependency updates. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
AGENTS.md(3 hunks)Makefile(1 hunks)pyproject.toml(2 hunks)src/draive/__init__.py(2 hunks)src/draive/aws/api.py(1 hunks)src/draive/aws/client.py(1 hunks)src/draive/aws/types.py(1 hunks)src/draive/embedding/__init__.py(1 hunks)src/draive/embedding/state.py(7 hunks)src/draive/embedding/types.py(2 hunks)src/draive/evaluators/utils.py(2 hunks)src/draive/helpers/volatile_vector_index.py(2 hunks)src/draive/multimodal/content.py(3 hunks)src/draive/multimodal/templates/repository.py(1 hunks)src/draive/multimodal/templates/types.py(1 hunks)src/draive/multimodal/templates/variables.py(3 hunks)src/draive/postgres/vector_index.py(1 hunks)src/draive/qdrant/index.py(1 hunks)src/draive/resources/template.py(1 hunks)src/draive/utils/__init__.py(0 hunks)src/draive/utils/vector_index.py(0 hunks)tests/test_evaluators_utils.py(1 hunks)tests/test_multimodal_template_variables.py(1 hunks)tests/test_tags_extraction.py(1 hunks)tests/test_tags_replacement.py(1 hunks)
💤 Files with no reviewable changes (2)
- src/draive/utils/init.py
- src/draive/utils/vector_index.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python 3.13+ for development
Use Ruff formatter for code formatting (viamake format), no other formatter
Run Ruff, Bandit, and Pyright (strict) linters viamake lint
Import symbols fromhaiwaydirectly usingfrom haiway import State, ctx
Use context scoping withctx.scope(...)to bind scopedDisposables, activeStateinstances and avoid global state
All logs must go throughctx.log_*methods; do not use
Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or looseAnyunless required by third-party boundaries
Prefer explicit attribute access with static types; avoid dynamicgetattrexcept at narrow boundaries
Prefer abstract immutable protocols (Mapping,Sequence,Iterable) overdict/list/setin public types
Usefinalwhere applicable; avoid inheritance, prefer type composition
Use precise unions (|) and narrow withmatch/isinstance, avoidcastunless provably safe and localized
Usehaiway.Statefor immutable data/config and service facades, construct with classmethods likeof(...)when ergonomic
Avoid in-place mutation; useState.updated(...)/functional builders to create new instances
Access active state throughhaiway.ctxinside async scopes (ctx.scope(...))
Public state methods that dispatch on the active instance should use@statemethod
Usectx.log_debug/info/warn/errorfor logs; do not use
Log around generation calls, tool dispatch, provider requests/responses without leaking secrets
Add appropriate metrics tracking usingctx.recordwhere applicable
Prefer structured and concise log messages; avoid excessive verbosity in hot paths
All I/O is async, keep boundaries async and usectx.spawnfor detached tasks
Ensure structured concurrency concepts and valid coroutine usage
Rely on haiway and asyncio packages with coroutines, avoid custom threading
Don't raise bareException, preserve contextual information in excep...
Files:
src/draive/multimodal/templates/repository.pytests/test_tags_replacement.pysrc/draive/aws/api.pysrc/draive/qdrant/index.pysrc/draive/embedding/__init__.pysrc/draive/multimodal/templates/variables.pysrc/draive/multimodal/content.pytests/test_evaluators_utils.pysrc/draive/multimodal/templates/types.pytests/test_multimodal_template_variables.pysrc/draive/resources/template.pysrc/draive/aws/types.pysrc/draive/evaluators/utils.pysrc/draive/postgres/vector_index.pytests/test_tags_extraction.pysrc/draive/aws/client.pysrc/draive/embedding/types.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/__init__.pysrc/draive/embedding/state.py
src/draive/**/!(test_)*.py
📄 CodeRabbit inference engine (AGENTS.md)
Translate provider/SDK errors into appropriate typed exceptions
Files:
src/draive/multimodal/templates/repository.pysrc/draive/aws/api.pysrc/draive/qdrant/index.pysrc/draive/embedding/__init__.pysrc/draive/multimodal/templates/variables.pysrc/draive/multimodal/content.pysrc/draive/multimodal/templates/types.pysrc/draive/resources/template.pysrc/draive/aws/types.pysrc/draive/evaluators/utils.pysrc/draive/postgres/vector_index.pysrc/draive/aws/client.pysrc/draive/embedding/types.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/__init__.pysrc/draive/embedding/state.py
src/draive/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Files:
src/draive/multimodal/templates/repository.pysrc/draive/aws/api.pysrc/draive/qdrant/index.pysrc/draive/embedding/__init__.pysrc/draive/multimodal/templates/variables.pysrc/draive/multimodal/content.pysrc/draive/multimodal/templates/types.pysrc/draive/resources/template.pysrc/draive/aws/types.pysrc/draive/evaluators/utils.pysrc/draive/postgres/vector_index.pysrc/draive/aws/client.pysrc/draive/embedding/types.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/__init__.pysrc/draive/embedding/state.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: No network in unit tests, mock providers/HTTP
Keep tests fast and specific to the code you change, start with unit tests around new types/functions and adapters
Use fixtures fromtests/or add focused ones; avoid heavy integration scaffolding
Usepytest-asynciofor coroutine tests with@pytest.mark.asynciodecorator
Prefer scoping withctx.scope(...)and bind requiredStateinstances explicitly in async tests
Avoid real I/O and network in tests; stub provider calls and HTTP
Files:
tests/test_tags_replacement.pytests/test_evaluators_utils.pytests/test_multimodal_template_variables.pytests/test_tags_extraction.py
src/draive/**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Update
__init__.pyexports and docs when API surface changes
Files:
src/draive/embedding/__init__.pysrc/draive/__init__.py
🧠 Learnings (8)
📚 Learning: 2025-11-27T14:40:28.848Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.848Z
Learning: Applies to src/draive/**/__init__.py : Update `__init__.py` exports and docs when API surface changes
Applied to files:
src/draive/aws/api.pysrc/draive/qdrant/index.pysrc/draive/embedding/__init__.pysrc/draive/postgres/vector_index.pysrc/draive/aws/client.pysrc/draive/embedding/types.pysrc/draive/__init__.pyAGENTS.md
📚 Learning: 2025-06-16T10:28:07.434Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 338
File: src/draive/lmm/__init__.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Applied to files:
pyproject.tomlAGENTS.md
📚 Learning: 2025-11-27T14:40:28.848Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.848Z
Learning: Applies to **/*.py : Build content with `MultimodalContent.of(...)` and prefer composing content blocks explicitly
Applied to files:
src/draive/multimodal/content.py
📚 Learning: 2025-05-28T17:41:57.460Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., `def func[T: Type]()`) is valid and should be used instead of the older TypeVar approach.
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.848Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.848Z
Learning: Applies to **/*.py : Use `haiway.State` for immutable data/config and service facades, construct with classmethods like `of(...)` when ergonomic
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.848Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.848Z
Learning: Applies to **/*.py : Access active state through `haiway.ctx` inside async scopes (`ctx.scope(...)`)
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.848Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.848Z
Learning: Applies to **/*.py : Rely on haiway and asyncio packages with coroutines, avoid custom threading
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.848Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.848Z
Learning: Applies to **/*.py : Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or loose `Any` unless required by third-party boundaries
Applied to files:
AGENTS.md
🧬 Code graph analysis (15)
tests/test_tags_replacement.py (1)
src/draive/multimodal/content.py (4)
MultimodalContent(25-592)of(42-66)of(619-647)replacing_tag(448-581)
src/draive/qdrant/index.py (2)
src/draive/embedding/types.py (1)
Embedded(25-28)src/draive/embedding/state.py (3)
ImageEmbedding(202-367)TextEmbedding(30-199)VectorIndex(371-587)
src/draive/embedding/__init__.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
src/draive/multimodal/templates/variables.py (1)
src/draive/multimodal/content.py (6)
MultimodalContent(25-592)parts(668-676)to_str(288-297)to_str(653-666)of(42-66)of(619-647)
tests/test_evaluators_utils.py (3)
src/draive/evaluators/utils.py (1)
extract_evaluation_result(25-57)src/draive/multimodal/content.py (5)
MultimodalContent(25-592)MultimodalTag(599-700)tags(427-446)of(42-66)of(619-647)src/draive/multimodal/templates/types.py (2)
of(56-84)of(162-194)
src/draive/multimodal/templates/types.py (1)
tests/test_toolbox.py (2)
meta(109-110)meta(310-311)
tests/test_multimodal_template_variables.py (1)
src/draive/multimodal/templates/variables.py (1)
parse_template_variables(17-21)
src/draive/resources/template.py (2)
src/draive/parameters/model.py (3)
get(542-545)get(548-552)get(554-559)src/draive/stages/types.py (4)
get(140-144)get(147-153)get(156-162)get(164-206)
src/draive/evaluators/utils.py (1)
src/draive/multimodal/content.py (7)
MultimodalContent(25-592)MultimodalTag(599-700)tag(405-425)of(42-66)of(619-647)to_str(288-297)to_str(653-666)
src/draive/postgres/vector_index.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
tests/test_tags_extraction.py (1)
src/draive/multimodal/content.py (5)
MultimodalContent(25-592)of(42-66)of(619-647)tags(427-446)tag(405-425)
src/draive/embedding/types.py (4)
src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/parameters/model.py (1)
DataModel(254-634)src/draive/multimodal/content.py (1)
resources(95-125)src/draive/resources/types.py (1)
ResourceContent(128-214)
src/draive/helpers/volatile_vector_index.py (4)
src/draive/embedding/state.py (11)
ImageEmbedding(202-367)embed_many(121-126)embed_many(129-134)embed_many(138-144)embed_many(147-153)embed_many(156-197)embed_many(290-295)embed_many(298-303)embed_many(307-313)embed_many(316-322)embed_many(325-365)src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/resources/types.py (2)
ResourceContent(128-214)to_bytes(207-208)src/draive/embedding/types.py (1)
Embedded(25-28)
src/draive/__init__.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
src/draive/embedding/state.py (7)
src/draive/qdrant/state.py (11)
collections(28-28)collections(31-31)collections(34-35)search(269-280)search(284-294)search(297-308)search(311-321)search(324-344)delete(396-403)delete(406-413)delete(416-428)src/draive/embedding/types.py (5)
Embedded(25-28)ValueEmbedding(32-39)VectorDeleting(71-79)VectorIndexing(43-52)VectorSearching(56-67)src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/resources/types.py (1)
ResourceContent(128-214)src/draive/postgres/vector_index.py (3)
index(74-167)search(169-285)delete(287-317)src/draive/qdrant/index.py (3)
index(25-83)search(85-158)delete(160-171)src/draive/qdrant/search.py (4)
search(18-29)search(32-43)search(46-57)search(59-94)
🪛 LanguageTool
AGENTS.md
[grammar] ~34-~34: Ensure spelling is correct
Context: ...e/init.py`. ## Style & Patterns - Draive is built on top of Haiway (state, conte...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
34-34: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (46)
Makefile (2)
10-10: Inconsistency with AI summary.The AI summary states that the
.envfile inclusion was removed, but Line 10 still contains-include .env(unmarked as changed). Please clarify whether this line was intentionally preserved or if the summary is inaccurate.
13-13: uv version upgrade 0.9.9 → 0.9.14 is safe; no breaking changes identified.Release notes for versions 0.9.10–0.9.14 show only enhancements, performance improvements, bug fixes, and documentation updates. No breaking changes are reported across this range.
src/draive/aws/types.py (1)
14-31: Docstring spacing change is fineAdding a blank line between the class docstring and
__slots__improves readability and does not affect behavior.pyproject.toml (1)
8-27: Confirm compatibility with haiway 0.39.2 and release 0.94.1Version and
haiwayconstraint bumps look consistent with the existing Python 3.13 toolchain; no config issues spotted. Please just ensure the suite (make format && make lint && pytest) has been run againsthaiway~=0.39.2.src/draive/aws/client.py (1)
19-38: Minor docstring formatting tweak onlyThe extra blank line between the class docstring and
__slots__is stylistic and has no behavioral impact.src/draive/multimodal/templates/repository.py (1)
60-68: Updated TemplatesRepository docstring matches implementationThe new description correctly reflects that the repository aggregates listing/loading/defining callables with in‑memory and file-backed helpers; no code changes needed.
src/draive/aws/api.py (1)
6-6: Expose AWSAPI via tuple allSwitching
__all__to a tuple is idiomatic and behaviorally equivalent here.src/draive/resources/template.py (1)
65-69: Parameter lookup refactor keeps behavior intactRemoving the redundant parentheses in the
parameterlookup maintains the samename or aliasresolution behavior and keeps the type asAttribute | None; no issues.tests/test_tags_extraction.py (1)
423-477: New self-closing tag tests provide solid edge-case coverageThe added tests around self-closing tags (with attributes, mixed with text/other tags, and malformed-attribute cases) are coherent and align with the expected
MultimodalContent/MultimodalTagbehavior; no changes requested.src/draive/embedding/__init__.py (1)
4-15: Re-exporting VectorIndex from embedding aligns public API with new locationIncluding
VectorIndexin the embedding package exports makes the new vector index state discoverable viadraive.embeddingand matches the refactor away fromdraive.utils; looks consistent with the surrounding API.tests/test_tags_replacement.py (2)
668-675: LGTM! Good coverage for self-closing tags with attributes.The test correctly verifies that self-closing tags with attributes are replaced properly—converting to paired tags when
strip_tags=Falseand yielding plain content whenstrip_tags=True.
678-681: LGTM! Malformed tag handling verified.This test ensures malformed self-closing tags (missing quoted attribute value) are left unchanged, which aligns with the parser's design to reject invalid syntax gracefully.
src/draive/embedding/types.py (1)
42-52: LGTM! Clean protocol definition for vector indexing.The
VectorIndexingprotocol properly defines the async callable signature with appropriate type bounds using Python 3.13+ generic syntax. TheValuebound correctly restricts to embeddable content types.src/draive/multimodal/content.py (6)
717-743: LGTM! Good use ofslots=Truefor memory optimization.Adding
slots=Trueto frozen dataclasses reduces memory overhead and improves attribute access performance for these frequently instantiated token types.
888-920: LGTM! Clean refactor of text tokenization.The early return for text without
<is a good optimization. The loop structure is clear with proper cursor management via_parse_tag_at.
923-1022: LGTM! Well-structured tag parser with proper edge case handling.The
_parse_tag_atfunction cleanly handles opening tags, closing tags, self-closing tags, and attribute parsing. The fast-path for tags without attributes (lines 959-965) is a good optimization. Return-based control flow with explicitNonefor malformed input is clear.
1040-1054: Attribute ordering issue: first attribute may be lost if duplicated inrest.In
_meta_from_attrs, line 1051-1052 addsfirst_keytorestonly if it's not already present. However, if the same attribute name appears twice in a tag (malformed but possible), the second occurrence would be inrestand the first would be silently dropped. This is likely acceptable behavior for malformed input, but worth noting.
1072-1120: LGTM! Robust attribute value parsing with proper escape handling.The escape sequence handling covers standard cases (
\n,\t,\r,\",\',\\) and correctly passes through unknown escapes. Rejecting newlines/carriage returns within quoted values (lines 1093-1094) prevents malformed multi-line attributes.
1123-1137: LGTM! Clean helper extraction.
_current_partsand_find_matching_contextare well-extracted helpers that improve readability of the main parsing logic.src/draive/qdrant/index.py (1)
6-12: LGTM! Clean import consolidation.The
VectorIndeximport is correctly relocated fromdraive.utilstodraive.embedding, and grouping all embedding-related imports together improves clarity. This aligns with the PR's refactoring to move vector index functionality into the embedding package.src/draive/multimodal/templates/types.py (1)
88-88: LGTM! Default aligns with TemplateDeclaration pattern.Adding
META_EMPTYas the default formetais consistent withTemplateDeclaration(line 199) and allows direct construction without explicitly providing metadata while maintaining the same behavior through theof()factory method.src/draive/__init__.py (2)
92-101: LGTM! VectorIndex relocation properly reflected.The import of
VectorIndexfromdraive.embeddingaligns with its new location in the embedding subsystem. The public API surface remains unchanged as the symbol is still exported via__all__.
213-213: Import cleanup is consistent with relocation.Removing
VectorIndexfrom thedraive.utilsimport is consistent with its move to the embedding package. Based on learnings, the__init__.pyexports are correctly updated to reflect the API surface change.tests/test_multimodal_template_variables.py (3)
12-17: LGTM! Tests correctly adapted to generator-based API.Wrapping
parse_template_variables()inset()properly handles the API change where the function now returns aGenerator[str]instead of a pre-constructed set. This maintains test semantics while adapting to the new return type.
20-21: Concise empty-case test.Good inline assertion for the no-placeholders case.
88-208: Comprehensive resolve_ test coverage.*The
resolve_text_templateandresolve_multimodal_templatetests thoroughly cover substitution, repeated placeholders, missing arguments, unused arguments, edge positions, empty templates, metadata preservation, and nested content flattening. Good test coverage.src/draive/postgres/vector_index.py (3)
10-16: LGTM! Import updated to new VectorIndex location.The
VectorIndeximport correctly references its new home indraive.embedding, consistent with the codebase-wide relocation.
24-27: Return type annotation improves API clarity.Explicitly annotating the return type as
VectorIndexmakes the function's contract clearer and aids static type checking.
319-323: Factory correctly constructs VectorIndex.The
VectorIndexconstruction withindexing,searching, anddeletingcallables matches theVectorIndexclass attributes defined insrc/draive/embedding/state.py.tests/test_evaluators_utils.py (4)
7-8: LGTM! Clean helper for test content construction.The
_content_with_tagshelper provides a concise way to construct test content from tags.
11-20: Good coverage of the happy path.Tests both the numeric
valueand themeta["comment"]extraction in a single coherent test case.
41-45: Good edge case: optional comment handling.Verifies that absence of a comment tag is valid and results in
Nonemetadata.
48-55: Comprehensive invalid rating test.Ensures unknown rating values are properly rejected with a
ValueError.src/draive/helpers/volatile_vector_index.py (3)
7-14: LGTM! Import path correctly updated.The
VectorIndeximport is now sourced fromdraive.embedding, aligning with the API relocation described in the PR.
34-57: LGTM! Value classification and validation logic is correct.The pre-validation phase properly:
- Validates the attribute selector
- Separates text and image values with appropriate type checks
- Validates MIME types for resource content
- Rejects mixed text/image inputs
59-89: Good refactoring with improved concurrency.Deferring lock acquisition until after embedding allows concurrent embedding operations. The
strict=Trueinzipcorrectly ensures length alignment between input values and embeddings.src/draive/multimodal/templates/variables.py (2)
1-8: LGTM! Clean regex-based approach.The unified
_PLACEHOLDER_PATTERNcentralizes variable matching logic and the regex correctly captures variable names without whitespace or%characters.
17-21: LGTM! Generator-based parsing is efficient.The lazy generator avoids materializing all variables upfront. Callers that need uniqueness can wrap with
set()as tests demonstrate.src/draive/evaluators/utils.py (3)
11-22: LGTM! Clear format instruction with both tags.The updated
FORMAT_INSTRUCTIONclearly specifies the expected XML structure with both comment and rating tags. The f-string approach ensures consistency with the constant names.
29-46: LGTM! Robust tag validation and extraction.The single-occurrence validation for both tags prevents ambiguous inputs, and the case-insensitive matching (
tag.name.lower()) handles varied casing gracefully. The clear error messages with content context aid debugging.
48-57: Good defensive error handling.Wrapping the result construction in try/except ensures any downstream parsing failures (e.g., invalid score values) are surfaced with context, rather than propagating cryptic exceptions.
src/draive/embedding/state.py (5)
1-27: LGTM! Clean import reorganization.Imports are well-organized and
VectorIndexis correctly added to__all__for public exposure from the embedding module.
203-208: LGTM! Class-level docstring added.The class docstring clearly describes the purpose of
ImageEmbeddingas a convenience wrapper around the embedding state.
370-372: LGTM! Well-designed VectorIndex facade.The
@finaldecorator correctly prevents subclassing, and the class provides a clean facade over the underlying indexing, searching, and deleting backends.
420-432: LGTM! Good observability instrumentation.The
ctx.record_infocalls provide meaningful telemetry with appropriate event names and attributes for tracking vector operations.
585-587: LGTM! Clean attribute declarations.The backend protocol attributes (
indexing,searching,deleting) are clearly typed with their respective protocols, ensuring type-safe delegation.
a8ec821 to
58dc405
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/guides/BasicUsage.md (1)
44-105: Align comments and sample output labels with updated gpt‑5 models.The code now configures
gpt-5-mini/gpt-5, but several comments and printed labels still reference older models:
- Line 73: comment still says
gpt-3.5-turbothough the scope config usesgpt-5-mini.- Line 78: label
"RESULT GPT 3.5 | temperature 0.4"no longer matches the configured model.- Line 97: comment says
gpt-3.5-turbodespite using the current context model.- Line 104: label
"RESULT GPT 3.5 | temperature 0.7"is stale.- The metrics snippet further down still shows
model: "gpt-3.5-turbo"and[gpt-3.5-turbo-0125]for token usage, while the narrative has moved to gpt‑5.To avoid confusion, update the comments and labels, for example:
- # now we are using gpt-3.5-turbo with temperature of 0.4 + # now we are using gpt-5-mini with temperature of 0.4 @@ - print("RESULT GPT 3.5 | temperature 0.4:", result) + print("RESULT GPT 5-mini | temperature 0.4:", result) @@ - # here we are using gpt-3.5-turbo with temperature of 0.7 + # here we are using gpt-5-mini with temperature of 0.7 @@ - print("RESULT GPT 3.5 | temperature 0.7:", result) + print("RESULT GPT 5-mini | temperature 0.7:", result)Optionally, re‑capture or simplify the metrics snippet so the logged model names and token-usage keys match the gpt‑5/gpt‑5‑mini configuration shown in the examples.
Also applies to: 129-141, 205-212
src/draive/multimodal/templates/variables.py (1)
62-93: Docstrings missing for public template APIs.These three exported functions (
parse_template_variables,resolve_text_template,resolve_multimodal_template) have no NumPy-style docstrings, which conflicts with the project guideline forsrc/draive/**/*.pypublic symbols. Consider adding concise docstrings documenting Parameters / Returns / Raises (e.g.,KeyErroron missing arguments).src/draive/embedding/state.py (4)
55-73: Tightenattributetyping to match runtime behavior inTextEmbedding.embed.
attributeis annotated as allowingstr, but the implementation assertsisinstance(attribute, AttributePath | Callable)and then casts toCallable[[Value], str]. Passing astrwill raise at runtime despite the signature suggesting it is supported.Either support strings or narrow the type:
- attribute: Callable[[Value], str] | AttributePath[Value, str] | str, + attribute: Callable[[Value], str] | AttributePath[Value, str], @@ - attribute: Callable[[Value], str] | AttributePath[Value, str] | str | None = None, + attribute: Callable[[Value], str] | AttributePath[Value, str] | None = None,Apply the same adjustment to the corresponding overloads and to
embed_many.
136-163: Mirror theattributetype fix inTextEmbedding.embed_many.
embed_manyhas the same| strin itsattributeannotations but also enforcesAttributePath | Callableat runtime. For consistency and to avoid misleading callers, narrow the type as forembed:- attribute: Callable[[Value], str] | AttributePath[Value, str] | str, + attribute: Callable[[Value], str] | AttributePath[Value, str], @@ - attribute: Callable[[Value], str] | AttributePath[Value, str] | str | None = None, + attribute: Callable[[Value], str] | AttributePath[Value, str] | None = None,This keeps the public API truthful and aligns with the underlying assertion.
246-286: FixImageEmbedding.embedcontent/attribute types to avoid nested sequences and misleading hints.Two issues here:
contentallows sequences, but the implementation always wraps it in a list:await self.embedding([content], ...)If a
Sequence[bytes]is passed, the backend receivesSequence[Sequence[bytes]], which is almost certainly not intended and contradicts the docstring (“Embed a single image payload or model-bound bytes.”).
attributeis annotated with| bytesbut the code assertsAttributePath | Callableonly, so bytes are not actually supported.Suggested fix:
- async def embed[Value: DataModel | State | bytes]( + async def embed[Value: DataModel | State | bytes]( self, - content: Sequence[Value] | Sequence[bytes] | Value | bytes, + content: Value | bytes, /, - attribute: Callable[[Value], bytes] | AttributePath[Value, bytes] | bytes | None = None, + attribute: Callable[[Value], bytes] | AttributePath[Value, bytes] | None = None, @@ - else: - assert isinstance( # nosec: B101 - attribute, AttributePath | Callable - ), "Prepare parameter path by using Type._.path.to.property" - bytes_selector = cast(Callable[[Value], bytes], attribute) + else: + assert isinstance( # nosec: B101 + attribute, AttributePath | Callable + ), "Prepare parameter path by using Type._.path.to.property" + bytes_selector = cast(Callable[[Value], bytes], attribute)This matches the documented behavior (single payload) and keeps the type hints aligned with runtime checks. Use
embed_manyfor batch inputs.
324-347: AlignImageEmbedding.embed_manyattribute typing with actual checks.As with the text embedding methods,
embed_manyfor images annotatesattributewith| bytesbut only acceptsAttributePath | Callableat runtime. Narrowing the type avoids confusion:- attribute: Callable[[Value], bytes] | AttributePath[Value, bytes] | bytes, + attribute: Callable[[Value], bytes] | AttributePath[Value, bytes], @@ - attribute: Callable[[Value], bytes] | AttributePath[Value, bytes] | bytes | None = None, + attribute: Callable[[Value], bytes] | AttributePath[Value, bytes] | None = None,This keeps the overloads and implementation consistent with the allowed selector types.
♻️ Duplicate comments (4)
docs/guides/BasicToolsUse.md (1)
43-43: LGTM - Model references updated consistently.The model name updates across code examples and expected output logs are consistent with the broader documentation update.
Also applies to: 106-106, 136-136, 223-223, 228-228, 305-305, 310-310
src/draive/embedding/types.py (1)
1-79: Document and align the new vector index protocols with implementations.Two follow-ups are still needed on the new public protocols:
Add NumPy-style docstrings for public protocols.
VectorIndexing,VectorSearching, andVectorDeletingare exported in__all__but lack docstrings. Please add short NumPy-style docstrings describing purpose and parameters (model,attribute/query/requirements/limit,values,extra), plus Returns/Raises, similar to other public types.
As per coding guidelines, public symbols insrc/draive/**/*.pyshould be documented.Match parameter defaults to concrete implementations.
The protocol signatures should include the same defaults used by concrete implementations (e.g., inVectorIndex.searchand the Postgres/volatile indexers), to keep structural typing precise:@runtime_checkable class VectorSearching(Protocol): async def __call__[Model: DataModel]( self, model: type[Model], /, *, - query: Sequence[float] | ResourceContent | TextContent | str | None, - score_threshold: float | None, - requirements: AttributeRequirement[Model] | None, - limit: int | None, + query: Sequence[float] | ResourceContent | TextContent | str | None = None, + score_threshold: float | None = None, + requirements: AttributeRequirement[Model] | None = None, + limit: int | None = None, **extra: Any, ) -> Sequence[Model]: ... @@ @runtime_checkable class VectorDeleting(Protocol): async def __call__[Model: DataModel]( self, model: type[Model], /, *, - requirements: AttributeRequirement[Model] | None, + requirements: AttributeRequirement[Model] | None = None, **extra: Any, ) -> None: ...AGENTS.md (1)
1-111: Tighten AGENTS.md to match current APIs and markdown conventions.A few small cleanups will keep this guide aligned with the codebase and coding_guidelines:
- Add a top-level heading (MD041).
The file starts with a plain sentence; markdownlint still expects an H1. Consider:- Draive is a python framework helping to build high-quality Gen-AI applications. Focuses on strict typing and functional programming principles extended with structured concurrency concepts. Delivers opinionated, strict rules and patterns resulting in modular, safe and highly maintainable applications. + # Draive + + Draive is a Python framework helping to build high-quality Gen-AI applications. It focuses on strict typing and functional programming principles extended with structured concurrency concepts. It delivers opinionated, strict rules and patterns resulting in modular, safe and highly maintainable applications.
- Update VectorIndex location in project layout.
VectorIndex has moved underdraive/embedding/, but the layout still mentions it under utils:-- `draive/utils/` — utilities (e.g., `Memory`, `VectorIndex`) +- `draive/utils/` — utilities (e.g., `Memory`, helpers reused across modules)Optionally, you can call out
VectorIndexexplicitly under thedraive/embedding/bullet.
Based on learnings, public API docs should track VectorIndex’s new module.
- Clarify import source for
Stateandctx.
This bullet now saysfrom draive import State, ctx, but the coding_guidelines provided for this review still recommend importing fromhaiway(from haiway import State, ctx). Please align the guidance one way or the other and adjust either this doc or the central guidelines so they don’t conflict.src/draive/multimodal/templates/variables.py (1)
24-59: Tail append check is redundant (already covered previously).When
cursor < len(template),template[cursor:]is guaranteed non-empty, so theif tail:guard is unnecessary and can be removed for simplicity:- if cursor < len(template): - tail: str = template[cursor:] - if tail: - append(tail) + if cursor < len(template): + tail: str = template[cursor:] + append(tail)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
AGENTS.md(3 hunks)Makefile(1 hunks)docs/cookbooks/BasicDataExtraction.md(2 hunks)docs/cookbooks/BasicMCP.md(1 hunks)docs/cookbooks/BasicRAG.md(1 hunks)docs/getting-started/first-steps.md(3 hunks)docs/getting-started/installation.md(1 hunks)docs/getting-started/quickstart.md(3 hunks)docs/guides/BasicConversation.md(2 hunks)docs/guides/BasicEvaluation.md(2 hunks)docs/guides/BasicModelGeneration.md(3 hunks)docs/guides/BasicStageUsage.md(1 hunks)docs/guides/BasicToolsUse.md(5 hunks)docs/guides/BasicUsage.md(6 hunks)docs/guides/ComprehensiveEvaluation.md(2 hunks)docs/guides/MultimodalContent.md(1 hunks)docs/index.md(1 hunks)llms.txt(1 hunks)pyproject.toml(2 hunks)src/draive/__init__.py(2 hunks)src/draive/aws/api.py(1 hunks)src/draive/aws/client.py(1 hunks)src/draive/aws/types.py(1 hunks)src/draive/embedding/__init__.py(1 hunks)src/draive/embedding/state.py(7 hunks)src/draive/embedding/types.py(2 hunks)src/draive/evaluators/utils.py(2 hunks)src/draive/helpers/volatile_vector_index.py(2 hunks)src/draive/multimodal/content.py(3 hunks)src/draive/multimodal/templates/repository.py(1 hunks)src/draive/multimodal/templates/types.py(1 hunks)src/draive/multimodal/templates/variables.py(3 hunks)src/draive/postgres/vector_index.py(1 hunks)src/draive/qdrant/index.py(1 hunks)src/draive/resources/template.py(1 hunks)src/draive/utils/__init__.py(0 hunks)src/draive/utils/vector_index.py(0 hunks)tests/test_evaluators_utils.py(1 hunks)tests/test_multimodal_template_variables.py(1 hunks)tests/test_tags_extraction.py(1 hunks)tests/test_tags_replacement.py(1 hunks)
💤 Files with no reviewable changes (2)
- src/draive/utils/init.py
- src/draive/utils/vector_index.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python 3.13+ for development
Use Ruff formatter for code formatting (viamake format), no other formatter
Run Ruff, Bandit, and Pyright (strict) linters viamake lint
Import symbols fromhaiwaydirectly usingfrom haiway import State, ctx
Use context scoping withctx.scope(...)to bind scopedDisposables, activeStateinstances and avoid global state
All logs must go throughctx.log_*methods; do not use
Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or looseAnyunless required by third-party boundaries
Prefer explicit attribute access with static types; avoid dynamicgetattrexcept at narrow boundaries
Prefer abstract immutable protocols (Mapping,Sequence,Iterable) overdict/list/setin public types
Usefinalwhere applicable; avoid inheritance, prefer type composition
Use precise unions (|) and narrow withmatch/isinstance, avoidcastunless provably safe and localized
Usehaiway.Statefor immutable data/config and service facades, construct with classmethods likeof(...)when ergonomic
Avoid in-place mutation; useState.updated(...)/functional builders to create new instances
Access active state throughhaiway.ctxinside async scopes (ctx.scope(...))
Public state methods that dispatch on the active instance should use@statemethod
Usectx.log_debug/info/warn/errorfor logs; do not use
Log around generation calls, tool dispatch, provider requests/responses without leaking secrets
Add appropriate metrics tracking usingctx.recordwhere applicable
Prefer structured and concise log messages; avoid excessive verbosity in hot paths
All I/O is async, keep boundaries async and usectx.spawnfor detached tasks
Ensure structured concurrency concepts and valid coroutine usage
Rely on haiway and asyncio packages with coroutines, avoid custom threading
Don't raise bareException, preserve contextual information in excep...
Files:
src/draive/aws/client.pysrc/draive/helpers/volatile_vector_index.pytests/test_tags_replacement.pysrc/draive/__init__.pysrc/draive/multimodal/templates/variables.pysrc/draive/evaluators/utils.pysrc/draive/multimodal/content.pysrc/draive/aws/types.pysrc/draive/resources/template.pysrc/draive/embedding/types.pysrc/draive/multimodal/templates/types.pytests/test_tags_extraction.pysrc/draive/aws/api.pysrc/draive/postgres/vector_index.pysrc/draive/multimodal/templates/repository.pytests/test_evaluators_utils.pysrc/draive/embedding/__init__.pysrc/draive/qdrant/index.pytests/test_multimodal_template_variables.pysrc/draive/embedding/state.py
src/draive/**/!(test_)*.py
📄 CodeRabbit inference engine (AGENTS.md)
Translate provider/SDK errors into appropriate typed exceptions
Files:
src/draive/aws/client.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/__init__.pysrc/draive/multimodal/templates/variables.pysrc/draive/evaluators/utils.pysrc/draive/multimodal/content.pysrc/draive/aws/types.pysrc/draive/resources/template.pysrc/draive/embedding/types.pysrc/draive/multimodal/templates/types.pysrc/draive/aws/api.pysrc/draive/postgres/vector_index.pysrc/draive/multimodal/templates/repository.pysrc/draive/embedding/__init__.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.py
src/draive/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Files:
src/draive/aws/client.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/__init__.pysrc/draive/multimodal/templates/variables.pysrc/draive/evaluators/utils.pysrc/draive/multimodal/content.pysrc/draive/aws/types.pysrc/draive/resources/template.pysrc/draive/embedding/types.pysrc/draive/multimodal/templates/types.pysrc/draive/aws/api.pysrc/draive/postgres/vector_index.pysrc/draive/multimodal/templates/repository.pysrc/draive/embedding/__init__.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: No network in unit tests, mock providers/HTTP
Keep tests fast and specific to the code you change, start with unit tests around new types/functions and adapters
Use fixtures fromtests/or add focused ones; avoid heavy integration scaffolding
Usepytest-asynciofor coroutine tests with@pytest.mark.asynciodecorator
Prefer scoping withctx.scope(...)and bind requiredStateinstances explicitly in async tests
Avoid real I/O and network in tests; stub provider calls and HTTP
Files:
tests/test_tags_replacement.pytests/test_tags_extraction.pytests/test_evaluators_utils.pytests/test_multimodal_template_variables.py
src/draive/**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Update
__init__.pyexports and docs when API surface changes
Files:
src/draive/__init__.pysrc/draive/embedding/__init__.py
🧠 Learnings (10)
📚 Learning: 2025-06-16T10:28:07.434Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 338
File: src/draive/lmm/__init__.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Applied to files:
pyproject.tomldocs/getting-started/installation.mdAGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to src/draive/**/__init__.py : Update `__init__.py` exports and docs when API surface changes
Applied to files:
src/draive/aws/client.pyAGENTS.mdsrc/draive/__init__.pysrc/draive/embedding/types.pysrc/draive/aws/api.pysrc/draive/postgres/vector_index.pysrc/draive/embedding/__init__.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Import symbols from `haiway` directly using `from haiway import State, ctx`
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Access active state through `haiway.ctx` inside async scopes (`ctx.scope(...)`)
Applied to files:
AGENTS.md
📚 Learning: 2025-05-28T17:41:57.460Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., `def func[T: Type]()`) is valid and should be used instead of the older TypeVar approach.
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use `haiway.State` for immutable data/config and service facades, construct with classmethods like `of(...)` when ergonomic
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Rely on haiway and asyncio packages with coroutines, avoid custom threading
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or loose `Any` unless required by third-party boundaries
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Build content with `MultimodalContent.of(...)` and prefer composing content blocks explicitly
Applied to files:
src/draive/multimodal/content.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to src/draive/**/*.py : Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Applied to files:
src/draive/embedding/types.py
🧬 Code graph analysis (13)
src/draive/helpers/volatile_vector_index.py (3)
src/draive/embedding/state.py (11)
ImageEmbedding(202-367)embed_many(121-126)embed_many(129-134)embed_many(138-144)embed_many(147-153)embed_many(156-197)embed_many(290-295)embed_many(298-303)embed_many(307-313)embed_many(316-322)embed_many(325-365)src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/resources/types.py (2)
ResourceContent(128-214)to_bytes(207-208)
tests/test_tags_replacement.py (1)
src/draive/multimodal/content.py (4)
MultimodalContent(25-592)of(42-66)of(619-647)replacing_tag(448-581)
src/draive/__init__.py (2)
src/draive/embedding/state.py (1)
VectorIndex(371-587)src/draive/utils/memory.py (1)
Memory(52-143)
src/draive/multimodal/templates/variables.py (1)
src/draive/multimodal/content.py (6)
MultimodalContent(25-592)parts(668-676)to_str(288-297)to_str(653-666)of(42-66)of(619-647)
src/draive/resources/template.py (1)
src/draive/parameters/model.py (3)
get(542-545)get(548-552)get(554-559)
src/draive/multimodal/templates/types.py (1)
tests/test_toolbox.py (2)
meta(109-110)meta(310-311)
tests/test_tags_extraction.py (1)
src/draive/multimodal/content.py (5)
MultimodalContent(25-592)of(42-66)of(619-647)tags(427-446)tag(405-425)
src/draive/postgres/vector_index.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
tests/test_evaluators_utils.py (2)
src/draive/evaluators/utils.py (1)
extract_evaluation_result(25-57)src/draive/multimodal/content.py (5)
MultimodalContent(25-592)MultimodalTag(599-700)tags(427-446)of(42-66)of(619-647)
src/draive/embedding/__init__.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
src/draive/qdrant/index.py (2)
src/draive/embedding/types.py (1)
Embedded(25-28)src/draive/embedding/state.py (3)
ImageEmbedding(202-367)TextEmbedding(30-199)VectorIndex(371-587)
tests/test_multimodal_template_variables.py (1)
src/draive/multimodal/templates/variables.py (1)
parse_template_variables(17-21)
src/draive/embedding/state.py (3)
src/draive/embedding/types.py (5)
Embedded(25-28)ValueEmbedding(32-39)VectorDeleting(71-79)VectorIndexing(43-52)VectorSearching(56-67)src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/resources/types.py (1)
ResourceContent(128-214)
🪛 LanguageTool
AGENTS.md
[grammar] ~34-~34: Ensure spelling is correct
Context: ...e/init.py`. ## Style & Patterns - Draive is built on top of Haiway (state, conte...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
llms.txt
[grammar] ~1-~1: Ensure spelling is correct
Context: # Draive + Haiway LLM Integration Cheat Sheet A...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~3-~3: Ensure spelling is correct
Context: ...ative, concise runtime notes for wiring Draive apps. Targets Python 3.13+, strict typi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~7-~7: Ensure spelling is correct
Context: ...nv. Call load_env()` once at startup; Draive never auto-loads secrets. ```python fro...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (35)
src/draive/multimodal/templates/types.py (1)
88-88: DefaultTemplate.metatoMETA_EMPTYlooks correct and consistentMaking
metadefault toMETA_EMPTYalignsTemplatewithTemplateDeclarationand allows direct construction without explicitly passing metadata, whileTemplate.of(...)still normalizes viaMeta.of(meta). This is consistent with existing uses that treat empty meta as the default and should not change behavior for existing callers.src/draive/resources/template.py (1)
66-68: LGTM! Formatting cleanup with no functional impact.The reformatting of the parameter assignment maintains identical semantics while improving readability.
src/draive/aws/types.py (1)
26-26: LGTM!The blank line improves readability by separating the docstring from the class attributes.
src/draive/aws/client.py (1)
36-36: LGTM!The blank line improves readability and maintains consistency with the formatting applied across AWS module files.
src/draive/aws/api.py (1)
6-6: LGTM!Using a tuple for
__all__is a Python best practice since it's immutable and more efficient than a list.Makefile (1)
10-10: Discrepancy between AI summary and actual code: .env loading removal claim.The AI summary states "Removed loading of .env via Makefile inclusion," but Line 10 still contains
-include .envand is not marked as changed in this PR. Please clarify whether the .env loading removal claim is accurate or if it was already removed in a prior commit.Also applies to: 13-13
docs/guides/ComprehensiveEvaluation.md (2)
13-13: API surface documentation updated to reflect EvaluatorResult.This aligns with the broader API changes described in the PR summary. Ensure that all evaluator examples and tests have been updated to use
EvaluatorResultconsistently.
237-237: Model reference confirmed as production-ready.OpenAI's
gpt-5model launched in production on August 7, 2025, and is currently available across multiple environments including ChatGPT (free/Plus/Pro/Team/Enterprise/Edu tiers) and the OpenAI API with staged access. The documentation reference is accurate for current deployment contexts.docs/guides/BasicStageUsage.md (1)
315-315: Model reference updated consistently.The example now uses
gpt-5. Ensure this model is available in all deployment contexts mentioned in the documentation.docs/index.md (2)
79-79: Quickstart example uses gpt-5-mini model.Ensure this model is available and recommended for new users across all deployment environments.
97-97: VectorIndex API reference updated.Verify that
VectorIndexis properly exported fromdraive.embeddingpublic API as described in the PR summary, so users can follow this guidance without import errors.src/draive/multimodal/templates/repository.py (1)
62-68: Docstring clarified and simplified.The updated docstring is clearer. Ensure it follows the NumPy docstring convention specified in the coding guidelines (Parameters, Returns, Raises sections where applicable).
docs/getting-started/installation.md (1)
43-49: Installation extras documentation significantly expanded with verified coverage.All listed optional extras (
openai_realtime,anthropic_bedrock,cohere_bedrock,bedrock,aws,ollama,vllm,qdrant,postgres,httpx,mcp,opentelemetry,docs) are correctly defined inpyproject.tomlwith exact naming matches. The comprehensive provider and backend coverage enhances user guidance for choosing installation packages.docs/guides/BasicModelGeneration.md (1)
110-111: No action needed. Documentation accurately reflects that gpt-5, gpt-5-mini, and gpt-5-nano are supported OpenAI models with JSON mode/Structured Outputs capability.docs/guides/BasicConversation.md (1)
77-78: API guidance is accurate and properly reflects ConversationMessage API.The documentation correctly uses
response.content.to_str()to render assistant text. Verification confirmsConversationMessagehas acontent: MultimodalContentattribute, andMultimodalContent.to_str()is the supported public method used consistently throughout the codebase. There is noresponse.textattribute in the current API.src/draive/multimodal/content.py (4)
717-743: Good use of__slots__for memory optimization.Adding
slots=Trueto these frozen dataclasses is a good optimization for token objects that may be created in large quantities during parsing.
888-920: Clean refactoring of tokenization with good separation of concerns.The delegation to
_parse_tag_atcentralizes tag parsing logic, and the early return for text without<is a sensible optimization. The fallback to emit<as a text token when parsing fails (lines 911-914) correctly handles malformed tags.
1072-1120: Robust attribute value parsing with proper escape handling.The function correctly handles escape sequences for
\n,\t,\r, quotes, and backslashes. Rejecting raw newlines (lines 1093-1094) is a sensible constraint. The lenient handling of unknown escapes (preserving both backslash and character) is a reasonable choice for forward compatibility.
1123-1137: Clean helper functions for stack management.Both helpers are well-typed and serve clear purposes.
_find_matching_contextcorrectly searches from the most recent context backward, which is essential for proper tag nesting behavior.docs/cookbooks/BasicRAG.md (1)
119-119: Model update consistent with other documentation changes.The model change to
gpt-5-miniis consistent with the broader PR updates. Verification of model validity is covered in the BasicMCP.md review.docs/cookbooks/BasicDataExtraction.md (1)
55-55: Model updates consistent across both extraction examples.Both scopes now use
gpt-5-mini, maintaining consistency with the broader documentation updates in this PR.Also applies to: 82-82
docs/guides/MultimodalContent.md (1)
168-168: Model updated togpt-5for image analysis.The use of
gpt-5(rather thangpt-5-miniused elsewhere) appears intentional for this multimodal/vision example, as image analysis typically requires more capable models. The same verification request from BasicMCP.md applies to confirmgpt-5availability.docs/getting-started/first-steps.md (1)
121-123: LGTM - Guardrails API update is consistent.The change from
ModerationStatetoGuardrailsModeration.of(provider="openai")aligns with the broader guardrails module refactoring visible throughout the PR.src/draive/qdrant/index.py (1)
6-12: LGTM - Import relocation is consistent with the embedding module refactoring.VectorIndex has been successfully moved from
draive.utilstodraive.embedding, and the import statement correctly groups it with other embedding-related symbols (Embedded,ImageEmbedding,TextEmbedding,mmr_vector_similarity_search).src/draive/__init__.py (1)
97-97: LGTM - Public API migration preserves backward compatibility.VectorIndex has been successfully relocated from
draive.utilstodraive.embeddingwhile maintaining its presence in the public API exports. The symbol remains accessible viafrom draive import VectorIndex, ensuring no breaking changes for consumers.Also applies to: 213-213, 371-371
tests/test_multimodal_template_variables.py (1)
15-15: LGTM - Tests correctly adapted to generator-based API.The tests have been properly updated to handle
parse_template_variablesreturning aGenerator[str]instead of a pre-constructed collection. Wrapping the result withset()ensures uniqueness validation while consuming the lazy generator.Also applies to: 21-21, 27-27, 35-35, 43-43, 51-51, 59-59, 67-67, 75-75, 83-83
tests/test_tags_extraction.py (1)
423-477: LGTM - Comprehensive test coverage for self-closing tags with attributes.The new test cases thoroughly validate:
- Attribute extraction (key-value pairs) from self-closing tags
- Interleaving of self-closing and regular tags
- Space before slash variations
- Graceful handling of malformed attribute syntax
The tests follow established patterns and ensure robust parsing behavior.
tests/test_evaluators_utils.py (1)
1-55: LGTM - Comprehensive test coverage for evaluator result parsing.The new test module thoroughly validates
extract_evaluation_resultbehavior:
- Correct parsing of rating and optional comment tags
- Required rating enforcement with clear error messages
- Multiple rating rejection to prevent ambiguity
- Optional comment handling
- Unknown rating value rejection
The test naming accurately reflects behavior (e.g., "rejects" multiple ratings rather than "skips"), and error cases properly use
pytest.raises.docs/getting-started/quickstart.md (1)
36-36: No action needed. The modelgpt-5-miniis valid and available through the OpenAI API as of December 2025, including support for Chat Completions and other endpoints with documented pricing. The documentation examples are accurate.docs/guides/BasicEvaluation.md (1)
67-82: EvaluatorResult note and model config look consistent.The updated OpenAIResponsesConfig model and the clarification about EvaluatorResult/.passed/.comment align with the surrounding example usage; no issues from this hunk.
src/draive/postgres/vector_index.py (1)
10-16: VectorIndex relocation wiring looks correct.Importing
VectorIndexfromdraive.embeddingand annotatingPostgresVectorIndexto return it matches the new embedding API; the factory construction remains consistent withVectorIndex’s fields.Also applies to: 24-40, 319-323
src/draive/embedding/__init__.py (1)
1-16: Public VectorIndex export is correctly wired.Re-exporting
VectorIndexfromdraive.embedding.statevia this__init__and adding it to__all__matches the new embedding API surface and keeps downstream imports (from draive.embedding import VectorIndex) valid.src/draive/helpers/volatile_vector_index.py (1)
7-18: VolatileVectorIndex refactor improves concurrency while maintaining consistent patterns.The refactoring correctly moves embedding work outside the lock, reducing contention while keeping storage mutations guarded. The text vs image separation via
text_values/image_valueswith mixed-type validation mirrors PostgresVectorIndex and QdrantVectorIndex implementations exactly.The assertion requiring
attributeto beAttributePath | Callable(not a literalValue) is consistent across all three VectorIndex implementations in the codebase—Postgres, Qdrant, and Volatile all enforce this same constraint, despite the broader type signature in the interface. All documented examples and usages passattribute=Model._.field_name(AttributePath syntax), confirming this is the established pattern. No changes needed; the code is correct.llms.txt (1)
1-193: Docs and examples look consistent with the updated APIs.The cheat sheet correctly reflects the new primitives (
ctx.scope,MultimodalContent,VectorIndex, guardrails, evaluators) and uses imports and patterns that match the current codebase. No changes needed from a code/API perspective.src/draive/embedding/state.py (1)
370-587: NewVectorIndexwrapper looks correct and idiomatic.The
VectorIndexstate cleanly delegates to injectedindexing/searching/deletingcallables, logs viactx.record_info, and exposes well-typed overloads for index/search/delete that match the correspondingVectorIndexing/VectorSearching/VectorDeletingprotocols. The docstrings cover parameters and return values clearly, and the public API shape aligns with the docs/examples (e.g., usage in the RAG snippet withChunk._.content).No changes needed here.
58dc405 to
63e2a3b
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guides/BasicUsage.md (1)
43-50: Align comments, print labels, and log snippet with new gpt‑5 model namesThe config now uses
model="gpt-5-mini"and later updates tomodel="gpt-5", and the metrics snippet was partially updated to showgpt-5. However, several surrounding comments and print labels still mention GPT‑3.5 / 4o (for example the comment at line 73 andprint("RESULT GPT 4o | temperature 0.4:", result)), and the metrics sample mixes old and new model identifiers. For clarity, it would be good to update these textual labels so they consistently reflect the configured models and the updated token usage keys.Also applies to: 69-105, 169-212
♻️ Duplicate comments (12)
pyproject.toml (1)
8-27: Verifyhaiway~=0.39.2is published before bumping.If 0.39.2 is not yet on PyPI, this constraint will break installs (previous review indicated 0.38.1 as latest). Please re-check haiway releases and pin to a version that actually exists before releasing 0.94.1.
Does the "haiway" package have a 0.39.2 release on PyPI, and if not, what is the latest available version?Makefile (1)
12-14: ReconfirmUV_VERSION := 0.9.14before merging.If 0.9.14 is ahead of the latest official uv release,
uv_checkwill continually re-run the installer and never see the version as “up to date”, as noted in the earlier review. Please verify the latest official uv version and keepUV_VERSIONin sync to avoid flaky tooling.Check the latest officially released version of the "uv" package manager from Astral (GitHub and/or PyPI) and confirm whether 0.9.14 is available.src/draive/multimodal/templates/variables.py (1)
17-21: ParameterizeGeneratorreturn type for strict typing.
Generatorfromcollections.abcrequires three type parameters (YieldType, SendType, ReturnType). The currentGenerator[str]is under-parameterized and will fail Pyright strict mode type checking. Use the fully parameterized form:def parse_template_variables( template: str, -) -> Generator[str]: +) -> Generator[str, None, None]: for match in _PLACEHOLDER_PATTERN.finditer(template): yield match.group(1)This is required to satisfy Pyright strict mode (
typeCheckingMode = "strict"in pyproject.toml) and the repo's strict typing requirements.docs/getting-started/first-steps.md (1)
111-116: FixVectorIndex()instantiation in the retrieval example
VectorIndex()still cannot be instantiated without providing the requiredindexing,searching, anddeletingimplementations. In practice this is supplied by backend-specific factories (e.g.,QdrantVectorIndex(),VolatileVectorIndex(), orPostgresVectorIndex()), so the quickstart example should import and use a concrete factory instead of callingVectorIndex()directly, otherwise copy‑pasted code will fail at runtime.src/draive/embedding/types.py (2)
55-67: Protocol parameters should include defaults to match implementations.The
VectorSearchingprotocol parameters (query,score_threshold,requirements,limit) are defined without default values, but implementations insrc/draive/embedding/state.pyandsrc/draive/helpers/volatile_vector_index.pyuse= Nonedefaults. For@runtime_checkableprotocols, parameter defaults should align to avoid structural conformance mismatches.@runtime_checkable class VectorSearching(Protocol): async def __call__[Model: DataModel]( self, model: type[Model], /, *, - query: Sequence[float] | ResourceContent | TextContent | str | None, - score_threshold: float | None, - requirements: AttributeRequirement[Model] | None, - limit: int | None, + query: Sequence[float] | ResourceContent | TextContent | str | None = None, + score_threshold: float | None = None, + requirements: AttributeRequirement[Model] | None = None, + limit: int | None = None, **extra: Any, ) -> Sequence[Model]: ...
70-79: Protocol parameter should include default to match implementations.Similar to
VectorSearching, therequirementsparameter should have a default value ofNoneto align with implementation signatures.@runtime_checkable class VectorDeleting(Protocol): async def __call__[Model: DataModel]( self, model: type[Model], /, *, - requirements: AttributeRequirement[Model] | None, + requirements: AttributeRequirement[Model] | None = None, **extra: Any, ) -> None: ...src/draive/multimodal/content.py (2)
935-936: Unnecessary variable alias.
text_localis assigned as an alias totextbuttextis never reassigned within this function. This alias adds no value and slightly reduces readability.- text_local = textThen replace
text_localwithtextat lines 982, 993, and 1010.
1040-1054: Redundant condition in_meta_from_attrs.When
restis notNone, it is initialized in_parse_tag_atat line 1018 withattrs = {first_key: first_val}, sofirst_keyis always already inrest. The condition at lines 1051-1052 never evaluates toTrue, making it dead code.While harmless, removing it improves clarity:
def _meta_from_attrs( first_key: str | None, first_val: str, rest: dict[str, str] | None, ) -> Meta: if first_key is None: return META_EMPTY if rest is None: return Meta.of({first_key: first_val}) - if first_key not in rest: - rest[first_key] = first_val - return Meta.of(rest)src/draive/evaluators/utils.py (2)
25-57: Add NumPy-style docstring and use explicitis Nonecheck.Two issues persist from prior review:
Missing docstring:
extract_evaluation_resultis a public API but lacks a NumPy-style docstring (Parameters/Returns/Raises).Falsy check on line 45:
if not rating_tag:is fragile ifMultimodalTagever implements__bool__. Prefer explicit identity:- if not rating_tag: + if rating_tag is None: raise ValueError(f"Invalid evaluator result - missing rating:\n{content}")
14-22: Align format instructions with parser behavior.
FORMAT_INSTRUCTIONinstructs "Do not leave empty." for the comment tag, yetextract_evaluation_resulttreats the comment as optional (line 52 gracefully handlesNone). Either tighten the parser to require a non-empty comment or relax the instruction text to indicate the comment is optional but recommended.src/draive/embedding/state.py (2)
82-100: Consider documentingRaisessection.The docstring is well-structured but omits the
Raisessection. The method can raiseAssertionErrorwhenattributeis provided but is neither anAttributePathnor a callable (line 106-108).
496-518: Consider addingRaisessection to search docstring.The search method can raise backend-specific exceptions. Per coding guidelines, documenting potential exceptions improves API clarity:
Returns ------- Sequence[Model] Matching model instances ordered by similarity. + + Raises + ------ + Exception + Backend-specific errors during search execution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
AGENTS.md(3 hunks)Makefile(1 hunks)docs/cookbooks/BasicDataExtraction.md(2 hunks)docs/cookbooks/BasicMCP.md(1 hunks)docs/cookbooks/BasicRAG.md(1 hunks)docs/getting-started/first-steps.md(3 hunks)docs/getting-started/installation.md(1 hunks)docs/getting-started/multimodal-data.md(4 hunks)docs/getting-started/printing-data.md(2 hunks)docs/getting-started/quickstart.md(3 hunks)docs/guides/BasicConversation.md(2 hunks)docs/guides/BasicEvaluation.md(2 hunks)docs/guides/BasicModelGeneration.md(3 hunks)docs/guides/BasicStageUsage.md(1 hunks)docs/guides/BasicToolsUse.md(5 hunks)docs/guides/BasicUsage.md(6 hunks)docs/guides/ComprehensiveEvaluation.md(2 hunks)docs/guides/MultimodalContent.md(1 hunks)docs/guides/Qdrant.md(5 hunks)docs/index.md(1 hunks)llms.txt(1 hunks)pyproject.toml(2 hunks)src/draive/__init__.py(2 hunks)src/draive/aws/api.py(1 hunks)src/draive/aws/client.py(1 hunks)src/draive/aws/types.py(1 hunks)src/draive/embedding/__init__.py(1 hunks)src/draive/embedding/state.py(7 hunks)src/draive/embedding/types.py(2 hunks)src/draive/evaluators/utils.py(2 hunks)src/draive/helpers/volatile_vector_index.py(2 hunks)src/draive/multimodal/content.py(3 hunks)src/draive/multimodal/templates/repository.py(1 hunks)src/draive/multimodal/templates/types.py(1 hunks)src/draive/multimodal/templates/variables.py(3 hunks)src/draive/postgres/vector_index.py(1 hunks)src/draive/qdrant/index.py(1 hunks)src/draive/resources/template.py(1 hunks)src/draive/utils/__init__.py(0 hunks)src/draive/utils/vector_index.py(0 hunks)tests/test_evaluators_utils.py(1 hunks)tests/test_multimodal_template_variables.py(1 hunks)tests/test_tags_extraction.py(1 hunks)tests/test_tags_replacement.py(1 hunks)
💤 Files with no reviewable changes (2)
- src/draive/utils/init.py
- src/draive/utils/vector_index.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python 3.13+ for development
Use Ruff formatter for code formatting (viamake format), no other formatter
Run Ruff, Bandit, and Pyright (strict) linters viamake lint
Import symbols fromhaiwaydirectly usingfrom haiway import State, ctx
Use context scoping withctx.scope(...)to bind scopedDisposables, activeStateinstances and avoid global state
All logs must go throughctx.log_*methods; do not use
Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or looseAnyunless required by third-party boundaries
Prefer explicit attribute access with static types; avoid dynamicgetattrexcept at narrow boundaries
Prefer abstract immutable protocols (Mapping,Sequence,Iterable) overdict/list/setin public types
Usefinalwhere applicable; avoid inheritance, prefer type composition
Use precise unions (|) and narrow withmatch/isinstance, avoidcastunless provably safe and localized
Usehaiway.Statefor immutable data/config and service facades, construct with classmethods likeof(...)when ergonomic
Avoid in-place mutation; useState.updated(...)/functional builders to create new instances
Access active state throughhaiway.ctxinside async scopes (ctx.scope(...))
Public state methods that dispatch on the active instance should use@statemethod
Usectx.log_debug/info/warn/errorfor logs; do not use
Log around generation calls, tool dispatch, provider requests/responses without leaking secrets
Add appropriate metrics tracking usingctx.recordwhere applicable
Prefer structured and concise log messages; avoid excessive verbosity in hot paths
All I/O is async, keep boundaries async and usectx.spawnfor detached tasks
Ensure structured concurrency concepts and valid coroutine usage
Rely on haiway and asyncio packages with coroutines, avoid custom threading
Don't raise bareException, preserve contextual information in excep...
Files:
src/draive/aws/client.pysrc/draive/embedding/__init__.pysrc/draive/__init__.pysrc/draive/multimodal/templates/repository.pysrc/draive/qdrant/index.pysrc/draive/postgres/vector_index.pysrc/draive/helpers/volatile_vector_index.pytests/test_multimodal_template_variables.pytests/test_tags_replacement.pysrc/draive/resources/template.pysrc/draive/evaluators/utils.pysrc/draive/aws/api.pytests/test_tags_extraction.pytests/test_evaluators_utils.pysrc/draive/multimodal/templates/types.pysrc/draive/aws/types.pysrc/draive/embedding/types.pysrc/draive/multimodal/templates/variables.pysrc/draive/multimodal/content.pysrc/draive/embedding/state.py
src/draive/**/!(test_)*.py
📄 CodeRabbit inference engine (AGENTS.md)
Translate provider/SDK errors into appropriate typed exceptions
Files:
src/draive/aws/client.pysrc/draive/embedding/__init__.pysrc/draive/__init__.pysrc/draive/multimodal/templates/repository.pysrc/draive/qdrant/index.pysrc/draive/postgres/vector_index.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/resources/template.pysrc/draive/evaluators/utils.pysrc/draive/aws/api.pysrc/draive/multimodal/templates/types.pysrc/draive/aws/types.pysrc/draive/embedding/types.pysrc/draive/multimodal/templates/variables.pysrc/draive/multimodal/content.pysrc/draive/embedding/state.py
src/draive/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Files:
src/draive/aws/client.pysrc/draive/embedding/__init__.pysrc/draive/__init__.pysrc/draive/multimodal/templates/repository.pysrc/draive/qdrant/index.pysrc/draive/postgres/vector_index.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/resources/template.pysrc/draive/evaluators/utils.pysrc/draive/aws/api.pysrc/draive/multimodal/templates/types.pysrc/draive/aws/types.pysrc/draive/embedding/types.pysrc/draive/multimodal/templates/variables.pysrc/draive/multimodal/content.pysrc/draive/embedding/state.py
src/draive/**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Update
__init__.pyexports and docs when API surface changes
Files:
src/draive/embedding/__init__.pysrc/draive/__init__.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: No network in unit tests, mock providers/HTTP
Keep tests fast and specific to the code you change, start with unit tests around new types/functions and adapters
Use fixtures fromtests/or add focused ones; avoid heavy integration scaffolding
Usepytest-asynciofor coroutine tests with@pytest.mark.asynciodecorator
Prefer scoping withctx.scope(...)and bind requiredStateinstances explicitly in async tests
Avoid real I/O and network in tests; stub provider calls and HTTP
Files:
tests/test_multimodal_template_variables.pytests/test_tags_replacement.pytests/test_tags_extraction.pytests/test_evaluators_utils.py
🧠 Learnings (12)
📚 Learning: 2025-06-16T10:28:07.434Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 338
File: src/draive/lmm/__init__.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Applied to files:
docs/getting-started/installation.mdAGENTS.mdpyproject.toml
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to src/draive/**/__init__.py : Update `__init__.py` exports and docs when API surface changes
Applied to files:
src/draive/embedding/__init__.pysrc/draive/__init__.pysrc/draive/qdrant/index.pysrc/draive/postgres/vector_index.pyAGENTS.mdsrc/draive/aws/api.pysrc/draive/embedding/types.pysrc/draive/embedding/state.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use context scoping with `ctx.scope(...)` to bind scoped `Disposables`, active `State` instances and avoid global state
Applied to files:
docs/guides/Qdrant.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Build content with `MultimodalContent.of(...)` and prefer composing content blocks explicitly
Applied to files:
docs/getting-started/multimodal-data.mddocs/getting-started/printing-data.mdsrc/draive/multimodal/content.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Import symbols from `haiway` directly using `from haiway import State, ctx`
Applied to files:
AGENTS.md
📚 Learning: 2025-05-28T17:41:57.460Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., `def func[T: Type]()`) is valid and should be used instead of the older TypeVar approach.
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use `haiway.State` for immutable data/config and service facades, construct with classmethods like `of(...)` when ergonomic
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Access active state through `haiway.ctx` inside async scopes (`ctx.scope(...)`)
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Rely on haiway and asyncio packages with coroutines, avoid custom threading
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or loose `Any` unless required by third-party boundaries
Applied to files:
AGENTS.mdsrc/draive/multimodal/templates/variables.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use `ResourceContent`/`ResourceReference` for media and data blobs
Applied to files:
docs/getting-started/printing-data.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to src/draive/**/*.py : Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Applied to files:
src/draive/embedding/types.py
🧬 Code graph analysis (13)
src/draive/embedding/__init__.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
src/draive/postgres/vector_index.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
src/draive/helpers/volatile_vector_index.py (3)
src/draive/embedding/state.py (12)
VectorIndex(371-587)ImageEmbedding(202-367)embed_many(121-126)embed_many(129-134)embed_many(138-144)embed_many(147-153)embed_many(156-197)embed_many(290-295)embed_many(298-303)embed_many(307-313)embed_many(316-322)embed_many(325-365)src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/resources/types.py (2)
ResourceContent(128-214)to_bytes(207-208)
tests/test_multimodal_template_variables.py (1)
src/draive/multimodal/templates/variables.py (1)
parse_template_variables(17-21)
tests/test_tags_replacement.py (1)
src/draive/multimodal/content.py (4)
MultimodalContent(25-592)of(42-66)of(619-647)replacing_tag(448-581)
src/draive/resources/template.py (2)
src/draive/parameters/model.py (3)
get(542-545)get(548-552)get(554-559)src/draive/stages/types.py (4)
get(140-144)get(147-153)get(156-162)get(164-206)
src/draive/evaluators/utils.py (1)
src/draive/multimodal/content.py (7)
MultimodalContent(25-592)MultimodalTag(599-700)tag(405-425)of(42-66)of(619-647)to_str(288-297)to_str(653-666)
tests/test_tags_extraction.py (1)
src/draive/multimodal/content.py (3)
MultimodalContent(25-592)of(42-66)of(619-647)
tests/test_evaluators_utils.py (2)
src/draive/evaluators/utils.py (1)
extract_evaluation_result(25-57)src/draive/multimodal/content.py (5)
MultimodalContent(25-592)MultimodalTag(599-700)tags(427-446)of(42-66)of(619-647)
src/draive/multimodal/templates/types.py (1)
tests/test_toolbox.py (2)
meta(109-110)meta(310-311)
src/draive/embedding/types.py (3)
src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/parameters/model.py (1)
DataModel(254-634)src/draive/resources/types.py (1)
ResourceContent(128-214)
src/draive/multimodal/templates/variables.py (1)
src/draive/multimodal/content.py (6)
MultimodalContent(25-592)parts(668-676)to_str(288-297)to_str(653-666)of(42-66)of(619-647)
src/draive/multimodal/content.py (1)
src/draive/resources/types.py (9)
of(77-94)of(103-120)of(131-138)of(142-149)of(152-177)of(220-230)of(234-244)of(248-257)of(260-295)
🪛 LanguageTool
docs/getting-started/multimodal-data.md
[style] ~174-~174: Using many exclamation marks might seem excessive (in this case: 15 exclamation marks for a text that’s 2748 characters long)
Context: ...400px; margin: auto; display: block;"} !!! tip ``` There are ready-to-use methods...
(EN_EXCESSIVE_EXCLAMATION)
AGENTS.md
[grammar] ~34-~34: Ensure spelling is correct
Context: ...e/init.py`. ## Style & Patterns - Draive is built on top of Haiway (state, conte...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
llms.txt
[grammar] ~1-~1: Ensure spelling is correct
Context: # Draive + Haiway LLM Integration Cheat Sheet A...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~3-~3: Ensure spelling is correct
Context: ...ative, concise runtime notes for wiring Draive apps. Targets Python 3.13+, strict typi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~7-~7: Ensure spelling is correct
Context: ...nv. Call load_env()` once at startup; Draive never auto-loads secrets. ```python fro...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/getting-started/multimodal-data.md
80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AGENTS.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (34)
src/draive/aws/client.py (1)
19-38: Blank line before__slots__is fine.The added spacing improves readability of the class body and has no behavioral impact.
src/draive/aws/types.py (1)
14-31:AWSError.__slots__now correctly includesuri.Adding
"uri"to__slots__matches the attributes assigned in__init__and prevents slot-relatedAttributeErrorwhen settingself.uri.src/draive/multimodal/templates/types.py (1)
86-89: DefaultingTemplate.metatoMETA_EMPTYis a good consistency win.This aligns
TemplatewithTemplateDeclaration, allows direct construction without explicitly passing meta, and preserves normalization viaTemplate.of(...).docs/getting-started/installation.md (1)
41-52: Extras overview is clear and matches defined optionals.The grouped bullets map cleanly onto the extras in
pyproject.tomland make it easy for users to pick provider/vector/utility combinations.llms.txt (1)
1-193: Cheat sheet structure and API references look consistent.The rewritten cheat sheet cleanly mirrors the current async/ctx/Multimodal/VectorIndex surfaces and emphasizes the same execution rules as in the coding guidelines; nothing stands out as inconsistent or misleading.
docs/guides/BasicConversation.md (1)
52-78: Conversation example now matches current model and content API.Using
OpenAIResponsesConfig(model="gpt-5-mini")and recommendingresponse.content.to_str()for assistant text is aligned with the rest of the docs and the MultimodalContent API.src/draive/aws/api.py (1)
6-6: Export tuple for__all__is fine.Switching
__all__to a tuple is stylistic and keeps AWSAPI’s public surface unchanged; no issues here.docs/guides/MultimodalContent.md (1)
165-179: Model update in multimodal example is consistent.Using
OpenAIResponsesConfig(model="gpt-5")in the image analysis example aligns with the broader documentation shift to GPT‑5 family models.docs/guides/BasicToolsUse.md (1)
40-45: Tooling examples now consistently usegpt-5-mini.All updated snippets (basic tools, Toolbox, observability trace) reference
OpenAIResponsesConfig(model="gpt-5-mini")and matching token usage, keeping the guide consistent with the rest of the docs.Also applies to: 100-108, 133-137, 222-229, 305-311
src/draive/__init__.py (1)
92-101: Top-level exports forVectorIndexandMemoryare coherent with new layout.Importing
VectorIndexfromdraive.embeddingandMemoryfromdraive.utilswhile keeping both indraive.__all__preserves the public surface and aligns with the new embedding/utils split.Also applies to: 213-213, 271-272, 371-371
docs/getting-started/quickstart.md (1)
33-37: Quickstart OpenAI examples consistently targetgpt-5-mini.All updated scopes now use
OpenAIResponsesConfig(model="gpt-5-mini"), keeping the quickstart aligned with the rest of the documentation and the recommended default model family.Also applies to: 60-65, 87-91
docs/guides/ComprehensiveEvaluation.md (2)
13-14: EvaluatorResult.performance wording looks consistentThe note about
EvaluatorResult.performanceas a percentage (possibly exceeding 100) is clear and consistent with the surrounding description of thresholds. No changes needed.
233-239: Model name update togpt-5is coherentUsing
"gpt-5"in theevaluation_modelmetadata matches the broader docs shift to the gpt‑5 family and keeps examples consistent. Looks good.tests/test_multimodal_template_variables.py (1)
15-84: Set-based assertions correctly enforce uniqueness without over-constraining orderSwitching to
set(parse_template_variables(...))in these tests makes them robust to ordering and focuses them on uniqueness of placeholder names, which matches the test intent. No issues from a behavior or coverage standpoint.docs/getting-started/printing-data.md (2)
43-47: Code-fencing thekindnote improves clarityWrapping the
kindexplanation in a fenced block makes it stand out as a literal reference/value description. Rendering remains valid; no issues.
132-136: Code-fencing the MultimodalTag metadata note is appropriatePresenting the MultimodalTag metadata explanation as a fenced block reads clearly and doesn’t affect surrounding structure. Looks good.
src/draive/qdrant/index.py (1)
6-12: VectorIndex import/annotation correctly aligned with embedding APIImporting
VectorIndexfromdraive.embeddingand annotatingQdrantVectorIndex()with that type matches the new embedding‑level ownership of the vector index abstraction. The factory still wiresindexing/searching/deletingcorrectly, so behavior is unchanged.Also applies to: 22-23
src/draive/resources/template.py (1)
60-72: Refactored parameter lookup keeps behavior unchangedThe single-expression lookup for
parameteris a pure formatting refactor and preserves the original semantics for resolving by name or alias. No functional or typing concerns here.docs/getting-started/first-steps.md (1)
12-18: gpt‑5 and GuardrailsModeration updates read correctlyThe changes to use
OpenAIResponsesConfig(model="gpt-5-mini")and to switch the moderation example toGuardrailsModeration.of(provider="openai")are coherent with the broader move to the gpt‑5 family and guardrails‑based moderation. Apart from the VectorIndex instantiation noted separately, these snippets look consistent.Also applies to: 59-67, 121-125
src/draive/embedding/__init__.py (1)
4-12: LGTM!The
VectorIndeximport and export are correctly added to the embedding package's public API surface, consistent with the relocation fromdraive.utils. Based on learnings,__init__.pyexports are properly updated when API surface changes.tests/test_tags_replacement.py (1)
668-682: LGTM!The new tests provide good coverage for self-closing tag replacement with attributes. The assertions correctly verify both
strip_tags=Falseandstrip_tags=Truemodes for valid self-closing tags. The malformed tag test appropriately verifies that invalid attribute syntax results in no replacement.docs/guides/BasicEvaluation.md (1)
67-67: Verify the model name "gpt-5-mini".The model name was changed to
gpt-5-mini. This appears to be a hypothetical future model. If this is intentional for documentation purposes, consider adding a note that users should substitute their actual model. Otherwise, verify this isn't a typo for an existing model likegpt-4o-mini.tests/test_tags_extraction.py (1)
423-476: LGTM!Comprehensive test coverage for self-closing tags with attributes:
- Attribute extraction verification (
src,altmetadata)- Mixed content with both self-closing and regular tags
- Space-before-slash handling (
<tag attr="val" />)- Malformed attribute syntax properly skipped while preserving valid tag discovery
Tests are well-organized and assertions are clear.
src/draive/postgres/vector_index.py (1)
10-16: LGTM!The
VectorIndeximport correctly updated to use the new location indraive.embedding, consistent with the API surface relocation across the codebase.src/draive/helpers/volatile_vector_index.py (1)
34-89: LGTM! Lock contention reduction is a good improvement.The refactoring correctly moves the expensive embedding operations (
ImageEmbedding.embed_many/TextEmbedding.embed_many) outside the lock, significantly reducing lock hold time. The lock now only protects the storage mutation (extend), which is the minimal critical section needed.The validation and value selection logic is clear:
- Validate attribute selector type
- Partition values into text vs image lists
- Reject mixed-type selections
- Perform embedding (outside lock)
- Build embedded models
- Acquire lock only for storage update
src/draive/multimodal/content.py (3)
717-743: LGTM! Good memory optimization withslots=True.Adding
slots=Trueto the token dataclasses reduces memory overhead, which is beneficial since many token instances may be created during content parsing.
888-920: LGTM! Efficient tokenization with early return and cursor-based parsing.The refactored
_tokenize_textfunction has good optimizations:
- Early return when no
<character exists (line 892-893)- Cursor-based iteration with direct
find()calls- Unified tag parsing via
_parse_tag_at
923-1022: Well-structured unified tag parser.The
_parse_tag_atfunction cleanly handles all tag variants (opening, closing, self-closing) with and without attributes. The fast paths for no-attribute cases (lines 960-965) are a good optimization. Escape sequence handling in_parse_attr_valuecorrectly processes\n,\t,\r,\", and\\.src/draive/embedding/state.py (6)
1-27: LGTM!Imports are well-organized, and
__all__correctly exportsVectorIndexalongside the existing embedding classes. The use offinalfrom typing and protocol types fromdraive.embedding.typesfollows project conventions.
202-367: LGTM!
ImageEmbeddingfollows the same well-structured pattern asTextEmbeddingwith proper docstrings. The implementation is consistent and the overloads provide good type safety for callers.
420-432: LGTM!The
indexmethod properly logs the indexing event with relevant attributes (model name, values count) before delegating to the underlying implementation. This follows the project's observability conventions.
519-532: LGTM!The
searchmethod correctly logs the search event and cleanly delegates to the searching protocol. The explicitresultsvariable before return aids debugging.
575-583: LGTM!The
deletemethod follows the same logging and delegation pattern consistently withindexandsearch.
585-587: LGTM!Protocol attributes (
VectorIndexing,VectorSearching,VectorDeleting) are correctly typed and provide clear extension points for backend implementations.
63e2a3b to
b867fca
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/draive/resources/template.py (1)
26-27: Add NumPy-style docstrings to public symbols.The public exports (
ResourceAvailabilityCheck,ResourceTemplate,resource) and their public methods lack docstrings. Per coding guidelines, all public symbols must include NumPy-style docstrings with Parameters, Returns, and Raises sections.Apply docstrings to the following public symbols:
class ResourceAvailabilityCheck(Protocol): """ Protocol for checking resource availability. This callable checks whether a resource is currently available. """ def __call__(self) -> bool: ...@final class ResourceTemplate[**Args]( Function[Args, Coroutine[None, None, Sequence[ResourceReference] | ResourceContent]] ): """ URI template handler for resources. Manages RFC 6570 URI templates with parameter validation, expansion, and extraction. Resolves template-based resources by expanding templates with provided arguments. Parameters ---------- template_uri : str RFC 6570 URI template string with parameters. name : str Human-readable name for the resource. description : str | None Optional description of the resource. mime_type : MimeType | None Optional MIME type of the resolved resource. availability_check : ResourceAvailabilityCheck | None Optional callable to check resource availability (defaults to always available). meta : Meta Metadata for the resource declaration. function : Callable[Args, ...] The decorated function that resolves the resource content. """ ...Also add docstrings to public methods:
resolve(): Resolves the resource by expanding template with provided argumentsmatches_uri(): Checks if a URI matches the template patternresolve_from_uri(): Resolves a resource from a URI by extracting and coercing parametersAnd the
resource()decorator function with its Parameters and Returns sections.Also applies to: 30-587, 613-638
docs/guides/BasicUsage.md (2)
68-104: Update comments to match the new model identifiers.The code has been updated to use gpt-5-mini and gpt-5, but several comments still reference the old model names:
- Line 73: "now we are using gpt-3.5-turbo" should be "now we are using gpt-5-mini"
- Line 93: "RESULT GPT 4o" should be "RESULT GPT 5"
- Line 97: "here we are using gpt-3.5-turbo" should be "here we are using gpt-5-mini"
- Line 104: "RESULT GPT 3.5" should be "RESULT GPT 5-mini"
148-180: Update example log output to reflect new model identifiers.The log output at line 170 shows
model: "gpt-3.5-turbo"and line 175 shows[gpt-3.5-turbo-0125], but the configuration at line 129 usesgpt-5-mini. Update the example output to match the new model configuration.
♻️ Duplicate comments (15)
Makefile (1)
12-14: ReconfirmUV_VERSIONpoints to an officially released uv buildA previous review already flagged
0.9.14as not yet officially released on GitHub/PyPI at that time. This Makefile still pins to0.9.14, so unless the official installer now serves that version,uv_checkwill keep trying to “upgrade” on every run and could encourage use of non-official mirrors.I’d either:
- Align
UV_VERSIONwith the latest official release (e.g.,0.9.13as per the earlier review), or- Explicitly confirm that
0.9.14is now published by Astral and update any docs/comments accordingly.You can quickly re-verify with something like:
#!/bin/bash # Check latest uv version on PyPI and GitHub echo "PyPI uv latest:" curl -s https://pypi.org/pypi/uv/json | jq -r '.info.version' echo "GitHub uv releases (top 5):" gh release list --repo astral-sh/uv --limit 5pyproject.toml (1)
27-27: Verify haiway 0.39.2 availability on PyPI.A previous review comment flagged that haiway 0.39.2 does not exist on PyPI (the latest was 0.38.1 as of November 2025). Please confirm that version 0.39.2 has been published before merging, otherwise dependency resolution will fail.
Run the following script to check the latest available version:
#!/bin/bash # Check PyPI for the latest haiway version curl -s https://pypi.org/pypi/haiway/json | jq -r '.info.version' # List all available versions curl -s https://pypi.org/pypi/haiway/json | jq -r '.releases | keys[]' | sort -V | tail -5docs/guides/MultimodalContent.md (1)
168-168: Verify OpenAI model identifier gpt-5.Please confirm that
gpt-5is a valid and available OpenAI model identifier.docs/guides/BasicStageUsage.md (1)
315-315: Verify OpenAI model identifier gpt-5.Please confirm that
gpt-5is a valid and available OpenAI model identifier.docs/getting-started/multimodal-data.md (1)
22-26: Add languages to non-code fences to satisfy MD040.The explanatory blocks under
!!! Note/!!! Tipstill use bare code fences, which keepsmarkdownlintMD040 failing. Tag them astext(or drop the fences) so they are no longer treated as untyped code blocks.Example fix:
-!!! Note - -``` -Constructors and helpers such as `MultimodalContent.of(*elements: "Multimodal")` use the `Multimodal` alias to normalize any mix of multimodal parts into one consistent `MultimodalContent`. -``` +!!! Note + +```text +Constructors and helpers such as `MultimodalContent.of(*elements: "Multimodal")` use the `Multimodal` alias to normalize any mix of multimodal parts into one consistent `MultimodalContent`. +```Apply the same pattern to the other explanatory fences:
- Lines 111‑113 (
MultimodalTagproduces text parts...)- Lines 145‑146 (
MultimodalContenthas more ready‑to‑use methods...)- Lines 161‑162 (note about
ModelToolResponsecontent)- Lines 178‑179 (note about methods like
without_tools()/reasoning()).Also applies to: 109-113, 142-146, 158-162, 175-179
tests/test_evaluators_utils.py (1)
11-55: Evaluator result tests look solid; consider one small tidy-up and an extra edge case.The suite exercises the key paths in
extract_evaluation_result(rating+comment, missing rating, multiple ratings, no comment, invalid rating). Two optional tightenings:
- In
test_extract_evaluation_result_does_not_require_comment, callextract_evaluation_result(content)once, store the result, and assert on both.valueand.meta["comment"]to avoid duplicate work.- Add a dedicated test for the “multiple comments” case (two
name="comment"tags plus one valid rating) to mirror the guard in the implementation.tests/test_tags_replacement.py (1)
668-682: Self-closing tag replacement tests cover the main paths; one optional extra assertion.The new tests correctly verify:
- A valid self-closing
<img .../>becomes<img ...>X</img>(or plainXwithstrip_tags=True).- Malformed
<img src=logo/>is ignored byreplacing_tag.For completeness, you could also assert that
content.replacing_tag("img", replacement="X", strip_tags=True)is unchanged intest_replacing_ignores_malformed_self_closing_tag, mirroring the non‑stripping case.src/draive/embedding/types.py (1)
42-79: NumPy-style docstrings and default parameter values still missing.The protocols
VectorIndexing,VectorSearching, andVectorDeletingcontinue to lack:
- NumPy-style docstrings (Parameters/Returns/Raises sections) as required by coding guidelines for public symbols
- Default parameter values (
= None) to match implementation signatures and ensure structural typing conformancedocs/guides/BasicEvaluation.md (1)
20-85: Documentation correctly describes the evaluator decorator pattern.The narrative and code are aligned:
- Evaluator functions return
EvaluationScore(lines 27-45)- The
@evaluatordecorator wraps that intoEvaluatorResult(line 41 comment)- Calling the decorated evaluator returns
EvaluatorResult(lines 74-81)This is standard decorator pattern and correctly documented.
AGENTS.md (2)
34-36: Clarify canonical import path forState/ctx.The guidance now recommends
from draive import State, ctx, but the codebase still imports fromhaiwaydirectly in multiple places (e.g.,from haiway import State, ctx). Please either:
- Update the guidance to reflect the actual convention (
from haiway import ...), or- Confirm that
draivere-exports these symbols and update imports across the codebase for consistency.#!/bin/bash # Check current import patterns for State and ctx echo "=== Imports from haiway ===" rg -n "from haiway import.*\bState\b" --type=py | head -20 echo "" echo "=== Imports from draive ===" rg -n "from draive import.*\bState\b" --type=py | head -20
1-1: Consider adding a top-level heading.Static analysis flagged MD041 (first line should be a top-level heading). Adding
# Draiveas the first line would satisfy strict markdown linting if desired.+# Draive + Draive is a python framework helping to build high-quality Gen-AI applications...src/draive/evaluators/utils.py (2)
25-57: Add NumPy-style docstring toextract_evaluation_result.Per coding guidelines, public functions should have docstrings with Parameters/Returns/Raises sections.
def extract_evaluation_result( content: MultimodalContent, /, ) -> EvaluationScore: + """Extract rating and optional comment from evaluator output. + + Parameters + ---------- + content + Multimodal content containing rating and comment tags. + + Returns + ------- + EvaluationScore + Parsed score with optional comment in metadata. + + Raises + ------ + ValueError + When rating tag is missing, duplicated, or contains an invalid value. + Also raised when comment tag appears more than once. + """ rating_tag: MultimodalTag | None = None
45-46: Use explicitis Nonecheck forrating_tag.Relying on
if not rating_tag:is fragile ifMultimodalTagever defines a falsy__bool__. Prefer explicit identity checks forNonecomparisons.- if not rating_tag: + if rating_tag is None: raise ValueError(f"Invalid evaluator result - missing rating:\n{content}")src/draive/multimodal/templates/variables.py (1)
17-21: Fully parameterizeGeneratorreturn type for strict typing.
Generator[str]is under-parameterized; strict type checkers expectGenerator[YieldType, SendType, ReturnType]. UseGenerator[str, None, None]to satisfy pyright strict mode.def parse_template_variables( template: str, -) -> Generator[str]: +) -> Generator[str, None, None]: for match in _PLACEHOLDER_PATTERN.finditer(template): yield match.group(1)src/draive/embedding/state.py (1)
370-372: Consider expanding VectorIndex class docstring.The docstring is minimal. Per coding guidelines, public symbols should have more comprehensive NumPy-style docstrings describing the delegation pattern and how callers configure the underlying protocols.
@final class VectorIndex(State): - """Vector index/store implementation handling index/search/delete operations.""" + """Vector index/store facade for embedding-based retrieval. + + Delegates indexing, searching, and deletion to pluggable protocol + implementations (``VectorIndexing``, ``VectorSearching``, ``VectorDeleting``). + Configure these via state binding in a ``ctx.scope(...)``. + + Attributes + ---------- + indexing : VectorIndexing + Protocol implementation for indexing operations. + searching : VectorSearching + Protocol implementation for similarity search. + deleting : VectorDeleting + Protocol implementation for entry removal. + """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
AGENTS.md(3 hunks)Makefile(1 hunks)docs/cookbooks/BasicDataExtraction.md(2 hunks)docs/cookbooks/BasicMCP.md(1 hunks)docs/cookbooks/BasicRAG.md(1 hunks)docs/getting-started/first-steps.md(3 hunks)docs/getting-started/installation.md(1 hunks)docs/getting-started/multimodal-data.md(4 hunks)docs/getting-started/printing-data.md(2 hunks)docs/getting-started/quickstart.md(3 hunks)docs/guides/BasicConversation.md(2 hunks)docs/guides/BasicEvaluation.md(5 hunks)docs/guides/BasicModelGeneration.md(3 hunks)docs/guides/BasicStageUsage.md(1 hunks)docs/guides/BasicToolsUse.md(5 hunks)docs/guides/BasicUsage.md(6 hunks)docs/guides/ComprehensiveEvaluation.md(2 hunks)docs/guides/MultimodalContent.md(1 hunks)docs/guides/Qdrant.md(5 hunks)docs/index.md(1 hunks)llms.txt(1 hunks)pyproject.toml(2 hunks)src/draive/__init__.py(2 hunks)src/draive/aws/api.py(1 hunks)src/draive/aws/client.py(1 hunks)src/draive/aws/types.py(1 hunks)src/draive/embedding/__init__.py(1 hunks)src/draive/embedding/state.py(7 hunks)src/draive/embedding/types.py(2 hunks)src/draive/evaluators/utils.py(2 hunks)src/draive/helpers/volatile_vector_index.py(2 hunks)src/draive/multimodal/content.py(3 hunks)src/draive/multimodal/templates/repository.py(1 hunks)src/draive/multimodal/templates/types.py(1 hunks)src/draive/multimodal/templates/variables.py(3 hunks)src/draive/postgres/vector_index.py(1 hunks)src/draive/qdrant/index.py(1 hunks)src/draive/resources/template.py(1 hunks)src/draive/utils/__init__.py(0 hunks)src/draive/utils/vector_index.py(0 hunks)tests/test_evaluators_utils.py(1 hunks)tests/test_multimodal_template_variables.py(1 hunks)tests/test_tags_extraction.py(1 hunks)tests/test_tags_replacement.py(1 hunks)
💤 Files with no reviewable changes (2)
- src/draive/utils/init.py
- src/draive/utils/vector_index.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python 3.13+ for development
Use Ruff formatter for code formatting (viamake format), no other formatter
Run Ruff, Bandit, and Pyright (strict) linters viamake lint
Import symbols fromhaiwaydirectly usingfrom haiway import State, ctx
Use context scoping withctx.scope(...)to bind scopedDisposables, activeStateinstances and avoid global state
All logs must go throughctx.log_*methods; do not use
Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or looseAnyunless required by third-party boundaries
Prefer explicit attribute access with static types; avoid dynamicgetattrexcept at narrow boundaries
Prefer abstract immutable protocols (Mapping,Sequence,Iterable) overdict/list/setin public types
Usefinalwhere applicable; avoid inheritance, prefer type composition
Use precise unions (|) and narrow withmatch/isinstance, avoidcastunless provably safe and localized
Usehaiway.Statefor immutable data/config and service facades, construct with classmethods likeof(...)when ergonomic
Avoid in-place mutation; useState.updated(...)/functional builders to create new instances
Access active state throughhaiway.ctxinside async scopes (ctx.scope(...))
Public state methods that dispatch on the active instance should use@statemethod
Usectx.log_debug/info/warn/errorfor logs; do not use
Log around generation calls, tool dispatch, provider requests/responses without leaking secrets
Add appropriate metrics tracking usingctx.recordwhere applicable
Prefer structured and concise log messages; avoid excessive verbosity in hot paths
All I/O is async, keep boundaries async and usectx.spawnfor detached tasks
Ensure structured concurrency concepts and valid coroutine usage
Rely on haiway and asyncio packages with coroutines, avoid custom threading
Don't raise bareException, preserve contextual information in excep...
Files:
src/draive/multimodal/templates/repository.pysrc/draive/aws/api.pysrc/draive/aws/types.pytests/test_tags_extraction.pysrc/draive/embedding/types.pytests/test_tags_replacement.pytests/test_multimodal_template_variables.pysrc/draive/aws/client.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.pysrc/draive/resources/template.pysrc/draive/__init__.pysrc/draive/evaluators/utils.pysrc/draive/multimodal/templates/types.pysrc/draive/multimodal/content.pysrc/draive/multimodal/templates/variables.pysrc/draive/embedding/__init__.pytests/test_evaluators_utils.pysrc/draive/postgres/vector_index.py
src/draive/**/!(test_)*.py
📄 CodeRabbit inference engine (AGENTS.md)
Translate provider/SDK errors into appropriate typed exceptions
Files:
src/draive/multimodal/templates/repository.pysrc/draive/aws/api.pysrc/draive/aws/types.pysrc/draive/embedding/types.pysrc/draive/aws/client.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.pysrc/draive/resources/template.pysrc/draive/__init__.pysrc/draive/evaluators/utils.pysrc/draive/multimodal/templates/types.pysrc/draive/multimodal/content.pysrc/draive/multimodal/templates/variables.pysrc/draive/embedding/__init__.pysrc/draive/postgres/vector_index.py
src/draive/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Files:
src/draive/multimodal/templates/repository.pysrc/draive/aws/api.pysrc/draive/aws/types.pysrc/draive/embedding/types.pysrc/draive/aws/client.pysrc/draive/helpers/volatile_vector_index.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.pysrc/draive/resources/template.pysrc/draive/__init__.pysrc/draive/evaluators/utils.pysrc/draive/multimodal/templates/types.pysrc/draive/multimodal/content.pysrc/draive/multimodal/templates/variables.pysrc/draive/embedding/__init__.pysrc/draive/postgres/vector_index.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: No network in unit tests, mock providers/HTTP
Keep tests fast and specific to the code you change, start with unit tests around new types/functions and adapters
Use fixtures fromtests/or add focused ones; avoid heavy integration scaffolding
Usepytest-asynciofor coroutine tests with@pytest.mark.asynciodecorator
Prefer scoping withctx.scope(...)and bind requiredStateinstances explicitly in async tests
Avoid real I/O and network in tests; stub provider calls and HTTP
Files:
tests/test_tags_extraction.pytests/test_tags_replacement.pytests/test_multimodal_template_variables.pytests/test_evaluators_utils.py
src/draive/**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Update
__init__.pyexports and docs when API surface changes
Files:
src/draive/__init__.pysrc/draive/embedding/__init__.py
🧠 Learnings (12)
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use context scoping with `ctx.scope(...)` to bind scoped `Disposables`, active `State` instances and avoid global state
Applied to files:
docs/guides/Qdrant.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use `ResourceContent`/`ResourceReference` for media and data blobs
Applied to files:
docs/getting-started/printing-data.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Build content with `MultimodalContent.of(...)` and prefer composing content blocks explicitly
Applied to files:
docs/getting-started/printing-data.mddocs/getting-started/multimodal-data.mdsrc/draive/multimodal/content.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to src/draive/**/__init__.py : Update `__init__.py` exports and docs when API surface changes
Applied to files:
src/draive/aws/api.pysrc/draive/aws/types.pysrc/draive/embedding/types.pyAGENTS.mddocs/getting-started/installation.mdsrc/draive/aws/client.pysrc/draive/qdrant/index.pysrc/draive/embedding/state.pysrc/draive/__init__.pysrc/draive/embedding/__init__.pysrc/draive/postgres/vector_index.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to src/draive/**/*.py : Add NumPy-style docstrings to public symbols including Parameters/Returns/Raises sections and rationale when not obvious
Applied to files:
src/draive/embedding/types.pysrc/draive/embedding/state.py
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Import symbols from `haiway` directly using `from haiway import State, ctx`
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Access active state through `haiway.ctx` inside async scopes (`ctx.scope(...)`)
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Use `haiway.State` for immutable data/config and service facades, construct with classmethods like `of(...)` when ergonomic
Applied to files:
AGENTS.md
📚 Learning: 2025-06-16T10:28:07.434Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 338
File: src/draive/lmm/__init__.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
Applied to files:
AGENTS.mddocs/getting-started/installation.mdpyproject.toml
📚 Learning: 2025-05-28T17:41:57.460Z
Learnt from: KaQuMiQ
Repo: miquido/draive PR: 327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., `def func[T: Type]()`) is valid and should be used instead of the older TypeVar approach.
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Rely on haiway and asyncio packages with coroutines, avoid custom threading
Applied to files:
AGENTS.md
📚 Learning: 2025-11-27T14:40:28.862Z
Learnt from: CR
Repo: miquido/draive PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T14:40:28.862Z
Learning: Applies to **/*.py : Ensure latest and most strict typing syntax from Python 3.13+; strict typing only with no untyped public APIs or loose `Any` unless required by third-party boundaries
Applied to files:
AGENTS.mdsrc/draive/multimodal/templates/variables.py
🧬 Code graph analysis (13)
tests/test_tags_extraction.py (1)
src/draive/multimodal/content.py (5)
MultimodalContent(25-592)of(42-66)of(619-647)tags(427-446)tag(405-425)
src/draive/embedding/types.py (3)
src/draive/multimodal/text.py (1)
TextContent(11-82)src/draive/parameters/model.py (1)
DataModel(254-634)src/draive/resources/types.py (1)
ResourceContent(128-214)
tests/test_tags_replacement.py (1)
src/draive/multimodal/content.py (4)
MultimodalContent(25-592)of(42-66)of(619-647)replacing_tag(448-581)
tests/test_multimodal_template_variables.py (1)
src/draive/multimodal/templates/variables.py (1)
parse_template_variables(17-21)
src/draive/qdrant/index.py (2)
src/draive/embedding/types.py (1)
Embedded(25-28)src/draive/embedding/state.py (3)
ImageEmbedding(202-367)TextEmbedding(30-199)VectorIndex(371-587)
src/draive/resources/template.py (2)
src/draive/parameters/model.py (3)
get(542-545)get(548-552)get(554-559)src/draive/stages/types.py (4)
get(140-144)get(147-153)get(156-162)get(164-206)
src/draive/__init__.py (2)
src/draive/embedding/state.py (1)
VectorIndex(371-587)src/draive/utils/memory.py (1)
Memory(52-143)
src/draive/evaluators/utils.py (2)
src/draive/multimodal/content.py (7)
MultimodalContent(25-592)MultimodalTag(599-700)tag(405-425)of(42-66)of(619-647)to_str(288-297)to_str(653-666)src/draive/evaluation/score.py (1)
EvaluationScore(15-215)
src/draive/multimodal/templates/types.py (1)
tests/test_toolbox.py (2)
meta(109-110)meta(310-311)
src/draive/multimodal/templates/variables.py (1)
src/draive/multimodal/content.py (6)
MultimodalContent(25-592)parts(668-676)to_str(288-297)to_str(653-666)of(42-66)of(619-647)
src/draive/embedding/__init__.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
tests/test_evaluators_utils.py (2)
src/draive/evaluators/utils.py (1)
extract_evaluation_result(25-57)src/draive/multimodal/content.py (5)
MultimodalContent(25-592)MultimodalTag(599-700)tags(427-446)of(42-66)of(619-647)
src/draive/postgres/vector_index.py (1)
src/draive/embedding/state.py (1)
VectorIndex(371-587)
🪛 LanguageTool
docs/getting-started/multimodal-data.md
[style] ~174-~174: Using many exclamation marks might seem excessive (in this case: 15 exclamation marks for a text that’s 2748 characters long)
Context: ...400px; margin: auto; display: block;"} !!! tip ``` There are ready-to-use methods...
(EN_EXCESSIVE_EXCLAMATION)
AGENTS.md
[grammar] ~34-~34: Ensure spelling is correct
Context: ...e/init.py`. ## Style & Patterns - Draive is built on top of Haiway (state, conte...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/getting-started/printing-data.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/getting-started/multimodal-data.md
80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (43)
src/draive/aws/client.py (2)
36-36: Formatting improvement: blank line separation is appropriate.Adding a blank line between the docstring and
__slots__improves code readability and follows PEP 257 conventions for separating documentation from implementation details.The rest of the class demonstrates excellent adherence to the coding guidelines:
- Strict typing throughout with no loose
Any- Proper use of
@finaldecorator- Well-structured NumPy-style docstring with clear Parameters documentation
- Correct import patterns (
from haiway import State)- Proper async context manager protocol implementation
1-79: Verify that AWS module exports are current in draive.aws.init.py.Per the learnings, when API surfaces change across the repository (e.g., the vector-index relocation mentioned in the PR), ensure that module-level exports are kept in sync. If this AWS client or any of its features have been modified as part of the broader cleanup, confirm the module's
__init__.pyreflects the current public API.src/draive/aws/types.py (1)
26-26: LGTM: Formatting improvement.The blank line improves readability by following standard Python conventions for separating docstrings from class body declarations.
docs/cookbooks/BasicMCP.md (1)
34-34: No issues identified. The model namegpt-5-miniis a valid OpenAI model available as of December 2025 and is appropriate for this documentation update.src/draive/resources/template.py (2)
66-68: Formatting change looks good.The multi-line reformatting of the parameter lookup expression improves readability while preserving semantics.
577-577: The pyright ignore comment is justified and necessary.Line 577 calls
__call__with**call_kwargs, unpacking a dict to parameters typed asMapping[str, ...](e.g.,parameters: Mapping[str, str] | None,tools: Mapping[str, Tool] | None). Pyright strict mode flags this pattern because dict values don't explicitly satisfy the abstractMappingtype in the static type system, despite being compatible at runtime. The type mismatch is inherent to the design:call_kwargsis dynamically built asdict[str, ...]and unpacked toMapping-typed parameters. Resolving this without the ignore would require verbose alternatives like casting individual values or refactoring the call interface, which would be less ergonomic. The suppress is reasonable and localized.src/draive/aws/api.py (1)
6-6: Export declaration as tuple looks goodUsing a tuple for
__all__is a nice tightening (immutable and conventional) with no behavioral change.docs/index.md (1)
76-81: Quickstart example model update is consistentUpdating the quickstart to
OpenAIResponsesConfig(model="gpt-5-mini")aligns with the rest of the docs and keeps the example current.docs/cookbooks/BasicDataExtraction.md (1)
53-57: Data extraction examples now consistently use gpt‑5‑miniBoth the basic and customized extraction scopes now use
OpenAIResponsesConfig(model="gpt-5-mini"), which keeps the cookbook consistent with the rest of the docs.Also applies to: 80-84
src/draive/multimodal/templates/repository.py (1)
60-68: Docstring wording refinement is accurateThe updated description (“Template storage and resolution backend.”) still correctly reflects
TemplatesRepository’s responsibilities and reads more directly.docs/cookbooks/BasicRAG.md (1)
114-122: RAG search scope now aligned with gpt‑5‑miniUsing
OpenAIResponsesConfig(model="gpt-5-mini")in the searching scope keeps the RAG example consistent with the rest of the docs’ model recommendations.docs/guides/Qdrant.md (1)
9-12: Qdrant guide reflow preserves semanticsThe rewrapped paragraphs and minor phrasing tweaks keep the Qdrant guide clearer without changing any of the technical guidance or code examples.
Also applies to: 45-47, 53-55, 70-73, 81-83, 86-90, 108-113, 124-125
src/draive/multimodal/templates/types.py (1)
88-88: LGTM! Field default improves consistency.Adding
META_EMPTYas the default value for themetafield aligns the direct class construction with the behavior of theof(...)classmethod, which already defaultsmetatoMeta.of(None). This change improves ergonomics while maintaining backward compatibility.docs/getting-started/installation.md (1)
43-52: LGTM! Improved organization of optional extras.The restructured format groups related provider extras together, making it easier for users to understand the available options. The new organization clearly distinguishes between hosted LLMs, AWS integrations, local deployments, and backend utilities.
tests/test_multimodal_template_variables.py (1)
15-15: LGTM! Tests correctly adapted to generator-based implementation.Wrapping
parse_template_variables(...)results withset(...)correctly adapts the tests to the new generator-based implementation. This maintains test correctness while allowing the underlying implementation to be more memory-efficient by yielding values lazily rather than constructing an eager set.Also applies to: 21-21, 27-27, 35-35, 43-43, 51-51, 59-59, 67-67, 75-75, 83-83
docs/getting-started/quickstart.md (1)
36-36: Model string updates in quickstart examples look consistent.Switching the OpenAI examples to
model="gpt-5-mini"in all three scopes is coherent with the rest of the docs and keeps the quickstart aligned with the recommended defaults.Also applies to: 63-63, 90-90
src/draive/__init__.py (1)
92-101: VectorIndex import move keeps the public API stable.Importing
VectorIndexfromdraive.embeddingwhile still exporting it via__all__maintains thefrom draive import VectorIndexsurface and aligns with the new embedding‑centric implementation.Based on learnings, this keeps the top-level exports in sync with the moved implementation.
Also applies to: 371-371
src/draive/qdrant/index.py (1)
6-12: Embedding/VectorIndex import consolidation matches the new embedding API.Using
from draive.embedding import Embedded, ImageEmbedding, TextEmbedding, VectorIndex, mmr_vector_similarity_searchis consistent with the VectorIndex move into the embedding subsystem and keeps theQdrantVectorIndexfactory wired against the new types.docs/guides/BasicModelGeneration.md (1)
63-71: Basic model generation examples correctly updated to gpt‑5 JSON‑mode models.The switch to
OpenAIResponsesConfig(model="gpt-5-mini")and the explicit list of supported JSON‑mode models (gpt-5,gpt-5-mini,gpt-5-nano, or custom) keep this guide consistent with the rest of the docs and the structured‑output story.Also applies to: 84-96, 110-111
docs/guides/ComprehensiveEvaluation.md (1)
13-14: EvaluatorResult/performance and evaluation_model updates are consistent.Referring to
EvaluatorResult.performancein the overview and using"gpt-5"in theevaluation_modelmetadata keeps this guide aligned with the current evaluation API and model defaults.Also applies to: 232-239
src/draive/embedding/__init__.py (1)
4-4: LGTM! VectorIndex correctly exposed in public API.The import and export of VectorIndex aligns with the broader API reorganization moving vector indexing from draive.utils to draive.embedding.
Also applies to: 12-12
src/draive/postgres/vector_index.py (1)
14-14: LGTM! Import path updated correctly.The VectorIndex import has been correctly updated from draive.utils to draive.embedding, aligning with the API reorganization.
tests/test_tags_extraction.py (1)
423-477: LGTM! Comprehensive test coverage for self-closing tags with attributes.The new tests thoroughly validate:
- Attribute extraction from self-closing tags (src, alt)
- Space handling before the closing slash
- Mixed content scenarios with both self-closing and regular tags
- Graceful handling of malformed attributes
docs/getting-started/first-steps.md (3)
15-15: LGTM! Model identifiers updated correctly.The OpenAI model configuration has been properly updated to use gpt-5-mini.
Also applies to: 64-64
111-115: LGTM! VectorIndex usage correctly replaced with VolatileVectorIndex.The import and instantiation have been properly updated:
- Import changed from
draive.embeddingtodraive.helpers- Instantiation changed from
VectorIndex()(which required callbacks) toVolatileVectorIndex()(which provides a complete implementation)This addresses the previous concern about VectorIndex requiring indexing, searching, and deleting callbacks.
121-124: LGTM! Guardrails API updated correctly.The moderation API has been properly updated from
ModerationStatetoGuardrailsModeration.of(provider="openai"), aligning with the broader API refactor.src/draive/helpers/volatile_vector_index.py (1)
26-89: LGTM! Performance improvement by performing embedding before lock acquisition.The refactored index function correctly:
- Validates and categorizes selected values into text or image types
- Rejects mixed types and unsupported MIME types
- Performs expensive embedding operations outside the lock (lines 59-70)
- Constructs embedded models before acquiring lock (lines 72-82)
- Only holds lock during storage mutation (lines 84-89)
This reduces lock contention and improves concurrency while maintaining correctness with strict type validation.
src/draive/multimodal/content.py (7)
20-21: LGTM! Public type alias for content parts.Exposing
MultimodalContentPartas a public type alias improves API discoverability and allows consumers to type-hint content parts without importing individual types.
717-743: Good addition ofslots=Truefor memory efficiency.Adding
slots=Trueto these internal frozen dataclasses reduces memory footprint and improves attribute access speed, which is beneficial for the high-volume tokenization path.
888-920: LGTM! Clean tokenization refactor with early exit optimization.The early return when no
<is present (lines 892-893) avoids unnecessary iteration. The cursor-based approach with_parse_tag_atcentralizes tag parsing logic effectively.
923-1019: Well-structured tag parsing with clear fast paths.The
_parse_tag_atfunction handles opening, closing, and self-closing tags uniformly. The fast paths at lines 959-964 for attribute-less tags avoid unnecessary dictionary allocations.
1037-1048:_meta_from_attrsnow avoids input mutation—addresses prior feedback.The function creates a new merged dict (
{first_key: first_val} | rest) instead of mutatingrest. This is clean and side-effect-free.
1066-1114: Escape sequence handling in_parse_attr_valueis thorough.The function correctly handles standard escapes (
\n,\t,\r,\",\',\\) and passes through unknown escapes literally. Rejecting newlines/carriage returns inside quoted values prevents malformed attributes.
1117-1131: Helper functions are concise and well-typed.
_current_partsand_find_matching_contextare appropriately factored out for reuse across tag collection and replacement flows.src/draive/multimodal/templates/variables.py (3)
1-8: Clean regex-based placeholder pattern.The
_PLACEHOLDER_PATTERNregex clearly defines the{%variable%}syntax. Usingfinditerfor iteration is efficient and avoids building intermediate structures.
24-57: LGTM! Clean text template resolution.The cursor-based approach with regex matching is straightforward. Raising
KeyErrorfor missing arguments provides clear error messages.
60-89: LGTM! Multimodal template resolution mirrors text version.The logic correctly handles empty results by returning
MultimodalContent.emptyand usesMultimodalContent.of(*parts)for normalization.src/draive/embedding/state.py (5)
1-27: LGTM! Clean imports and updated exports.The imports are well-organized with protocols (
VectorIndexing,VectorSearching,VectorDeleting) and content types (TextContent,ResourceContent) properly sourced. AddingVectorIndexto__all__correctly exposes it as part of the public API.
30-36: Good docstring additions to embedding classes.The
TextEmbeddingandImageEmbeddingclasses now have concise docstrings explaining their purpose and delegation pattern.
397-432: LGTM! Well-documented index method with observability.The
indexmethod has a clear docstring and appropriatectx.record_infocall for tracing. The overload pattern correctly supports both classmethod and instance usage.
484-532: LGTM! Search method with comprehensive parameters.The search method supports flexible querying with optional query vector, score threshold, requirements, and limit. The docstring clearly documents the parameters.
555-587: LGTM! Delete method completes the CRUD surface.The delete method follows the same pattern as index/search with proper observability and documentation.
llms.txt (1)
131-149: VectorIndex import is correct at top level; attribute syntax is valid.The code patterns in lines 131–149 are correct:
from draive import VectorIndex— VectorIndex is re-exported in the top-leveldraivepackage (seesrc/draive/__init__.py). This import is valid and preferred over the submodule path.
await index.index(..., attribute=Chunk._.content)— TheChunk._field access syntax is the correct AttributePath API. TheVolatileVectorIndeximplementation explicitly validates this at line 35, requiring eitherAttributePathorCallable, with the error message confirming theModel._.path.to.propertypattern as intended.No changes needed.
Likely an incorrect or invalid review comment.
| ``` | ||
| `MultimodalTag` is the only multimodal element that exposes metadata inline. Values stored in `meta` appear as XML-style tag attributes. | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The fenced code block is missing a language specifier, which triggers the markdownlint MD040 warning. Since this is explanatory text, add text as the language identifier.
Apply this diff:
-```
+```text
`MultimodalTag` is the only multimodal element that exposes metadata inline. Values stored in `meta` appear as XML-style tag attributes.
Based on coding guidelines and static analysis hints.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In docs/getting-started/printing-data.md around lines 134 to 136, the fenced
code block lacks a language identifier causing markdownlint MD040; update the
block by adding the language specifier "text" after the opening backticks so the
block reads text ... to suppress the warning and match guidelines.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| OpenAIResponsesConfig(model="gpt-5-mini"), # model configuration | ||
| disposables=(OpenAI(),), # lifecycle-managed OpenAI client | ||
| ): |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align example with response.content.to_str() guidance
The switch to OpenAIResponsesConfig(model="gpt-5-mini") and the note about response.content.to_str() both look good and consistent with the rest of the docs.
To reduce potential confusion, consider updating the example block to either:
- Print only the assistant text:
print(response.content.to_str())or
- Explicitly mention in the snippet comment that
print(response)shows the structured object, whileresponse.content.to_str()is for plain text rendering.
Also applies to: 77-78
🤖 Prompt for AI Agents
In docs/guides/BasicConversation.md around lines 54-56 (and also apply the same
change to lines 77-78), the example prints the full response object which may
confuse readers; update the example to print only the assistant text by
replacing the generic print(response) with printing response.content.to_str(),
or add an inline comment clarifying that print(response) shows the structured
response object while response.content.to_str() renders plain assistant text,
and ensure the code snippet and comment reflect this choice consistently.
| Description("Fallback sampling temperature for text generation"), | ||
| ] | ||
| from collections.abc import Sequence | ||
| from draive import DataModel, ModelGeneration |
There was a problem hiding this comment.
Inconsistent DataModel import paths.
Line 47 imports DataModel from draive, but line 134 imports from draive.parameters. Standardize on a single import path to avoid confusion.
Decide on the canonical import and update one of the examples. If draive.parameters is the correct module, update line 47; if draive is the public API, update line 134.
Also applies to: 134-134
🤖 Prompt for AI Agents
In llms.txt around line 47, the import for DataModel is inconsistent with the
later import at line 134 (draive.parameters); standardize by updating line 47 to
import DataModel from draive.parameters (i.e., replace "from draive import
DataModel, ModelGeneration" with "from draive.parameters import DataModel" and
keep ModelGeneration import from the appropriate public module if needed),
ensuring both examples use the same canonical path.
| FORMAT_INSTRUCTION: Final[str] = f"""\ | ||
| <FORMAT> | ||
| Respond using exactly the following XML structure: | ||
| <REASONING>Concise, step-by-step justification that supports the rating.</REASONING> | ||
| <RATING>Selected rating value with exactly one of the allowed rating names (no quotes or extra text)</RATING> | ||
| Respond using exactly the following XML structure and include no other text before or after it. | ||
| Include both the rating and the comment tags exactly once. | ||
|
|
||
| <{COMMENT_TAG_NAME}>Concise, step-by-step justification that supports the rating. Do not leave empty.</{COMMENT_TAG_NAME}> | ||
| <{RATING_TAG_NAME}>Single, lowercase rating name chosen from the available ratings list (no quotes or extra text).</{RATING_TAG_NAME}> | ||
| </FORMAT> | ||
| """ # noqa: E501 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
FORMAT_INSTRUCTION now aligns better with parser behavior.
The instruction text now explicitly requires both tags exactly once, which matches the validation logic. However, the comment tag instruction says "Do not leave empty" while the parser still allows an empty comment (stored as None in meta). Consider relaxing the instruction text or validating non-empty comment content.
🤖 Prompt for AI Agents
In src/draive/evaluators/utils.py around lines 14–22, the FORMAT_INSTRUCTION
text contradicts the parser by saying the comment tag must "Do not leave empty"
while the parser permits an empty comment (stored as None). Fix this by editing
the instruction string to remove or relax that prohibitive phrase so it matches
parser behavior (e.g., state that the comment should contain a concise
justification or may be left empty if none), and update any related
validation/tests to reflect the relaxed requirement.
No description provided.