From 64bd6a92218c23b1513cc11e201048546d7f8b9e Mon Sep 17 00:00:00 2001 From: mountain Date: Tue, 21 Jul 2026 16:58:49 +0800 Subject: [PATCH 1/2] feat(web): read a document's converted source text in the Documents pane (#197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Documents pane listed each ingested doc (name/type/hash) with no way to read the converted full text ingestion produced under wiki/sources/. Backend: new POST /api/v1/document/source resolves a doc hash to its source text — short docs read .md; long docs concatenate the per-page .json (page text joined by a thematic break). Hash is the identifier (unique, avoids doc_name/stem collisions); resolution prefers the registry's stored source_path then falls back to the wiki/sources/.{md,json} convention, with a path-traversal guard. Read-only (sources are do-not-edit). Frontend: document rows are now clickable and open a wide read-only slide-out reader (MarkdownView) — ESC/overlay/close to dismiss, independent scroll, content cached + memoized per hash, native find-in-page preserved (no virtualization). Delete stays inline (stopPropagation). Closed drawer is inert. Known limitation: images embedded in long-doc pages are not rendered inline yet. --- frontend/src/api/wiki.ts | 19 +++ frontend/src/locales/en/kb.json | 10 ++ frontend/src/locales/zh/kb.json | 10 ++ frontend/src/pages/KbDetail.tsx | 200 +++++++++++++++++++++++++++++++- openkb/api.py | 2 + openkb/api_documents_router.py | 30 +++++ openkb/api_models.py | 18 +++ openkb/documents.py | 88 ++++++++++++++ tests/test_api_documents.py | 130 +++++++++++++++++++++ 9 files changed, 504 insertions(+), 3 deletions(-) create mode 100644 openkb/api_documents_router.py create mode 100644 openkb/documents.py create mode 100644 tests/test_api_documents.py diff --git a/frontend/src/api/wiki.ts b/frontend/src/api/wiki.ts index 5b8aec73..71525df7 100644 --- a/frontend/src/api/wiki.ts +++ b/frontend/src/api/wiki.ts @@ -53,6 +53,25 @@ export function getKbInventory(kb: string): Promise { return apiFetch("/api/v1/list", { body: { kb } }) } +/** A document's ingested source text (`/api/v1/document/source`). This is the + * READ-ONLY conversion output stored under `wiki/sources/` — short docs are a + * single Markdown string; long docs are per-page text concatenated into one. + * `pages` is the page count for long docs and null for short. */ +export interface DocumentSource { + hash: string + name: string + doc_name: string + type: string + format: string + content: string + pages: number | null +} + +/** Fetch a document's converted full text by its `hash` (the /list identifier). */ +export function getDocumentSource(kb: string, hash: string): Promise { + return apiFetch("/api/v1/document/source", { body: { kb, hash } }) +} + /** Result of `/api/v1/page/delete`. `backlinks` are 'section/stem' refs whose * inbound [[links]] will be / were demoted to plain text. */ export interface PageDeleteResult { diff --git a/frontend/src/locales/en/kb.json b/frontend/src/locales/en/kb.json index 6c89a8b2..d3543146 100644 --- a/frontend/src/locales/en/kb.json +++ b/frontend/src/locales/en/kb.json @@ -34,6 +34,16 @@ "empty": "No documents yet · they compile into the KB automatically after upload", "pages_one": "{{count}} page", "pages_other": "{{count}} pages", + "reader": { + "open": "Read document", + "title": "Document", + "readonly": "Read-only", + "close": "Close", + "loading": "Loading document…", + "error": "Couldn't load document: {{error}}", + "retry": "Retry", + "emptyDoc": "This document has no extracted text." + }, "delete": { "action": "Delete document", "prompt": "Delete?", diff --git a/frontend/src/locales/zh/kb.json b/frontend/src/locales/zh/kb.json index 9668ce27..f93280cd 100644 --- a/frontend/src/locales/zh/kb.json +++ b/frontend/src/locales/zh/kb.json @@ -34,6 +34,16 @@ "empty": "暂无文档 · 上传后会自动编译进知识库", "pages_one": "{{count}} 页", "pages_other": "{{count}} 页", + "reader": { + "open": "查看文档", + "title": "文档", + "readonly": "只读", + "close": "关闭", + "loading": "正在加载文档…", + "error": "加载文档失败：{{error}}", + "retry": "重试", + "emptyDoc": "该文档没有可显示的正文。" + }, "delete": { "action": "删除文档", "prompt": "确认删除？", diff --git a/frontend/src/pages/KbDetail.tsx b/frontend/src/pages/KbDetail.tsx index 25ae7d36..c2a3f1b7 100644 --- a/frontend/src/pages/KbDetail.tsx +++ b/frontend/src/pages/KbDetail.tsx @@ -2,9 +2,9 @@ import { useCallback, useEffect, useMemo, useRef, useState, type RefObject } fro import { useNavigate, useParams } from 'react-router' import { useTranslation } from 'react-i18next' import { motion, useReducedMotion } from 'motion/react' -import { FileText, Link2, Loader2, Pencil, Upload, RefreshCw, Settings2, Trash2, Circle, CheckCircle2, CircleSlash2, XCircle } from 'lucide-react' +import { FileText, Link2, Loader2, Pencil, Upload, RefreshCw, Settings2, Trash2, Circle, CheckCircle2, CircleSlash2, XCircle, X, BookOpen } from 'lucide-react' import { toast } from 'sonner' -import { deletePage, editPage, getKbInventory, getPage, getPageLinks, type KbInventory, type WikiDocument } from '@/api/wiki' +import { deletePage, editPage, getDocumentSource, getKbInventory, getPage, getPageLinks, type DocumentSource, type KbInventory, type WikiDocument } from '@/api/wiki' import { streamUpload, removeDocument, type AddResult } from '@/api/maintenance' import { ApiError } from '@/api/client' import MarkdownView from '@/components/MarkdownView' @@ -444,6 +444,7 @@ export default function KbDetail() { /> ) : section === 'documents' ? ( void +}) { + const { t } = useTranslation(['kb', 'common']) + const [source, setSource] = useState(null) + const [error, setError] = useState(null) + const [loading, setLoading] = useState(false) + const [reloadSeq, setReloadSeq] = useState(0) + // Per-hash cache returns the SAME DocumentSource object on re-open, so the + // memoized body below is not re-parsed. + const cacheRef = useRef>(new Map()) + const scrollRef = useRef(null) + const open = doc !== null + const hash = doc?.hash ?? null + + useEffect(() => { + if (!hash) return + const cached = cacheRef.current.get(hash) + if (cached) { + setSource(cached) + setError(null) + setLoading(false) + return + } + let cancelled = false + setLoading(true) + setSource(null) + setError(null) + getDocumentSource(kb, hash) + .then((r) => { + if (cancelled) return + cacheRef.current.set(hash, r) + setSource(r) + }) + .catch((e) => { + if (!cancelled) setError(errMsg(e)) + }) + .finally(() => { + if (!cancelled) setLoading(false) + }) + return () => { + cancelled = true + } + }, [kb, hash, reloadSeq]) + + // ESC closes; listener attached only while open. + useEffect(() => { + if (!open) return + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose() + } + window.addEventListener('keydown', onKey) + return () => window.removeEventListener('keydown', onKey) + }, [open, onClose]) + + // Reset scroll to top when switching documents. + useEffect(() => { + scrollRef.current?.scrollTo(0, 0) + }, [hash]) + + // Parse Markdown once per fetched source (cache yields a stable object ref). + const body = useMemo( + () => (source ? : null), + [source], + ) + const hasContent = source != null && source.content.trim().length > 0 + + return ( + <> +
+ + + ) +} + function DocumentsPane({ + kb, documents, invError, uploading, @@ -858,6 +1029,7 @@ function DocumentsPane({ onRefresh, onDelete, }: { + kb: string documents: WikiDocument[] invError: string | null uploading: boolean @@ -874,6 +1046,8 @@ function DocumentsPane({ // `deletingName` is the row whose remove request is in flight. const [confirmName, setConfirmName] = useState(null) const [deletingName, setDeletingName] = useState(null) + // The document whose read-only source drawer is open (null = closed). + const [openDoc, setOpenDoc] = useState(null) const handleDelete = async (name: string) => { setDeletingName(name) try { @@ -884,6 +1058,7 @@ function DocumentsPane({ } } return ( + <>

@@ -981,8 +1156,20 @@ function DocumentsPane({ {documents.map((d, i) => (

setOpenDoc(d)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + setOpenDoc(d) + } + }} + title={t('kb:docs.reader.open')} className={cn( 'anim-fade-up rounded-2xl border border-[hsl(var(--glass-border))] glass-2 px-4 py-3 flex items-center gap-3', + 'cursor-pointer transition-colors hover:border-foreground/20 hover:bg-accent/40', + 'focus:outline-none focus-visible:ring-2 focus-visible:ring-accent-brand/50', `anim-d${Math.min(i + 1, 4)}`, )} > @@ -996,7 +1183,12 @@ function DocumentsPane({ {d.pages != null && <> · {t('kb:docs.pages', { count: d.pages })}}
-
+ {/* Trailing controls (hash chip + delete). stopPropagation so a + click here never opens the reader drawer. */} +
e.stopPropagation()} + > {d.hash && ( {d.hash.slice(0, 8)} @@ -1047,5 +1239,7 @@ function DocumentsPane({
+ setOpenDoc(null)} /> + ) } diff --git a/openkb/api.py b/openkb/api.py index 91fe86df..16b5db87 100644 --- a/openkb/api.py +++ b/openkb/api.py @@ -31,6 +31,7 @@ from openkb.agent.query import build_run_config_from_bundle, run_query from openkb.api_config import apply_kb_config_patch, read_kb_config from openkb.api_config_router import config_router +from openkb.api_documents_router import documents_router from openkb.api_graph import graph_router from openkb.api_helpers import ( _configure_cors, @@ -167,6 +168,7 @@ async def lifespan(app: FastAPI): app.include_router(config_router) app.include_router(kbs_router) app.include_router(pages_router) + app.include_router(documents_router) @app.get("/api/v1/kbs", response_model=KbListResponse) async def list_kbs_endpoint( diff --git a/openkb/api_documents_router.py b/openkb/api_documents_router.py new file mode 100644 index 00000000..ccb10f3a --- /dev/null +++ b/openkb/api_documents_router.py @@ -0,0 +1,30 @@ +"""Document REST endpoints: read a document's ingested source text. + +An APIRouter (sibling of api_pages_router.py) so api.py stays under the +per-file line gate. Read-only: source documents are ``Do not modify directly`` +artifacts, so there is no edit/delete counterpart here (document removal lives +on ``/api/v1/remove``). +""" + +from __future__ import annotations + +from fastapi import APIRouter, Depends, HTTPException +from starlette.concurrency import run_in_threadpool + +from openkb.api_helpers import _resolve_kb, require_bearer_token +from openkb.api_models import DocumentSourceRequest, DocumentSourceResponse +from openkb.documents import read_document_source + +documents_router = APIRouter() + + +@documents_router.post("/api/v1/document/source", response_model=DocumentSourceResponse) +async def document_source_endpoint( + request: DocumentSourceRequest, + _: None = Depends(require_bearer_token), +) -> DocumentSourceResponse: + kb_dir = _resolve_kb(request.kb) + result = await run_in_threadpool(read_document_source, kb_dir, request.hash) + if result is None: + raise HTTPException(status_code=404, detail="Document source not found.") + return DocumentSourceResponse(**result) diff --git a/openkb/api_models.py b/openkb/api_models.py index 0a1f5076..d83d1400 100644 --- a/openkb/api_models.py +++ b/openkb/api_models.py @@ -285,6 +285,24 @@ class PageResponse(BaseModel): content: str +class DocumentSourceRequest(BaseModel): + kb: str = Field(..., min_length=1) + # SHA-256 hash key from /list — the unique document identifier (avoids + # ambiguity when two documents share a doc_name/filename stem). + hash: str = Field(..., min_length=1) + + +class DocumentSourceResponse(BaseModel): + hash: str + name: str + doc_name: str + type: str + format: str + content: str + # Page count for long docs (per-page JSON); None for short (single .md). + pages: int | None = None + + class PageDeleteRequest(BaseModel): kb: str = Field(..., min_length=1) # A '
/' wiki-page ref, e.g. "concepts/attention". Only diff --git a/openkb/documents.py b/openkb/documents.py new file mode 100644 index 00000000..8df3265d --- /dev/null +++ b/openkb/documents.py @@ -0,0 +1,88 @@ +"""Read the ingested source text for a document (REST ``/document/source``). + +The Documents pane lists ingested docs by hash; this resolves a hash to the +converted full text under ``wiki/sources/``. Short docs are stored as +``.md``; long docs as ``.json`` — a per-page +``list[{page, content, images}]`` (see ``indexer._write_long_doc_artifacts``). +Read-only: sources are ``Do not modify directly`` artifacts. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +from openkb.state import HashRegistry + + +def _render_pages(pages: list[dict[str, Any]]) -> str: + """Join a long doc's per-page ``content`` into one Markdown string. + + Pages are separated by a thematic break (``---``) so the reader keeps + page boundaries; blank/missing page content is skipped. + """ + parts = [str(page.get("content", "")).strip() for page in pages] + return "\n\n---\n\n".join(part for part in parts if part) + + +def _resolve_source_file(kb_dir: Path, meta: dict, doc_name: str) -> Path | None: + """Resolve a document's source file, guarding against path traversal. + + Prefers the registry's stored ``source_path`` (a KB-relative posix path), + then falls back to the ``wiki/sources/.{md,json}`` convention + (older entries carry no ``source_path``). Returns ``None`` when nothing + resolves to an existing file inside ``wiki/sources/``. + """ + sources_dir = (kb_dir / "wiki" / "sources").resolve() + candidates: list[Path] = [] + stored = meta.get("source_path") + if stored: + candidates.append(kb_dir / stored) + candidates.append(sources_dir / f"{doc_name}.md") + candidates.append(sources_dir / f"{doc_name}.json") + for candidate in candidates: + try: + resolved = candidate.resolve() + except OSError: + continue + if resolved.is_file() and resolved.is_relative_to(sources_dir): + return resolved + return None + + +def read_document_source(kb_dir: Path, file_hash: str) -> dict[str, Any] | None: + """Return the ingested source text for the document identified by hash. + + Returns ``None`` when the hash is unknown OR its source file is missing, + so the caller maps both to a 404. ``format`` is always ``"markdown"``; + ``pages`` is the page count for long docs (per-page JSON) and ``None`` for + short docs. + """ + registry = HashRegistry(kb_dir / ".openkb" / "hashes.json") + meta = registry.get(file_hash) + if meta is None: + return None + + doc_name = meta.get("doc_name") or Path(meta.get("name", "")).stem + source = _resolve_source_file(kb_dir, meta, doc_name) + if source is None: + return None + + if source.suffix == ".json": + pages = json.loads(source.read_text(encoding="utf-8")) + content = _render_pages(pages) + page_count: int | None = len(pages) + else: + content = source.read_text(encoding="utf-8") + page_count = None + + return { + "hash": file_hash, + "name": meta.get("name", doc_name), + "doc_name": doc_name, + "type": meta.get("type", "unknown"), + "format": "markdown", + "content": content, + "pages": page_count, + } diff --git a/tests/test_api_documents.py b/tests/test_api_documents.py new file mode 100644 index 00000000..c38d8509 --- /dev/null +++ b/tests/test_api_documents.py @@ -0,0 +1,130 @@ +"""Tests for the document-source REST endpoint (read a doc's ingested text). + +Mirrors the helpers/patterns in tests/test_api.py. The endpoint resolves a +document hash to the converted full text under wiki/sources/ — short docs are +``.md``; long docs are ``.json`` (a per-page +``list[{page, content, images}]``). +""" + +from __future__ import annotations + +import json + +from fastapi.testclient import TestClient + +from openkb.api import create_app + + +def _client(monkeypatch, token: str | None = "secret") -> TestClient: + if token is None: + monkeypatch.delenv("OPENKB_API_TOKEN", raising=False) + else: + monkeypatch.setenv("OPENKB_API_TOKEN", token) + return TestClient(create_app()) + + +def _auth(token: str = "secret") -> dict[str, str]: + return {"Authorization": f"Bearer {token}"} + + +def _use_named_kb(monkeypatch, kb_dir, name: str = "test-kb") -> str: + def resolve(kb): + assert kb == name + return kb_dir + + monkeypatch.setattr("openkb.api_helpers.resolve_kb_alias", resolve) + return name + + +def _write_hashes(kb_dir, hashes: dict) -> None: + (kb_dir / ".openkb" / "hashes.json").write_text(json.dumps(hashes), encoding="utf-8") + + +def test_document_source_short_md(monkeypatch, kb_dir): + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes(kb_dir, {"h1": {"name": "notes.md", "doc_name": "notes", "type": "md"}}) + (kb_dir / "wiki" / "sources" / "notes.md").write_text("# Notes\n\nBody text.", encoding="utf-8") + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "h1"}, headers=_auth()) + + assert resp.status_code == 200 + payload = resp.json() + assert payload["content"] == "# Notes\n\nBody text." + assert payload["format"] == "markdown" + assert payload["pages"] is None + assert payload["name"] == "notes.md" + + +def test_document_source_long_json_concatenates_pages(monkeypatch, kb_dir): + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes(kb_dir, {"h2": {"name": "paper.pdf", "doc_name": "paper", "type": "long_pdf"}}) + pages = [ + {"page": 1, "content": "Page one text.", "images": []}, + {"page": 2, "content": "Page two text.", "images": []}, + ] + (kb_dir / "wiki" / "sources" / "paper.json").write_text(json.dumps(pages), encoding="utf-8") + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "h2"}, headers=_auth()) + + assert resp.status_code == 200 + payload = resp.json() + assert "Page one text." in payload["content"] + assert "Page two text." in payload["content"] + # page boundaries preserved via a thematic break + assert "---" in payload["content"] + assert payload["pages"] == 2 + + +def test_document_source_prefers_stored_source_path(monkeypatch, kb_dir): + """When metadata carries source_path, it wins over the doc_name convention.""" + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes( + kb_dir, + { + "h3": { + "name": "report.txt", + "doc_name": "report", + "type": "md", + "source_path": "wiki/sources/report.md", + } + }, + ) + (kb_dir / "wiki" / "sources" / "report.md").write_text("Converted.", encoding="utf-8") + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "h3"}, headers=_auth()) + + assert resp.status_code == 200 + assert resp.json()["content"] == "Converted." + + +def test_document_source_unknown_hash_404(monkeypatch, kb_dir): + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes(kb_dir, {}) + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "nope"}, headers=_auth()) + + assert resp.status_code == 404 + + +def test_document_source_missing_file_404(monkeypatch, kb_dir): + """Hash is known but the source file was deleted → 404, not a 500.""" + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes(kb_dir, {"h4": {"name": "gone.md", "doc_name": "gone", "type": "md"}}) + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "h4"}, headers=_auth()) + + assert resp.status_code == 404 + + +def test_document_source_requires_auth(monkeypatch, kb_dir): + client = _client(monkeypatch) + _use_named_kb(monkeypatch, kb_dir) + + resp = client.post("/api/v1/document/source", json={"kb": "test-kb", "hash": "h1"}) + + assert resp.status_code == 401 From fc8197b36aacf1dbefb4dc748e8bf538ddccb7db Mon Sep 17 00:00:00 2001 From: mountain Date: Tue, 21 Jul 2026 19:30:55 +0800 Subject: [PATCH 2/2] fix(web): address xhigh code-review findings for the document reader (#197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctness / a11y: - Rebuild the reader drawer on Radix Dialog (like KbSettingsSheet) instead of a hand-rolled overlay: proper modal focus trap, initial + return focus, Escape, and background inert (was: aria-modal with none of it) [#4]. This also removes the hand-rolled window keydown listener that re-subscribed every render [#7]. - Restructure each document row so the open-reader target is a real -
+ { + if (!next) onClose() + }} + > + + {open && ( + + + + + { + openerRef.current = document.activeElement as HTMLElement | null + }} + onCloseAutoFocus={(e) => { + e.preventDefault() + openerRef.current?.focus() + }} + > + +
+ + + +
+ +
{shown?.name}
+ +
+ {shown?.display_type && {shown.display_type}} + {shown?.pages != null && · {t('kb:docs.pages', { count: shown.pages })}} + + + {t('kb:docs.reader.readonly')} + +
+
+ +
-
-
- {loading && ( -
- - {t('kb:docs.reader.loading')} -
- )} - {error && ( -
-

- {t('kb:docs.reader.error', { error })} -

- -
- )} - {!loading && !error && !hasContent && source != null && ( -
- {t('kb:docs.reader.emptyDoc')} -
- )} - {!loading && !error && hasContent && ( -
{body}
- )} -
-
- - +
+
+ {loading && ( +
+ + {t('kb:docs.reader.loading')} +
+ )} + {error && ( +
+

+ {t('kb:docs.reader.error', { error })} +

+ +
+ )} + {!loading && !error && isEmpty && ( +
+ {t('kb:docs.reader.emptyDoc')} +
+ )} + {!loading && !error && !isEmpty && ( +
{body}
+ )} +
+
+ + + + )} + + ) } +/** Documents section: upload dropzone + uploaded-document list + remote + * connector cards. Moved verbatim from the old Sources tab body. */ + function DocumentsPane({ kb, documents, @@ -1046,8 +1031,64 @@ function DocumentsPane({ // `deletingName` is the row whose remove request is in flight. const [confirmName, setConfirmName] = useState(null) const [deletingName, setDeletingName] = useState(null) - // The document whose read-only source drawer is open (null = closed). + // Document reader drawer. State lives HERE (not in the drawer, which unmounts + // on close via Radix) so the per-hash cache and memoized body survive + // open/close and re-opening is instant without re-parsing Markdown. const [openDoc, setOpenDoc] = useState(null) + const [docSource, setDocSource] = useState(null) + const [docLoading, setDocLoading] = useState(false) + const [docError, setDocError] = useState(null) + const [docReloadSeq, setDocReloadSeq] = useState(0) + const sourceCache = useRef>(new Map()) + const closeDrawer = useCallback(() => setOpenDoc(null), []) + const openHash = openDoc?.hash ?? null + + // Drop cached content when the inventory changes (a recompile can rewrite a + // document's converted text under the same raw hash), so the next open + // refetches instead of serving stale text. + useEffect(() => { + sourceCache.current.clear() + }, [documents]) + + // Fetch the open document's source (per-hash cache; retry via docReloadSeq). + useEffect(() => { + if (!openHash) return + const cached = sourceCache.current.get(openHash) + if (cached) { + setDocSource(cached) + setDocError(null) + setDocLoading(false) + return + } + let cancelled = false + setDocLoading(true) + setDocSource(null) + setDocError(null) + getDocumentSource(kb, openHash) + .then((r) => { + if (cancelled) return + sourceCache.current.set(openHash, r) + setDocSource(r) + }) + .catch((e) => { + if (!cancelled) setDocError(errMsg(e)) + }) + .finally(() => { + if (!cancelled) setDocLoading(false) + }) + return () => { + cancelled = true + } + }, [kb, openHash, docReloadSeq]) + + // Parse Markdown once per fetched source (stable cache ref → no re-parse). + const readerBody = useMemo( + () => + docSource && docSource.content.trim() ? : null, + [docSource], + ) + const readerEmpty = docSource != null && docSource.content.trim().length === 0 + const handleDelete = async (name: string) => { setDeletingName(name) try { @@ -1156,39 +1197,35 @@ function DocumentsPane({ {documents.map((d, i) => (
setOpenDoc(d)} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - setOpenDoc(d) - } - }} - title={t('kb:docs.reader.open')} className={cn( 'anim-fade-up rounded-2xl border border-[hsl(var(--glass-border))] glass-2 px-4 py-3 flex items-center gap-3', - 'cursor-pointer transition-colors hover:border-foreground/20 hover:bg-accent/40', - 'focus:outline-none focus-visible:ring-2 focus-visible:ring-accent-brand/50', + 'transition-colors hover:border-foreground/20', `anim-d${Math.min(i + 1, 4)}`, )} > - - - -
-
{d.name}
-
- {d.display_type} - {d.pages != null && <> · {t('kb:docs.pages', { count: d.pages })}} -
-
- {/* Trailing controls (hash chip + delete). stopPropagation so a - click here never opens the reader drawer. */} -
e.stopPropagation()} + {/* The open-reader target is a real + {/* Trailing controls: hash chip + delete, siblings of the open button. */} +
{d.hash && ( {d.hash.slice(0, 8)} @@ -1239,7 +1276,15 @@ function DocumentsPane({
- setOpenDoc(null)} /> + setDocReloadSeq((s) => s + 1)} + onClose={closeDrawer} + /> ) } diff --git a/openkb/api_documents_router.py b/openkb/api_documents_router.py index ccb10f3a..d8c84cd1 100644 --- a/openkb/api_documents_router.py +++ b/openkb/api_documents_router.py @@ -24,7 +24,12 @@ async def document_source_endpoint( _: None = Depends(require_bearer_token), ) -> DocumentSourceResponse: kb_dir = _resolve_kb(request.kb) - result = await run_in_threadpool(read_document_source, kb_dir, request.hash) + try: + result = await run_in_threadpool(read_document_source, kb_dir, request.hash) + except (OSError, ValueError) as exc: + # Corrupt/unreadable source file (bad JSON, unexpected shape, I/O error): + # a controlled 500 with a clean message beats an unhandled stack trace. + raise HTTPException(status_code=500, detail="Could not read document source.") from exc if result is None: raise HTTPException(status_code=404, detail="Document source not found.") return DocumentSourceResponse(**result) diff --git a/openkb/documents.py b/openkb/documents.py index 8df3265d..28409dbf 100644 --- a/openkb/documents.py +++ b/openkb/documents.py @@ -13,6 +13,7 @@ from pathlib import Path from typing import Any +from openkb.cli import _LONG_DOC_TYPES from openkb.state import HashRegistry @@ -20,9 +21,9 @@ def _render_pages(pages: list[dict[str, Any]]) -> str: """Join a long doc's per-page ``content`` into one Markdown string. Pages are separated by a thematic break (``---``) so the reader keeps - page boundaries; blank/missing page content is skipped. + page boundaries; blank/missing content and non-dict entries are skipped. """ - parts = [str(page.get("content", "")).strip() for page in pages] + parts = [str(page.get("content", "")).strip() for page in pages if isinstance(page, dict)] return "\n\n---\n\n".join(part for part in parts if part) @@ -39,8 +40,12 @@ def _resolve_source_file(kb_dir: Path, meta: dict, doc_name: str) -> Path | None stored = meta.get("source_path") if stored: candidates.append(kb_dir / stored) - candidates.append(sources_dir / f"{doc_name}.md") - candidates.append(sources_dir / f"{doc_name}.json") + # Long docs are stored as ``.json`` (per-page), short docs as + # ``.md``. Order the by-convention fallbacks by THIS doc's type so + # two docs sharing a doc_name (one short, one long) each resolve to their + # own file rather than whichever extension is tried first. + exts = (".json", ".md") if meta.get("type") in _LONG_DOC_TYPES else (".md", ".json") + candidates.extend(sources_dir / f"{doc_name}{ext}" for ext in exts) for candidate in candidates: try: resolved = candidate.resolve() @@ -71,6 +76,8 @@ def read_document_source(kb_dir: Path, file_hash: str) -> dict[str, Any] | None: if source.suffix == ".json": pages = json.loads(source.read_text(encoding="utf-8")) + if not isinstance(pages, list): + raise ValueError(f"source JSON is not a page list: {source.name}") content = _render_pages(pages) page_count: int | None = len(pages) else: diff --git a/tests/test_api_documents.py b/tests/test_api_documents.py index c38d8509..861a6edf 100644 --- a/tests/test_api_documents.py +++ b/tests/test_api_documents.py @@ -121,6 +121,64 @@ def test_document_source_missing_file_404(monkeypatch, kb_dir): assert resp.status_code == 404 +def test_document_source_doc_name_collision_resolves_by_type(monkeypatch, kb_dir): + """Two docs share a doc_name (short .md + long .json), and the long entry has + no source_path. Each hash must resolve to its OWN file via the type ordering, + not whichever extension is tried first.""" + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes( + kb_dir, + { + "short": {"name": "dup.md", "doc_name": "dup", "type": "md"}, + "long": {"name": "dup.pdf", "doc_name": "dup", "type": "long_pdf"}, + }, + ) + (kb_dir / "wiki" / "sources" / "dup.md").write_text("SHORT content", encoding="utf-8") + (kb_dir / "wiki" / "sources" / "dup.json").write_text( + json.dumps([{"page": 1, "content": "LONG page one", "images": []}]), encoding="utf-8" + ) + + long_resp = client.post( + "/api/v1/document/source", json={"kb": kb, "hash": "long"}, headers=_auth() + ) + assert long_resp.status_code == 200 + assert "LONG page one" in long_resp.json()["content"] + assert "SHORT content" not in long_resp.json()["content"] + assert long_resp.json()["pages"] == 1 + + short_resp = client.post( + "/api/v1/document/source", json={"kb": kb, "hash": "short"}, headers=_auth() + ) + assert short_resp.json()["content"] == "SHORT content" + + +def test_document_source_malformed_json_is_controlled_500(monkeypatch, kb_dir): + """A corrupt/unexpected source JSON returns a controlled 500 with a clean + message, not an unhandled exception.""" + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes(kb_dir, {"h": {"name": "broken.pdf", "doc_name": "broken", "type": "long_pdf"}}) + (kb_dir / "wiki" / "sources" / "broken.json").write_text("{not valid json", encoding="utf-8") + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "h"}, headers=_auth()) + + assert resp.status_code == 500 + assert "Could not read" in resp.json()["detail"] + + +def test_document_source_non_list_json_is_controlled_500(monkeypatch, kb_dir): + """A source JSON that parses but is not a page list is rejected cleanly.""" + client = _client(monkeypatch) + kb = _use_named_kb(monkeypatch, kb_dir) + _write_hashes(kb_dir, {"h": {"name": "odd.pdf", "doc_name": "odd", "type": "long_pdf"}}) + (kb_dir / "wiki" / "sources" / "odd.json").write_text('{"pages": 3}', encoding="utf-8") + + resp = client.post("/api/v1/document/source", json={"kb": kb, "hash": "h"}, headers=_auth()) + + assert resp.status_code == 500 + + def test_document_source_requires_auth(monkeypatch, kb_dir): client = _client(monkeypatch) _use_named_kb(monkeypatch, kb_dir)