Skip to content

Feature/knowledge document read #888

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.

Sign up for GitHub

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

Merged
qdaxb merged 3 commits into from
Mar 31, 2026
Merged

Feature/knowledge document read #888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
020dd4d
docs(backend): add knowledge document read spec and plan
kissghosts Mar 31, 2026
1eb5912
feat(backend): add orchestrated knowledge document reads
kissghosts Mar 31, 2026
63ae8f4
fix(backend): tighten knowledge document read handling
kissghosts Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ coverage/
.claude/settings.local.json
.history
.wegentrc
.worktrees/

# PyInstaller
*.spec
Expand Down
13 changes: 7 additions & 6 deletions backend/app/api/endpoints/internal/rag.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,11 @@ async def read_document(
Document content with pagination info
"""
try:
from app.services.rag.document_read_service import document_read_service
from app.services.rag.document_read_service import (
DOCUMENT_READ_ERROR_ACCESS_DENIED,
DOCUMENT_READ_ERROR_NOT_FOUND,
document_read_service,
)

results = document_read_service.read_documents(
db=db,
Expand All @@ -748,12 +752,9 @@ async def read_document(
)
result = results[0] if results else None

if not result or result.get("error") == "Document not found":
if not result or result.get("error_code") == DOCUMENT_READ_ERROR_NOT_FOUND:
raise HTTPException(status_code=404, detail="Document not found")
if (
result.get("error")
== "Access denied: document not in allowed knowledge bases"
):
if result.get("error_code") == DOCUMENT_READ_ERROR_ACCESS_DENIED:
raise HTTPException(
status_code=403,
detail="Access denied: document not in allowed knowledge bases",
Expand Down
235 changes: 102 additions & 133 deletions backend/app/api/endpoints/knowledge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@
KnowledgeService,
knowledge_base_qa_service,
)
from app.services.knowledge.orchestrator import knowledge_orchestrator
from app.services.knowledge.orchestrator import (
MAX_DOCUMENT_READ_LIMIT,
knowledge_orchestrator,
)
from shared.telemetry.decorators import (
add_span_event,
capture_trace_context,
Expand All @@ -61,6 +64,70 @@
router = APIRouter()


def _serialize_standalone_document_detail(
detail: DocumentDetailResponse | dict,
*,
include_content: bool,
include_summary: bool,
) -> dict:
"""Preserve the standalone endpoint's pre-refactor response shape."""
payload = detail.model_dump() if hasattr(detail, "model_dump") else dict(detail)
response = {
"document_id": payload["document_id"],
}
if include_content:
response["content"] = payload.get("content")
response["content_length"] = payload.get("content_length")
response["truncated"] = payload.get("truncated")
if include_summary:
response["summary"] = payload.get("summary")
return response


def _raise_document_detail_http_error(error: ValueError) -> None:
"""Map orchestrator detail errors to stable HTTP responses."""
error_msg = str(error)
if "not found" in error_msg.lower():
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=error_msg,
)
if "access denied" in error_msg.lower():
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=error_msg,
)
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=error_msg,
)


def _validate_knowledge_base_access_or_raise(
db: Session,
*,
knowledge_base_id: int,
user: User,
):
"""Restore the KB-scoped endpoint's KB-level error semantics."""
knowledge_base, has_access = KnowledgeService.get_knowledge_base(
db=db,
knowledge_base_id=knowledge_base_id,
user_id=user.id,
)
if not knowledge_base:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Knowledge base not found",
)
if not has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied to this knowledge base",
)
return knowledge_base


# ============== Knowledge Base Endpoints ==============


