Repository navigation
LCORE-4582: Knowledge Capability - auto mode #2926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,14 +16,18 @@ | |
|
|
||
| from collections.abc import Sequence | ||
| from dataclasses import dataclass | ||
| from typing import TYPE_CHECKING, Optional | ||
| from typing import Optional | ||
|
|
||
| from pydantic_ai.capabilities import AbstractCapability | ||
| from pydantic_ai.tools import AgentDepsT | ||
| from pydantic_ai.messages import TextContent, UserContent | ||
| from pydantic_ai.models import ModelRequestContext | ||
| from pydantic_ai.tools import AgentDepsT, RunContext | ||
| from sentence_transformers import CrossEncoder | ||
|
|
||
| if TYPE_CHECKING: | ||
| from knowledge.sources.base import KnowledgeSource | ||
| from knowledge.search import search_sources | ||
| from knowledge.sources.base import KnowledgeSource | ||
| from knowledge.types import KnowledgeMatch | ||
| from knowledge.utils import append_latest_message, extract_latest_message_text | ||
|
|
||
|
|
||
| @dataclass | ||
|
|
@@ -38,3 +42,53 @@ class Knowledge(AbstractCapability[AgentDepsT]): | |
| `'cross-encoder/ms-marco-MiniLM-L-6-v2'`) used to rerank matches by | ||
| relevance to the query before they're returned or injected. Loaded | ||
| lazily on first use; requires the `sentence-transformers` package.""" | ||
|
|
||
| async def before_model_request( | ||
| self, | ||
| ctx: RunContext[AgentDepsT], | ||
| request_context: ModelRequestContext, | ||
| ) -> ModelRequestContext: | ||
| """Search every auto-mode source concurrently, rank the merged matches, and inject them.""" | ||
|
|
||
| def to_message(matches: list[KnowledgeMatch]) -> Optional[UserContent]: | ||
| if not matches: | ||
| return None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] code-organization The to_message helper is defined as a nested closure inside before_model_request but captures nothing from the enclosing scope. The established codebase pattern factors such helpers into module-level private functions or static/private methods. |
||
|
|
||
| match_blocks = [ | ||
| f'<match id="{match.id}" source="{match.source}" ' | ||
| f'knowledge_source="{match.metadata.get("knowledge_source", "")}">\n' | ||
| f"{match.content.strip()}\n" | ||
| f"</match>" | ||
| for match in matches | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case When match.source is None, the f-string renders source="None" into the XML. This could confuse an LLM that sees None as a meaningful source name rather than understanding the source is absent. Suggested fix: Consider guarding the interpolation with match.source or empty string and updating the test expectation to match. |
||
| ] | ||
|
Comment on lines
+57
to
+63
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win Escape match content and attributes inside the
Proposed fix+from html import escape
...
match_blocks = [
- f'<match id="{match.id}" source="{match.source}" '
- f'knowledge_source="{match.metadata.get("knowledge_source", "")}">\n'
- f"{match.content.strip()}\n"
+ f'<match id="{escape(match.id)}" source="{escape(match.source or "")}" '
+ f'knowledge_source="{escape(str(match.metadata.get("knowledge_source", "")))}">\n'
+ f"{escape(match.content.strip(), quote=False)}\n"
f"</match>"
for match in matches
]🤖 Prompt for AI Agents |
||
|
|
||
| body = "\n\n".join(match_blocks) | ||
|
|
||
| return TextContent( | ||
| content=f"<knowledge>\n{body}\n</knowledge>", | ||
| metadata={ | ||
| "kind": "knowledge", | ||
| "knowledge_scores": {match.id: match.score for match in matches}, | ||
| "knowledge_metadata": { | ||
| match.id: match.metadata for match in matches | ||
| }, | ||
| "knowledge_sources": { | ||
| match.id: match.metadata.get("knowledge_source") | ||
| for match in matches | ||
| }, | ||
| }, | ||
| ) | ||
|
|
||
| auto_sources = [source for source in self.sources if "auto" in source.mode] | ||
| if not auto_sources: | ||
| return request_context | ||
|
|
||
| prompt = extract_latest_message_text(request_context.messages) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] edge-case search_sources (which uses asyncio.gather without return_exceptions=True) can propagate an unhandled exception from any knowledge source's .search() method. Because before_model_request has no try/except, a single backend failure will abort the model request hook rather than gracefully degrading by skipping knowledge injection. Suggested fix: Wrap the search_sources call in a try/except Exception that logs the error and returns request_context unchanged, so the model request proceeds without knowledge context instead of failing entirely. |
||
| if not prompt: | ||
| return request_context | ||
|
|
||
| matches = await search_sources(auto_sources, prompt, self.reranker) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if message := to_message(matches): | ||
| append_latest_message(request_context.messages, message) | ||
|
Comment on lines
+86
to
+92
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
pip download pydantic-ai-slim==2.27.1 --no-deps -d /tmp/pai -q && cd /tmp/pai && unzip -qo *.whl
rg -n -C5 'ModelRequestContext\(|before_model_request' pydantic_ai | head -120Repository: lightspeed-core/lightspeed-stack Length of output: 1360 🏁 Script executed: set -u
printf '%s\n' '--- revisions ---'
git rev-parse --show-toplevel
git rev-parse e02a7266c5b2cf296cf045e56f7c64de18ad9026
git rev-parse 20de9ed8e56554d0403799d072733d29c500b13c
printf '%s\n' '--- capability at head ---'
nl -ba src/knowledge/capability.py | sed -n '1,240p'
printf '%s\n' '--- related helpers and references ---'
rg -n -F --glob '*.py' -e 'extract_latest_message_text' -e 'append_latest_message' -e 'find_latest_message' -e 'ModelRequestContext' -e 'before_model_request' src tests || test "$?" -eq 1
printf '%s\n' '--- dependency declarations ---'
rg -n -F --glob '!*lock*' --glob '*.{toml,txt,md,yml,yaml}' -e 'pydantic-ai' -e 'pydantic_ai' -e '2.27.1' . || test "$?" -eq 1
printf '%s\n' '--- exact diff ---'
git diff --find-renames 20de9ed8e56554d0403799d072733d29c500b13c e02a7266c5b2cf296cf045e56f7c64de18ad9026 -- src/knowledgeRepository: lightspeed-core/lightspeed-stack Length of output: 43207 🏁 Script executed: set -u
rm -rf /tmp/pai-2271 && mkdir -p /tmp/pai-2271
pip download pydantic-ai==2.27.1 --no-deps -d /tmp/pai-2271 -q
python3 - <<'PY'
from pathlib import Path
import zipfile
root = Path('/tmp/pai-2271')
wheels = list(root.glob('*.whl'))
print('wheels:', [p.name for p in wheels])
for wheel in wheels:
with zipfile.ZipFile(wheel) as z:
z.extractall(root / 'src')
PY
printf '%s\n' '--- pydantic-ai hook and context source ---'
rg -n -C8 -F --glob '*.py' -e 'ModelRequestContext' -e 'before_model_request' -e 'AbstractCapability' /tmp/pai-2271/src/pydantic_ai | sed -n '1,260p'
printf '%s\n' '--- knowledge utility source ---'
nl -ba src/knowledge/utils.py | sed -n '1,130p'
printf '%s\n' '--- focused knowledge tests ---'
nl -ba tests/unit/knowledge/test_capability.py | sed -n '380,710p'
printf '%s\n' '--- utility tests ---'
nl -ba tests/unit/knowledge/test_utils.py | sed -n '100,275p'Repository: lightspeed-core/lightspeed-stack Length of output: 26762 🌐 Web query:
💡 Result: Limit auto-mode retrieval to the current user prompt. When this hook receives a later Require the newest Suggested fixdiff --git a/src/knowledge/capability.py b/src/knowledge/capability.py
--- a/src/knowledge/capability.py
+++ b/src/knowledge/capability.py
@@ -20,11 +20,11 @@
from typing import Optional
from pydantic_ai.capabilities import AbstractCapability
-from pydantic_ai.messages import TextContent, UserContent
+from pydantic_ai.messages import ModelRequest, TextContent, UserContent
from pydantic_ai.models import ModelRequestContext
from pydantic_ai.tools import AgentDepsT, RunContext
from sentence_transformers import CrossEncoder
from knowledge.reranker import rerank
from knowledge.sources.base import KnowledgeSource
from knowledge.types import KnowledgeMatch
-from knowledge.utils import append_latest_message, extract_latest_message_text
+from knowledge.utils import (
+ append_latest_message,
+ extract_latest_message_text,
+ find_latest_message,
+)
@@ -106,7 +106,12 @@
if not auto_sources:
return request_context
- prompt = extract_latest_message_text(request_context.messages)
+ latest = find_latest_message(request_context.messages, ModelRequest)
+ if latest is None:
+ return request_context
+
+ prompt = extract_latest_message_text([latest])
if not prompt:
return request_contextdiff --git a/src/knowledge/utils.py b/src/knowledge/utils.py
--- a/src/knowledge/utils.py
+++ b/src/knowledge/utils.py
@@ -11,7 +11,7 @@
from collections.abc import Sequence
from typing import Optional, TypeVar
-from pydantic_ai.messages import ModelMessage, ModelRequest, UserContent, UserPromptPart
+from pydantic_ai.messages import ModelMessage, ModelRequest, TextContent, UserContent, UserPromptPart
@@ -49,12 +49,25 @@
return None
+def _is_knowledge_part(part: UserPromptPart) -> bool:
+ content = part.content
+ return isinstance(content, Sequence) and any(
+ isinstance(item, TextContent)
+ and item.metadata.get("kind") == "knowledge"
+ for item in content
+ )
+
+
def extract_latest_message_text(messages: Sequence[ModelMessage]) -> Optional[str]:
"""Return the text of the most recent user prompt, or `None` if absent.
Scans the message history for the last [`UserPromptPart`][pydantic_ai.messages.UserPromptPart].
"""
- part = find_latest_part(messages, UserPromptPart)
+ part = next(
+ (
+ part
+ for message in reversed(messages)
+ for part in reversed(message.parts)
+ if isinstance(part, UserPromptPart) and not _is_knowledge_part(part)
+ ),
+ None,
+ )
if not isinstance(part, UserPromptPart):
return None🤖 Prompt for AI Agents |
||
|
|
||
| return request_context | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] doc-style
The before_model_request docstring is a one-liner without Parameters/Returns sections. The established pattern in sibling capabilities documents these blocks for non-trivial methods.