feat: add REST API server and Knowledge Workbench web UI#150
Conversation
|
@jidechao
I'll follow up with implementation feedback after. |
…p BOM, unvendor web/ - config.yaml.example: restore gpt-5.4 / en (unintentional flip to local dev values openai/deepseek-v4-flash / zh) - openkb/cli.py, tests/test_api.py: remove stray UTF-8 BOM - web/: stop vendoring the built bundle; add web/ to .gitignore; README notes the Workbench UI requires `npm run build` (API-only otherwise) Refs: PR VectifyAI#150
|
|
@KylinMountain Thanks for the review. All three actionable points are addressed in the latest push (
Verification: Standing by for the implementation-layer feedback. |
KylinMountain
left a comment
There was a problem hiding this comment.
Review — REST API + Knowledge Workbench
Nice, largely-additive feature — CLI behavior is unchanged and the shared SSE helper is only consumed by the new API paths. The high-severity issues cluster in three areas, flagged inline.
Must-fix (correctness / data):
- Mutation endpoints (
recompile/lint --fix/init) run sync, lock-holding work directly on the event loop;remove/addalready userun_in_threadpool, these don't. Becausekb_ingest_lock's reentrancy bookkeeping isthreading.local, concurrent same-KB requests on the one event-loop thread bypass mutual exclusion → KB corruption (different-KB requests instead stall the loop). [api.py:321/541/581] - Watcher
stop()can orphan an in-flight compile and a restart then double-ingestsraw/. [watch_service.py:204] _save_query_answer: empty/colliding exploration slug silently loses data (acute for CJK questions), drops the CLI's ghost-link stripping, and writes the question into YAML unescaped. [api.py:801]
Should-fix: leaked/never-terminating /watch/events SSE [api.py:1145]; frontend doesn't abort the stream on unmount/KB-switch [useSSEStream.js]; CORS * + credentials [api.py:699]; token sent to a user-supplied origin + non-constant-time compare [sse.js:50, api.py:772]; per-KB LLM key ignored due to override=False [api.py:321].
Verified clean (so you don't re-chase): KB short-names go through validate_kb_name (fullmatch on [A-Za-z0-9_-]+) → no path traversal; upload filenames via Path(...).name; the .env inheritance filter correctly excludes OPENKB_* server vars while keeping LLM_API_KEY; _sse always emits single-line data:; CLI query/chat streaming is unchanged by the refactor.
Minor / cleanup: duplicated KB-dir check (api.py:730 & 787 → extract _is_kb_dir); redundant _setup_llm_key wrapper; dead model.dict() Pydantic-v1 branch.
Thanks for the thorough test plan and the offer to split the PR — splitting API vs Web UI would make these land more incrementally.
|
Also, I suggest setting the default webpage language to English, but with a language switcher available—that might work better. @jidechao |
Follow-up: structure & docs organizationA few organizational notes beyond the inline correctness comments — all about keeping the surface lean and single-sourced. 1.
|
…I#150) Must-fix (correctness/data): - Serialize same-KB mutations with an asyncio.Lock before kb_ingest_lock. The ingest lock tracks reentrancy in threading.local, but the event loop runs every request on one thread, so concurrent same-KB requests were mis-counted as re-entrant and bypassed mutual exclusion. [init/lint --fix/recompile] - Watcher stop() now joins the worker before clearing state and leaves a draining marker, so a restart can no longer orphan an in-flight compile and double-ingest raw/. [watch_service.py] - Extract a shared save_exploration() used by both CLI and REST: strips ghost wikilinks, generates a CJK-safe slug (hash fallback), uniquifies on collision, and escapes the question for YAML frontmatter. [cli.py] Should-fix: - Cap /watch/events SSE with a default timeout and an is_disconnected check. - Abort the in-flight SSE on component unmount / KB switch (AbortController). - Force allow_credentials off when CORS origins is a wildcard. - Constant-time token compare (hmac.compare_digest); the SPA warns when the configured API base is cross-origin and non-HTTPS. - Isolate per-KB LLM credentials via LlmCredentialBundle (dotenv_values, no os.environ pollution; a per-request LitellmModel instance). Regression (introduced by the per-KB bundle isolation above): - build_run_config_from_bundle passed model=f"litellm/{model}" to LitellmModel, but LitellmModel feeds model verbatim to litellm.acompletion, so the prefix produced "litellm/openai/deepseek-v4-flash" and litellm rejected it as an unknown provider. CLI was unaffected (bundle=None); all REST query/chat failed. Refactor/cleanup: - Extract _is_kb_dir; move _setup_llm_key to cli; model_dump() (pydantic v2). - Drop the hand-maintained openkb-postman.json (OpenAPI is served at /docs). - Slim README to short subsections + links; move the REST reference and Workbench tour to examples/rest-api/README.md. Frontend: - Default the UI to English with a zh/en toggle (i18n.jsx). - index.html lang/title default to English. Tests: 118 passed (query/api/api_watch/watch_service/per_request_overrides/save_exploration).
|
@KylinMountain Thanks for the thorough review — all of it is addressed in d7e82b8. Replying point by point below. Review summaryMust-fix
Should-fix
Minor/cleanup
Follow-up: languageDefault is now English with a zh/en toggle (icon button in the sidebar, persisted to Follow-up: structure & docs
Verification
Screenshots from the first round are in the thread above; happy to capture fresh ones against the updated UI if useful. |
|
@jidechao there's conflicts you need to resolve, thanks. |
…p BOM, unvendor web/ - config.yaml.example: restore gpt-5.4 / en (unintentional flip to local dev values openai/deepseek-v4-flash / zh) - openkb/cli.py, tests/test_api.py: remove stray UTF-8 BOM - web/: stop vendoring the built bundle; add web/ to .gitignore; README notes the Workbench UI requires `npm run build` (API-only otherwise) Refs: PR VectifyAI#150 Co-authored-by: jidechao <408645320@qq.com>
d7e82b8 to
8d5bb89
Compare
…I#150) Must-fix (correctness/data): - Serialize same-KB mutations with an asyncio.Lock before kb_ingest_lock. The ingest lock tracks reentrancy in threading.local, but the event loop runs every request on one thread, so concurrent same-KB requests were mis-counted as re-entrant and bypassed mutual exclusion. [init/lint --fix/recompile] - Watcher stop() now joins the worker before clearing state and leaves a draining marker, so a restart can no longer orphan an in-flight compile and double-ingest raw/. [watch_service.py] - Extract a shared save_exploration() used by both CLI and REST: strips ghost wikilinks, generates a CJK-safe slug (hash fallback), uniquifies on collision, and escapes the question for YAML frontmatter. [cli.py] Should-fix: - Cap /watch/events SSE with a default timeout and an is_disconnected check. - Abort the in-flight SSE on component unmount / KB switch (AbortController). - Force allow_credentials off when CORS origins is a wildcard. - Constant-time token compare (hmac.compare_digest); the SPA warns when the configured API base is cross-origin and non-HTTPS. - Isolate per-KB LLM credentials via LlmCredentialBundle (dotenv_values, no os.environ pollution; a per-request LitellmModel instance). Regression (introduced by the per-KB bundle isolation above): - build_run_config_from_bundle passed model=f"litellm/{model}" to LitellmModel, but LitellmModel feeds model verbatim to litellm.acompletion, so the prefix produced "litellm/openai/deepseek-v4-flash" and litellm rejected it as an unknown provider. CLI was unaffected (bundle=None); all REST query/chat failed. Refactor/cleanup: - Extract _is_kb_dir; move _setup_llm_key to cli; model_dump() (pydantic v2). - Drop the hand-maintained openkb-postman.json (OpenAPI is served at /docs). - Slim README to short subsections + links; move the REST reference and Workbench tour to examples/rest-api/README.md. Frontend: - Default the UI to English with a zh/en toggle (i18n.jsx). - index.html lang/title default to English. Tests: 118 passed (query/api/api_watch/watch_service/per_request_overrides/save_exploration). Co-authored-by: jidechao <408645320@qq.com>
…p BOM, unvendor web/ - config.yaml.example: restore gpt-5.4 / en (unintentional flip to local dev values openai/deepseek-v4-flash / zh) - openkb/cli.py, tests/test_api.py: remove stray UTF-8 BOM - web/: stop vendoring the built bundle; add web/ to .gitignore; README notes the Workbench UI requires `npm run build` (API-only otherwise) Refs: PR VectifyAI#150 Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
…I#150) Must-fix (correctness/data): - Serialize same-KB mutations with an asyncio.Lock before kb_ingest_lock. The ingest lock tracks reentrancy in threading.local, but the event loop runs every request on one thread, so concurrent same-KB requests were mis-counted as re-entrant and bypassed mutual exclusion. [init/lint --fix/recompile] - Watcher stop() now joins the worker before clearing state and leaves a draining marker, so a restart can no longer orphan an in-flight compile and double-ingest raw/. [watch_service.py] - Extract a shared save_exploration() used by both CLI and REST: strips ghost wikilinks, generates a CJK-safe slug (hash fallback), uniquifies on collision, and escapes the question for YAML frontmatter. [cli.py] Should-fix: - Cap /watch/events SSE with a default timeout and an is_disconnected check. - Abort the in-flight SSE on component unmount / KB switch (AbortController). - Force allow_credentials off when CORS origins is a wildcard. - Constant-time token compare (hmac.compare_digest); the SPA warns when the configured API base is cross-origin and non-HTTPS. - Isolate per-KB LLM credentials via LlmCredentialBundle (dotenv_values, no os.environ pollution; a per-request LitellmModel instance). Regression (introduced by the per-KB bundle isolation above): - build_run_config_from_bundle passed model=f"litellm/{model}" to LitellmModel, but LitellmModel feeds model verbatim to litellm.acompletion, so the prefix produced "litellm/openai/deepseek-v4-flash" and litellm rejected it as an unknown provider. CLI was unaffected (bundle=None); all REST query/chat failed. Refactor/cleanup: - Extract _is_kb_dir; move _setup_llm_key to cli; model_dump() (pydantic v2). - Drop the hand-maintained openkb-postman.json (OpenAPI is served at /docs). - Slim README to short subsections + links; move the REST reference and Workbench tour to examples/rest-api/README.md. Frontend: - Default the UI to English with a zh/en toggle (i18n.jsx). - index.html lang/title default to English. Tests: 118 passed (query/api/api_watch/watch_service/per_request_overrides/save_exploration). Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
8d5bb89 to
1ee9997
Compare
Add a production-ready REST API (FastAPI) and a bundled React single-page
app ("Knowledge Workbench") served at the same origin, so OpenKB can be
driven from the browser, Postman, or other HTTP clients without the CLI.
REST API (openkb/api.py):
- Endpoints under /api/v1: /kbs, /init, /add, /query, /chat,
/chat/sessions{,/load,/delete}, /list, /status, /lint, /remove,
/recompile, /watch/{start,stop,status}, /watch/events (SSE)
- Bearer-token auth, SSE streaming for query/chat/add/remove/recompile,
multipart upload, and KB name resolution via OPENKB_KB_ROOT
- Shared SSE event layer (iter_agent_response_events) reused by query/chat
so the CLI and API emit identical event streams
- Background file-watcher service (openkb/watch_service.py) for auto-compile
Knowledge Workbench (frontend/, built to web/):
- React + Vite dark three-pane workbench: Overview, Documents, Query,
Chat, Maintenance, with a live retrieval/reasoning inspector timeline
- Streamed answers, multi-turn chat with persisted sessions, drag-and-drop
upload with per-file progress, lint/recompile/watch controls
- Markdown via react-markdown + remark-gfm + rehype-highlight
Also: initialize_kb inherits project-root config.yaml and .env (filtering
OPENKB_* server vars) so a KB created from the UI runs out of the box;
POSTMAN collection; README docs.
Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
…p BOM, unvendor web/ - config.yaml.example: restore gpt-5.4 / en (unintentional flip to local dev values openai/deepseek-v4-flash / zh) - openkb/cli.py, tests/test_api.py: remove stray UTF-8 BOM - web/: stop vendoring the built bundle; add web/ to .gitignore; README notes the Workbench UI requires `npm run build` (API-only otherwise) Refs: PR VectifyAI#150 Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
…I#150) Must-fix (correctness/data): - Serialize same-KB mutations with an asyncio.Lock before kb_ingest_lock. The ingest lock tracks reentrancy in threading.local, but the event loop runs every request on one thread, so concurrent same-KB requests were mis-counted as re-entrant and bypassed mutual exclusion. [init/lint --fix/recompile] - Watcher stop() now joins the worker before clearing state and leaves a draining marker, so a restart can no longer orphan an in-flight compile and double-ingest raw/. [watch_service.py] - Extract a shared save_exploration() used by both CLI and REST: strips ghost wikilinks, generates a CJK-safe slug (hash fallback), uniquifies on collision, and escapes the question for YAML frontmatter. [cli.py] Should-fix: - Cap /watch/events SSE with a default timeout and an is_disconnected check. - Abort the in-flight SSE on component unmount / KB switch (AbortController). - Force allow_credentials off when CORS origins is a wildcard. - Constant-time token compare (hmac.compare_digest); the SPA warns when the configured API base is cross-origin and non-HTTPS. - Isolate per-KB LLM credentials via LlmCredentialBundle (dotenv_values, no os.environ pollution; a per-request LitellmModel instance). Regression (introduced by the per-KB bundle isolation above): - build_run_config_from_bundle passed model=f"litellm/{model}" to LitellmModel, but LitellmModel feeds model verbatim to litellm.acompletion, so the prefix produced "litellm/openai/deepseek-v4-flash" and litellm rejected it as an unknown provider. CLI was unaffected (bundle=None); all REST query/chat failed. Refactor/cleanup: - Extract _is_kb_dir; move _setup_llm_key to cli; model_dump() (pydantic v2). - Drop the hand-maintained openkb-postman.json (OpenAPI is served at /docs). - Slim README to short subsections + links; move the REST reference and Workbench tour to examples/rest-api/README.md. Frontend: - Default the UI to English with a zh/en toggle (i18n.jsx). - index.html lang/title default to English. Tests: 118 passed (query/api/api_watch/watch_service/per_request_overrides/save_exploration). Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
1ee9997 to
af09640
Compare
Co-authored-by: jidechao <62241490+jidechao@users.noreply.github.com>
- Split add-upload handling into a fast reservation (unique-path allocation + placeholder, under the per-KB mutation lock) and the body transfer (outside the lock), so a large or slow upload no longer blocks lint/recompile/other adds on the same KB while same-name uploads stay race-free. - Persist a query stream's final answer (save + log) before the disconnect check, so a client that drops at the last moment does not lose a save=True write; only the client-facing SSE frame is skipped. Mirrors the chat stream, which records the turn before yielding. - resolve_credential_bundle now falls back kb-local .env -> process env -> global ~/.config/openkb/.env (kb-local wins), so process-env and global-.env deployments (Docker/systemd) keep working, still without mutating os.environ. - Move process-wide setup (load_dotenv/tracing/litellm) into create_app() and run via uvicorn factory, so importing the module has no global side effects. - Return a JSON 404 for unknown /api/* paths instead of the SPA's index.html. - watch/stop reports the real post-stop status; the watcher resolves a per-KB credential bundle at start and threads it through background ingests. - Tests: update _add_for_api mocks for the bundle arg and cover the credential fallback. Add fastapi/uvicorn/python-multipart (api extra) and httpx (dev).
- Chat and Query clear their message list (and Chat its session) when the active KB changes, so stale answers from the previous KB do not linger. - hasConnection() again treats a configured token as connected; a cross-origin non-HTTPS base is advisory only (checkBaseSafety warns before each request) and no longer blocks LAN/dev http:// endpoints from connecting.
…kbench # Conflicts: # README.md
Previously the bundled web UI existed only after a source-tree `cd frontend && npm run build`, so `pip install "openkb[api]"` from PyPI served no UI (`/` 404'd). Build the SPA into the package and ship it in the wheel so the Workbench is available straight from a normal install, while keeping the generated bundle out of git (the unvendor decision stands): - Vite builds into openkb/web (was ../web), so the bundle lives inside the package rather than a top-level dir that would pollute site-packages. - hatchling `artifacts` force-includes openkb/web in the sdist and wheel even though it stays git-ignored, so the built UI ships without committing generated JS. The build still succeeds when the bundle is absent (API-only). - publish.yml builds the frontend (npm ci && npm run build) before packaging. - _mount_web_ui resolves the bundle next to the module (openkb/web), covering installed wheels and source checkouts alike. - README: install via `pip install "openkb[api]"` + `openkb-api`; the from-source npm build is now just a dev note. Trim the overlong Workbench feature bullet to match its siblings. The UI is "optional" via the [api] extra (server deps) — no second package needed; the ~520KB static bundle is inert without the server. Verified locally: `uv build` includes openkb/web in both sdist and wheel, and the server serves index.html + assets at /.
The Workbench is a local-first tool: it should start and open in the browser with no configuration. Auth was mandatory (require_bearer_token returned 500 when OPENKB_API_TOKEN was unset), and the SPA popped a Connection dialog demanding a token before it would even talk to its own same-origin API. Flip auth to opt-in: - require_bearer_token: with no OPENKB_API_TOKEN configured the API is open (the local default). Set it and every request must carry the bearer token; a missing/wrong one is 401. - main(): warn on startup when bound to a non-loopback host without a token, so an exposed server can't be silently world-open. - Frontend: don't prompt on load. The SPA calls the same-origin API directly; the Connection dialog opens only reactively when a request 401s (i.e. the server has a token configured) via the existing openkb:unauthorized event. - Tests: replace the old "no token -> 500" assertions (test_api / test_api_watch) with the new contract — no token -> requests allowed; token set -> 401 when the header is missing. A deployer who exposes the server sets OPENKB_API_TOKEN to require auth.
- Install via `pip install "openkb[api]"` and run `openkb-api` with zero config; the bundled UI now ships in the wheel, so the npm build is only a source-checkout note. Drop the stale "no build -> / returns 404" claim. - Document auth as opt-in in the guide's Authentication section: OPENKB_API_TOKEN unset -> open (local default), set -> bearer token required (else 401), with an "exposing the server" warning. Fix the stale "OPENKB_API_TOKEN is required" line and the "500 when not configured" status code. - Note the Workbench connects to the local API with no prompt when no token is set (the Connection dialog is now only for a configured token / remote base).
Add a `web` job (npm ci && npm run build) so a broken frontend — vite/config error, stale package-lock, JS error — is caught at PR time instead of only in publish.yml after a release tag is pushed. Mirrors the release build step and its pinned action SHAs.
…he CLI The exposed-without-token warning lived only in main(), so a non-CLI launch (uvicorn/gunicorn --factory) that binds a public interface without a token got a silently world-open API with no warning. Move it into create_app()'s lifespan startup so it fires for every launch path. create_app() can't see the bind host under a factory launch, so the check is host-agnostic (warn whenever no token is configured) — a single calm log line, harmless on loopback, and silent in the unit suite (tests don't enter the TestClient context manager, so lifespan never runs there).
PR VectifyAI#150's CI had never actually run — fork-PR workflows sat in `action_required` pending maintainer approval — so several failures accumulated undetected across the PR's own new code: - ruff E402: hoist mid-file imports to the top of tests/test_api.py. - ruff format: reformat 6 PR-touched files (cli.py, agent/chat.py, agent/compiler.py, watch_service.py, tests/test_remove.py, tests/test_watch_service.py) — deterministic, no logic change. - mypy (cli.py iter_recompile): assert the guarded non-None doc_name before _resolve_doc_identifier, and annotate the per-doc result dict as dict[str, Any] so the float `elapsed` entries type-check. All four CI steps (ruff check ., ruff format --check ., mypy openkb, pytest) pass locally; 1102 tests green.





Summary
Adds a production-ready REST API (FastAPI) and a bundled Knowledge Workbench web UI to OpenKB, so a knowledge base can be built, queried, and maintained entirely from the browser, Postman, or any HTTP client — no CLI required.
The two are co-served from one origin:
openkb-apimounts the built SPA at/while exposing the JSON/SSE API under/api/v1.REST API (
openkb/api.py,openkb/watch_service.py)GET /kbs+POST /init— list and create knowledge bases (resolvable by short name viaOPENKB_KB_ROOT)POST /add(multipart, SSE) — upload + compile documents with per-file progressPOST /query(SSE) — one-shot question, streamed answerPOST /chat+POST /chat/sessions{,/load,/delete}(SSE) — multi-turn chat with persisted, resumable sessionsPOST /list,/status— inventory and index statsPOST /lint(optionalfix),POST /remove(SSE),POST /recompile(SSE)POST /watch/{start,stop,status}+GET /watch/events(SSE) — background file watcheriter_agent_response_eventslayer means the CLI and API emit identical event streamsopenkb-postman.jsonKnowledge Workbench (
frontend/→ builtweb/)A React + Vite single-page app (dark, three-pane):
initialize_kbinherits the operator's project-rootconfig.yamland LLM credentials (.env, withOPENKB_*server vars filtered out), so a KB created from the UI runs out of the box.Build / packaging
web/holds the pre-built bundle and is mounted by the API;frontend/source is included andnode_modules/distare gitignoredopenkb-api(alsopython -m openkb.api)Test plan
pytest tests/test_api.py tests/test_api_watch.py tests/test_watch_service.py tests/test_remove.py— 126 passednpm run buildproducesweb/index.html+web/assets/*tool_calltimeline, chat create/resume/delete, lint/recompile/watchNotes