Expand Down Expand Up @@ -695,8 +762,8 @@ async def update_document_content(


@document_router.get("/{document_id}/detail")
@trace_sync("get_document_detail_standalone", "knowledge.api")
def get_document_detail_standalone(
@trace_async("get_document_detail_standalone", "knowledge.api")
async def get_document_detail_standalone(
document_id: int,
include_content: bool = Query(True, description="Include document content"),
include_summary: bool = Query(True, description="Include document summary"),
Expand All @@ -709,76 +776,23 @@ def get_document_detail_standalone(
This is a convenience endpoint for getting document content when the kb_id
is not readily available (e.g., in citation tooltips).
"""
from app.models.knowledge import KnowledgeDocument

# Get document
document = (
db.query(KnowledgeDocument).filter(KnowledgeDocument.id == document_id).first()
)

if not document:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Document not found",
)

# Check access permission via knowledge base
kb, has_access = KnowledgeService.get_knowledge_base(
db=db,
knowledge_base_id=document.kind_id,
user_id=current_user.id,
)
if not kb:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Knowledge base not found",
try:
detail = await knowledge_orchestrator.get_document_detail(
db=db,
user=current_user,
document_id=document_id,
include_content=include_content,
include_summary=include_summary,
offset=0,
limit=MAX_DOCUMENT_READ_LIMIT,
)
if not has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied to this document",
return _serialize_standalone_document_detail(
detail,
include_content=include_content,
include_summary=include_summary,
)

# Get content if requested
content = None
content_length = None
truncated = False

if include_content and document.attachment_id:
try:
from app.services.context import context_service

attachment = context_service.get_context_by_id(
db=db,
context_id=document.attachment_id,
)
if attachment and attachment.extracted_text:
full_content = attachment.extracted_text
content_length = len(full_content)
# Truncate if too large
max_length = 100000
if content_length > max_length:
content = full_content[:max_length]
truncated = True
else:
content = full_content
except Exception as e:
logger.warning(f"Failed to get document content: {e}")

# Build response
response = {
"document_id": document_id,
}

if include_content:
response["content"] = content
response["content_length"] = content_length
response["truncated"] = truncated

if include_summary:
response["summary"] = document.summary

return response
except ValueError as error:
_raise_document_detail_http_error(error)
Comment on lines +779 to +795

@coderabbitai coderabbitai Bot Mar 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve 404/403 checks when both include flags are false.

KnowledgeOrchestrator.get_document_detail() only performs document access validation when include_content or include_summary is true. With the current delegation, GET /{document_id}/detail?include_content=false&include_summary=false now returns {"document_id": ...} with HTTP 200 even for missing or unauthorized documents. Please either reject that parameter combination or force a preflight access check before returning the bare ID.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/app/api/endpoints/knowledge.py` around lines 776 - 790, The current
handler returns a 200 for requests with include_content=false and
include_summary=false because access checks are only done inside
KnowledgeOrchestrator.get_document_detail when one of those flags is true; fix
by forcing a preflight access check: if not include_content and not
include_summary, call knowledge_orchestrator.get_document_detail(db=db,
user=current_user, document_id=document_id, include_content=True,
include_summary=False) inside the same try/except (so ValueError still maps via
_raise_document_detail_http_error), then discard content/summary and return the
minimal serialized response via _serialize_standalone_document_detail; this
preserves authorization/404 behavior while keeping the response body limited
when both flags are false.



@document_router.post("/batch/delete", response_model=BatchOperationResult)
Expand Down Expand Up @@ -1104,23 +1118,13 @@ async def get_document_detail(
- summary: Document summary object (if include_summary=true)
"""
from app.models.knowledge import KnowledgeDocument
from app.models.subtask_context import SubtaskContext
from app.services.knowledge import get_summary_service

# Validate KB access permission first
kb, has_access = KnowledgeService.get_knowledge_base(db, kb_id, current_user.id)
if not kb:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Knowledge base not found",
)
if not has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied to this knowledge base",
)
_validate_knowledge_base_access_or_raise(
db,
knowledge_base_id=kb_id,
user=current_user,
)

# Validate document belongs to the specified knowledge base
document = (
db.query(KnowledgeDocument)
.filter(
Expand All @@ -1135,53 +1139,18 @@ async def get_document_detail(
detail="Document not found in the specified knowledge base",
)

# Initialize response data
content = None
content_length = None
truncated = None
summary = None

# Get document content if requested
if include_content:
content = ""
truncated = False
max_length = 100000 # 100k characters limit for frontend display

if document.attachment_id:
context = (
db.query(SubtaskContext)
.filter(SubtaskContext.id == document.attachment_id)
.first()
)

if context and context.extracted_text:
content = context.extracted_text
# Truncate if too long
if len(content) > max_length:
content = content[:max_length]
truncated = True

content_length = len(content)

# Get document summary if requested
if include_summary:
summary_service = get_summary_service(db)
summary_obj = await summary_service.get_document_summary(doc_id)
# Convert DocumentSummary object to dict for response
if summary_obj:
summary = (
summary_obj.model_dump()
if hasattr(summary_obj, "model_dump")
else summary_obj
)

return DocumentDetailResponse(
document_id=doc_id,
content=content,
content_length=content_length,
truncated=truncated,
summary=summary,
)
try:
return await knowledge_orchestrator.get_document_detail(
db=db,
user=current_user,
document_id=doc_id,
include_content=include_content,
include_summary=include_summary,
offset=0,
limit=MAX_DOCUMENT_READ_LIMIT,
)
except ValueError as error:
_raise_document_detail_http_error(error)


@summary_router.get("/{kb_id}/documents/{doc_id}/summary")
Expand Down
61 changes: 60 additions & 1 deletion backend/app/mcp_server/tools/knowledge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@
from app.mcp_server.auth import TaskTokenInfo
from app.mcp_server.tools.decorator import build_mcp_tools_dict, mcp_tool
from app.models.user import User
from app.services.knowledge.orchestrator import knowledge_orchestrator
from app.services.knowledge.orchestrator import (
MAX_DOCUMENT_READ_LIMIT,
knowledge_orchestrator,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -302,6 +305,62 @@ def create_document(
db.close()


@mcp_tool(
name="read_document_content",
description="Read document content with offset/limit pagination.",
server="knowledge",
param_descriptions={
"document_id": "Document ID to read",
"offset": "Character offset to start reading from",
"limit": "Maximum number of characters to return",
},
)
def read_document_content(
token_info: TaskTokenInfo,
document_id: int,
offset: int = 0,
limit: int = MAX_DOCUMENT_READ_LIMIT,
) -> Dict[str, Any]:
"""
Read raw document content with offset/limit pagination.

Args:
token_info: Task token information containing user context
document_id: Document ID
offset: Character offset to start reading from
limit: Maximum number of characters to return (defaults to backend limit)

Returns:
Dict with document content slice and pagination metadata
"""
db = SessionLocal()
try:
user = _get_user_from_token(db, token_info)
if not user:
return {"error": "User not found"}

result = knowledge_orchestrator.read_document_content(
db=db,
user=user,
document_id=document_id,
offset=offset,
limit=limit,
)

return result.model_dump()

except ValueError as e:
logger.warning(f"[MCP] read_document_content validation error: {e}")
return {"error": str(e)}

except Exception as e:
logger.error(f"[MCP] read_document_content error: {e}", exc_info=True)
return {"error": str(e)}

finally:
db.close()


@mcp_tool(
name="update_document_content",
description="Update document content. Only supports TEXT type documents.",
Expand Down
Loading
Loading