Phase 2: domain models - #330
Conversation
The unit of indexable/retrievable text. Includes from_langchain() and to_langchain() boundary converters for migration compatibility. Imports are deferred in converter methods so core/ stays pure.
Document is the input to the indexing pipeline. ProcessedDocument is the result after parsing (text blocks + images). Includes DocumentType enum and from_langchain/to_langchain converters.
Domain user model with role enum (viewer/editor/owner), partition memberships carried inline, and OIDC session model for cookie-based auth. Derived from SQLAlchemy models in components/indexer/vectordb/.
Catalog models for tracking document lifecycle (QUEUED -> COMPLETED) and batch indexation jobs. Derived from TaskStateManager + File table.
Query input model, per-chunk scored results (RetrievalResult, ScoredChunk), and end-to-end retrieval output (RetrievalResponse).
Conversation + Message for chat history persistence. ContextualizedQuery for LLM query rewriting (HyDE, multi-query). Prompt + PromptType enum for template management.
Consumers can now import cleanly: from openrag.core.models import Chunk, Document, User, RetrievalQuery
📝 WalkthroughWalkthroughThe PR introduces a new domain models package ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
openrag/core/models/user.py (1)
11-14: Centralize the RBAC role hierarchy on the enum.The enum defines valid roles but not their required ordering, so authorization code may reimplement comparisons inconsistently. Add a small
levelhelper here to makeviewer < editor < ownercanonical.Suggested role-level helper
class PartitionRole(str, Enum): VIEWER = "viewer" EDITOR = "editor" OWNER = "owner" + + `@property` + def level(self) -> int: + return { + PartitionRole.VIEWER: 1, + PartitionRole.EDITOR: 2, + PartitionRole.OWNER: 3, + }[self]Based on learnings, Applies to
openrag/**/*.py: Use token-based authentication with role-based access control (RBAC) for multi-tenant partition access, with role hierarchy: viewer (1) < editor (2) < owner (3).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openrag/core/models/user.py` around lines 11 - 14, Add a canonical role ordering to the PartitionRole enum by adding a numeric level mapping and simple helpers: implement a level property (e.g., returning 1 for VIEWER, 2 for EDITOR, 3 for OWNER) and convenience comparison helpers such as __lt__ (and/or an at_least(self, other) method) so callers can reliably compare roles (use PartitionRole.level, PartitionRole.__lt__ or PartitionRole.at_least in authorization checks). Ensure the mapping and helper names are added to the PartitionRole class so existing RBAC code can use them instead of reimplementing ordering.openrag/core/models/document.py (1)
59-80: Extension map is missing common types covered byDocumentType.The enum defines
pptx/doc/emland more, butdetect_content_typehas no entries for formats liketextextension variants (.text), additional image types (.gif,.webp,.bmp,.tiff), audio (.flac,.ogg,.m4a), or video (.mov,.mkv,.webm). Depending on intended scope, consider broadening the mapping or documenting that unknown extensions fall back toTEXT(which may silently mis-classify binary files).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openrag/core/models/document.py` around lines 59 - 80, The detect_content_type function currently maps only a subset of extensions and falls back to DocumentType.TEXT; update the mapping in detect_content_type to include the missing common extensions (e.g., add "text" -> DocumentType.TEXT; images: "gif","webp","bmp","tiff" -> DocumentType.IMAGE; audio: "flac","ogg","m4a" -> DocumentType.AUDIO; video: "mov","mkv","webm" -> DocumentType.VIDEO) and change the default return from DocumentType.TEXT to a safer fallback such as DocumentType.BINARY (or another appropriate enum member) so binary/non-text files aren’t misclassified; modify the mapping dictionary in detect_content_type and adjust the final return accordingly.openrag/core/models/prompt.py (1)
22-31:prompt_typefield doesn't use thePromptTypeenum.
PromptTypeis defined in this module butPrompt.prompt_typeis typed as plainstr, so invalid values won't be validated and consumers lose IDE/enum support. Consider typing it asPromptType(with an appropriate default) for consistency with the rest of the domain models in this PR that pair enums with their fields.♻️ Proposed change
- prompt_type: str = "" + prompt_type: PromptType = PromptType.SYS_PROMPT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openrag/core/models/prompt.py` around lines 22 - 31, Prompt.prompt_type is declared as str but should use the PromptType enum for validation and IDE support; update the Prompt class to declare prompt_type: PromptType and give it a sensible enum default (e.g., PromptType.CUSTOM or PromptType.DEFAULT depending on which member exists in the PromptType enum) so code uses the enum type and default value instead of a plain string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openrag/core/models/chunk.py`:
- Around line 46-54: The conversion ChunkType(metadata.pop("chunk_type",
"text")) is brittle and can raise ValueError for legacy/non-standard chunk_type
values; change it to pop the raw value into a variable (e.g., raw_chunk_type),
attempt to construct ChunkType(raw_chunk_type) inside a try/except, and on
ValueError map known legacy strings to the new enum or else fall back to a safe
default like ChunkType.TEXT; ensure the original metadata key is removed (using
pop) and that you reference ChunkType and the class factory (cls(...)/from_doc)
so the conversion is resilient to old metadata formats.
In `@openrag/core/models/document.py`:
- Around line 82-91: The from_langchain classmethod currently ignores
filename-based content detection and always leaves content_type as the default
TEXT; update Document.from_langchain to extract the source filename
(metadata.pop("source", "")) and pass it to the existing detect_content_type
function (or Document.detect_content_type) to set the document's content_type
accordingly before returning the new Document instance; ensure you preserve
other metadata/pop semantics and fallback to DocumentType.TEXT when
detect_content_type returns None or on empty filename.
In `@openrag/core/models/query.py`:
- Around line 15-21: Add Pydantic validation to the RetrievalQuery model to
constrain caller-controlled retrieval limits: enforce top_k >= 1 and reasonable
upper bound (e.g., <= 1000), similarity_threshold between 0.0 and 1.0,
related_limit >= 0 and capped (e.g., <= 100), and max_ancestor_depth either None
or an int >= 0 and capped (e.g., <= 100). Implement these as Field(...)
constraints on the RetrievalQuery attributes (top_k, similarity_threshold,
related_limit, max_ancestor_depth) or add `@validator` methods on class
RetrievalQuery to coerce/raise on invalid values and provide clear error
messages referencing the field name.
In `@openrag/core/models/user.py`:
- Around line 46-62: The OIDCSession model is missing the Fernet-encrypted IdP
token fields used elsewhere; update the OIDCSession class to include
id_token_encrypted, access_token_encrypted, and refresh_token_encrypted (each
typed as str | None with default None using Field) so adapters and
session/refresh/logout flows preserve encrypted token material; ensure the field
names match what openrag/components/indexer/vectordb/utils.py (the token
encryption/round-trip logic) expects and add them to the OIDCSession
dataclass/Model (class OIDCSession) alongside the existing session_token_hash
and token-expiry fields.
---
Nitpick comments:
In `@openrag/core/models/document.py`:
- Around line 59-80: The detect_content_type function currently maps only a
subset of extensions and falls back to DocumentType.TEXT; update the mapping in
detect_content_type to include the missing common extensions (e.g., add "text"
-> DocumentType.TEXT; images: "gif","webp","bmp","tiff" -> DocumentType.IMAGE;
audio: "flac","ogg","m4a" -> DocumentType.AUDIO; video: "mov","mkv","webm" ->
DocumentType.VIDEO) and change the default return from DocumentType.TEXT to a
safer fallback such as DocumentType.BINARY (or another appropriate enum member)
so binary/non-text files aren’t misclassified; modify the mapping dictionary in
detect_content_type and adjust the final return accordingly.
In `@openrag/core/models/prompt.py`:
- Around line 22-31: Prompt.prompt_type is declared as str but should use the
PromptType enum for validation and IDE support; update the Prompt class to
declare prompt_type: PromptType and give it a sensible enum default (e.g.,
PromptType.CUSTOM or PromptType.DEFAULT depending on which member exists in the
PromptType enum) so code uses the enum type and default value instead of a plain
string.
In `@openrag/core/models/user.py`:
- Around line 11-14: Add a canonical role ordering to the PartitionRole enum by
adding a numeric level mapping and simple helpers: implement a level property
(e.g., returning 1 for VIEWER, 2 for EDITOR, 3 for OWNER) and convenience
comparison helpers such as __lt__ (and/or an at_least(self, other) method) so
callers can reliably compare roles (use PartitionRole.level,
PartitionRole.__lt__ or PartitionRole.at_least in authorization checks). Ensure
the mapping and helper names are added to the PartitionRole class so existing
RBAC code can use them instead of reimplementing ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c9af78f-f286-4819-8e55-62c23c62aee3
📒 Files selected for processing (11)
openrag/core/models/__init__.pyopenrag/core/models/catalog.pyopenrag/core/models/chunk.pyopenrag/core/models/contextualization.pyopenrag/core/models/conversation.pyopenrag/core/models/document.pyopenrag/core/models/prompt.pyopenrag/core/models/query.pyopenrag/core/models/retrieval_response.pyopenrag/core/models/retrieval_result.pyopenrag/core/models/user.py
| metadata = dict(doc.metadata) if doc.metadata else {} | ||
| return cls( | ||
| id=metadata.pop("_id", str(uuid.uuid4())), | ||
| document_id=metadata.pop("file_id", ""), | ||
| text=doc.page_content, | ||
| partition=metadata.pop("partition", "default"), | ||
| page_number=metadata.pop("page", None), | ||
| chunk_type=ChunkType(metadata.pop("chunk_type", "text")), | ||
| metadata=metadata, |
There was a problem hiding this comment.
Make chunk_type conversion tolerant of legacy metadata.
Line 53 can raise ValueError for existing LangChain documents whose chunk_type is not exactly one of the new enum values. The existing chunker path in openrag/components/indexer/chunker/chunker.py:245-260 can pass through non-standard entity metadata, so one chunk can abort the whole bridge conversion.
Suggested defensive conversion
metadata = dict(doc.metadata) if doc.metadata else {}
+ raw_chunk_type = metadata.pop("chunk_type", ChunkType.TEXT.value)
+ try:
+ chunk_type = raw_chunk_type if isinstance(raw_chunk_type, ChunkType) else ChunkType(raw_chunk_type)
+ except ValueError:
+ metadata["chunk_type"] = raw_chunk_type
+ chunk_type = ChunkType.TEXT
+
return cls(
id=metadata.pop("_id", str(uuid.uuid4())),
document_id=metadata.pop("file_id", ""),
text=doc.page_content,
partition=metadata.pop("partition", "default"),
page_number=metadata.pop("page", None),
- chunk_type=ChunkType(metadata.pop("chunk_type", "text")),
+ chunk_type=chunk_type,
metadata=metadata,
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openrag/core/models/chunk.py` around lines 46 - 54, The conversion
ChunkType(metadata.pop("chunk_type", "text")) is brittle and can raise
ValueError for legacy/non-standard chunk_type values; change it to pop the raw
value into a variable (e.g., raw_chunk_type), attempt to construct
ChunkType(raw_chunk_type) inside a try/except, and on ValueError map known
legacy strings to the new enum or else fall back to a safe default like
ChunkType.TEXT; ensure the original metadata key is removed (using pop) and that
you reference ChunkType and the class factory (cls(...)/from_doc) so the
conversion is resilient to old metadata formats.
| @classmethod | ||
| def from_langchain(cls, doc: Any) -> Document: | ||
| """Convert a LangChain Document to a domain Document.""" | ||
| metadata = dict(doc.metadata) if doc.metadata else {} | ||
| return cls( | ||
| filename=metadata.pop("source", ""), | ||
| text=doc.page_content, | ||
| partition=metadata.pop("partition", "default"), | ||
| metadata=metadata, | ||
| ) |
There was a problem hiding this comment.
from_langchain ignores filename-based content type detection.
content_type will always default to DocumentType.TEXT regardless of the source filename (e.g. .pdf, .docx), even though detect_content_type is available right in this class. If callers rely on content_type downstream, LangChain-sourced documents will be mis-tagged.
♻️ Proposed change
def from_langchain(cls, doc: Any) -> Document:
"""Convert a LangChain Document to a domain Document."""
metadata = dict(doc.metadata) if doc.metadata else {}
+ filename = metadata.pop("source", "")
return cls(
- filename=metadata.pop("source", ""),
+ filename=filename,
+ content_type=cls.detect_content_type(filename),
text=doc.page_content,
partition=metadata.pop("partition", "default"),
metadata=metadata,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def from_langchain(cls, doc: Any) -> Document: | |
| """Convert a LangChain Document to a domain Document.""" | |
| metadata = dict(doc.metadata) if doc.metadata else {} | |
| return cls( | |
| filename=metadata.pop("source", ""), | |
| text=doc.page_content, | |
| partition=metadata.pop("partition", "default"), | |
| metadata=metadata, | |
| ) | |
| `@classmethod` | |
| def from_langchain(cls, doc: Any) -> Document: | |
| """Convert a LangChain Document to a domain Document.""" | |
| metadata = dict(doc.metadata) if doc.metadata else {} | |
| filename = metadata.pop("source", "") | |
| return cls( | |
| filename=filename, | |
| content_type=cls.detect_content_type(filename), | |
| text=doc.page_content, | |
| partition=metadata.pop("partition", "default"), | |
| metadata=metadata, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openrag/core/models/document.py` around lines 82 - 91, The from_langchain
classmethod currently ignores filename-based content detection and always leaves
content_type as the default TEXT; update Document.from_langchain to extract the
source filename (metadata.pop("source", "")) and pass it to the existing
detect_content_type function (or Document.detect_content_type) to set the
document's content_type accordingly before returning the new Document instance;
ensure you preserve other metadata/pop semantics and fallback to
DocumentType.TEXT when detect_content_type returns None or on empty filename.
| top_k: int = 10 | ||
| similarity_threshold: float = 0.95 | ||
| filters: dict[str, Any] = Field(default_factory=dict) | ||
| include_related: bool = False | ||
| include_ancestors: bool = False | ||
| related_limit: int = 10 | ||
| max_ancestor_depth: int | None = None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "query.py" | grep -E "openrag.*models.*query"Repository: linagora/openrag
Length of output: 90
🏁 Script executed:
cat -n openrag/core/models/query.py | head -50Repository: linagora/openrag
Length of output: 808
🏁 Script executed:
# Check if there are any validators or constraints already in place
rg -A 5 -B 5 "class RetrievalQuery" openrag/Repository: linagora/openrag
Length of output: 591
🏁 Script executed:
# Check how top_k, related_limit, max_ancestor_depth are used downstream
rg "top_k|related_limit|max_ancestor_depth" openrag/ -A 3 -B 1 --type py | head -100Repository: linagora/openrag
Length of output: 6853
🏁 Script executed:
# Find all usages of RetrievalQuery class
rg "RetrievalQuery" openrag/ --type pyRepository: linagora/openrag
Length of output: 260
🏁 Script executed:
# Check the vector DB implementation to see if it handles negative/unbounded values
rg "def.*search|def.*get_file_ancestors" openrag/ -A 10 --type py | grep -A 10 "top_k\|related_limit\|max_ancestor_depth" | head -80Repository: linagora/openrag
Length of output: 5269
🏁 Script executed:
# Look specifically at the vectordb implementation
find openrag -name "*vectordb*" -o -name "*milvus*" | head -5Repository: linagora/openrag
Length of output: 214
🏁 Script executed:
# Check how RetrievalQuery is actually used - is it instantiated anywhere?
rg "RetrievalQuery(" openrag/ --type pyRepository: linagora/openrag
Length of output: 140
🏁 Script executed:
# Check the actual vectordb implementation and how it uses these parameters
cat -n openrag/components/indexer/vectordb/vectordb.py | head -200Repository: linagora/openrag
Length of output: 7472
🏁 Script executed:
# Look at the _expand_with_related_chunks implementation which uses related_limit and max_ancestor_depth
rg -B 5 -A 20 "_expand_with_related_chunks" openrag/ --type pyRepository: linagora/openrag
Length of output: 10198
🏁 Script executed:
# Get the full implementation of _expand_with_related_chunks
rg -A 80 "^async def _expand_with_related_chunks" openrag/components/retriever.pyRepository: linagora/openrag
Length of output: 3030
🏁 Script executed:
# Check the get_file_ancestors implementation in vectordb to see if it validates max_ancestor_depth
rg -B 3 -A 20 "async def get_file_ancestors\|def get_file_ancestors" openrag/ --type pyRepository: linagora/openrag
Length of output: 42
🏁 Script executed:
# Look at how top_k is used in the actual Milvus search implementation
rg -B 5 -A 25 "def async_search" openrag/components/indexer/vectordb/vectordb.py | head -150Repository: linagora/openrag
Length of output: 2103
🏁 Script executed:
# Get more of the async_search implementation to see how top_k and similarity_threshold are used
rg -A 80 "async def async_search" openrag/components/indexer/vectordb/vectordb.py | tail -100Repository: linagora/openrag
Length of output: 4045
🏁 Script executed:
# Look for where top_k is passed to Milvus search
rg "search_request\|AnnSearchRequest\|top_k" openrag/components/indexer/vectordb/vectordb.py | head -40Repository: linagora/openrag
Length of output: 42
🏁 Script executed:
# Check if there's validation in get_ancestor_chunks or get_related_chunks for the limits
rg -B 5 -A 20 "async def get_ancestor_chunks\|async def get_related_chunks" openrag/components/indexer/vectordb/vectordb.py | head -100Repository: linagora/openrag
Length of output: 42
🏁 Script executed:
# Search for any direct instantiation of RetrievalQuery
rg "RetrievalQuery\(" openrag/ --type py -iRepository: linagora/openrag
Length of output: 121
🏁 Script executed:
# Check if there are any imports of RetrievalQuery elsewhere
rg "from.*RetrievalQuery|import.*RetrievalQuery" openrag/ --type pyRepository: linagora/openrag
Length of output: 144
🏁 Script executed:
# Look at the actual Milvus parameters more carefully - check what happens with negative top_k or out-of-range threshold
cat -n openrag/components/indexer/vectordb/vectordb.py | sed -n '300,450p'Repository: linagora/openrag
Length of output: 7049
🏁 Script executed:
# Check if RetrievalQuery or other models in openrag/core/models/ use ConfigDict
rg "ConfigDict|model_config" openrag/core/models/ --type pyRepository: linagora/openrag
Length of output: 42
🏁 Script executed:
# Check the complete RetrievalQuery definition to ensure we have the full picture
cat openrag/core/models/query.pyRepository: linagora/openrag
Length of output: 647
🏁 Script executed:
# Verify if RetrievalQuery is actually used anywhere in production code paths
rg "RetrievalQuery" openrag/ --type py -B 2 -A 2Repository: linagora/openrag
Length of output: 1082
🏁 Script executed:
# Final check: search for any direct instantiation of RetrievalQuery in the codebase
rg "RetrievalQuery\s*\(" openrag/ --type pyRepository: linagora/openrag
Length of output: 121
🏁 Script executed:
# Check if negative/unbounded values would cause issues at the Milvus level
# Look at what happens when top_k is negative or zero
cat -n openrag/components/indexer/vectordb/vectordb.py | sed -n '400,500p'Repository: linagora/openrag
Length of output: 4653
🏁 Script executed:
# Confirm the complete picture by checking the Milvus search to see what happens with top_k
cat -n openrag/components/indexer/vectordb/vectordb.py | sed -n '540,600p'Repository: linagora/openrag
Length of output: 3059
Add field validators to constrain caller-controlled retrieval limits.
The domain model RetrievalQuery currently accepts unbounded or negative values for top_k, similarity_threshold, related_limit, and max_ancestor_depth. Although the API layer applies constraints via FastAPI, the exported model itself lacks self-validation. Invalid values could propagate to downstream Milvus calls if the model is instantiated outside the API layer. Apply these Pydantic constraints to ensure the domain model is self-validating:
Suggested Pydantic constraints
- top_k: int = 10
- similarity_threshold: float = 0.95
+ top_k: int = Field(default=10, gt=0, le=100)
+ similarity_threshold: float = Field(default=0.95, ge=0.0, le=1.0)
filters: dict[str, Any] = Field(default_factory=dict)
include_related: bool = False
include_ancestors: bool = False
- related_limit: int = 10
- max_ancestor_depth: int | None = None
+ related_limit: int = Field(default=10, ge=0, le=100)
+ max_ancestor_depth: int | None = Field(default=None, ge=0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| top_k: int = 10 | |
| similarity_threshold: float = 0.95 | |
| filters: dict[str, Any] = Field(default_factory=dict) | |
| include_related: bool = False | |
| include_ancestors: bool = False | |
| related_limit: int = 10 | |
| max_ancestor_depth: int | None = None | |
| top_k: int = Field(default=10, gt=0, le=100) | |
| similarity_threshold: float = Field(default=0.95, ge=0.0, le=1.0) | |
| filters: dict[str, Any] = Field(default_factory=dict) | |
| include_related: bool = False | |
| include_ancestors: bool = False | |
| related_limit: int = Field(default=10, ge=0, le=100) | |
| max_ancestor_depth: int | None = Field(default=None, ge=0) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openrag/core/models/query.py` around lines 15 - 21, Add Pydantic validation
to the RetrievalQuery model to constrain caller-controlled retrieval limits:
enforce top_k >= 1 and reasonable upper bound (e.g., <= 1000),
similarity_threshold between 0.0 and 1.0, related_limit >= 0 and capped (e.g.,
<= 100), and max_ancestor_depth either None or an int >= 0 and capped (e.g., <=
100). Implement these as Field(...) constraints on the RetrievalQuery attributes
(top_k, similarity_threshold, related_limit, max_ancestor_depth) or add
`@validator` methods on class RetrievalQuery to coerce/raise on invalid values and
provide clear error messages referencing the field name.
| class OIDCSession(BaseModel): | ||
| """An active OIDC session linking a user to IdP tokens. | ||
|
|
||
| Session token is opaque (stored hashed in DB). | ||
| IdP tokens (access, refresh, id) are Fernet-encrypted in DB. | ||
| """ | ||
|
|
||
| id: int = 0 | ||
| session_token_hash: str = "" | ||
| user_id: int = 0 | ||
| sid: str | None = None | ||
| sub: str = "" | ||
| access_token_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | ||
| session_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | ||
| created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | ||
| last_refresh_at: datetime | None = None | ||
| revoked_at: datetime | None = None |
There was a problem hiding this comment.
Preserve encrypted IdP token fields in the OIDC session model.
OIDCSession represents the persisted session, but it omits the encrypted id_token, access_token, and refresh_token fields created by openrag/components/indexer/vectordb/utils.py:935-970. If adapters round-trip through this domain model, refresh and logout/session-management flows can lose the token material they need.
Suggested fields
class OIDCSession(BaseModel):
@@
sid: str | None = None
sub: str = ""
+ id_token_encrypted: bytes | None = None
+ access_token_encrypted: bytes | None = None
+ refresh_token_encrypted: bytes | None = None
access_token_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC))As per coding guidelines, In OIDC mode, encrypt IdP tokens (id_token, access_token, refresh_token) using Fernet before storing in the oidc_sessions table.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class OIDCSession(BaseModel): | |
| """An active OIDC session linking a user to IdP tokens. | |
| Session token is opaque (stored hashed in DB). | |
| IdP tokens (access, refresh, id) are Fernet-encrypted in DB. | |
| """ | |
| id: int = 0 | |
| session_token_hash: str = "" | |
| user_id: int = 0 | |
| sid: str | None = None | |
| sub: str = "" | |
| access_token_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | |
| session_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | |
| created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | |
| last_refresh_at: datetime | None = None | |
| revoked_at: datetime | None = None | |
| class OIDCSession(BaseModel): | |
| """An active OIDC session linking a user to IdP tokens. | |
| Session token is opaque (stored hashed in DB). | |
| IdP tokens (access, refresh, id) are Fernet-encrypted in DB. | |
| """ | |
| id: int = 0 | |
| session_token_hash: str = "" | |
| user_id: int = 0 | |
| sid: str | None = None | |
| sub: str = "" | |
| id_token_encrypted: bytes | None = None | |
| access_token_encrypted: bytes | None = None | |
| refresh_token_encrypted: bytes | None = None | |
| access_token_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | |
| session_expires_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | |
| created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | |
| last_refresh_at: datetime | None = None | |
| revoked_at: datetime | None = None |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openrag/core/models/user.py` around lines 46 - 62, The OIDCSession model is
missing the Fernet-encrypted IdP token fields used elsewhere; update the
OIDCSession class to include id_token_encrypted, access_token_encrypted, and
refresh_token_encrypted (each typed as str | None with default None using Field)
so adapters and session/refresh/logout flows preserve encrypted token material;
ensure the field names match what openrag/components/indexer/vectordb/utils.py
(the token encryption/round-trip logic) expects and add them to the OIDCSession
dataclass/Model (class OIDCSession) alongside the existing session_token_hash
and token-expiry fields.
1 participant
Summary
All domain models in
core/models/. Pure Pydantic, no infrastructure imports.Chunk,ChunkType— unit of indexable/retrievable textDocument,ProcessedDocument,TextBlock,ImageBlock,DocumentTypeUser,PartitionRole,UserPartition,OIDCSessionDocumentRecord,IndexationJob,DocumentStatus,JobStatusRetrievalQuery,RetrievalResult,ScoredChunk,RetrievalResponseConversation,Message,ContextualizedQuery,Prompt,PromptType__init__.pyre-exports for clean importsLangChain converters
ChunkandDocumentincludefrom_langchain()/to_langchain()methods.These are temporary boundary converters needed during the migration — old code
returns LangChain
Documentobjects, new code uses domain models. The convertersbridge the gap at the boundary. They will be removed in Phase 12 along with the
LangChain dependency.
Verification
python scripts/check_layer_imports.py→ OKfrom openrag.core.models import Chunk, Document, User, RetrievalQuery→ OKSummary by CodeRabbit
Release Notes