Simplify LLM API key configuration - #6
Merged
Conversation
- Hardcode reading LLM_API_KEY env var instead of indirecting through config - Remove llm_api_key_env from DEFAULT_CONFIG, okb init prompts, and config.yaml - Provider-specific env vars (OPENAI_API_KEY, etc.) still work via LiteLLM auto-detection - One less config field, one less okb init step
rejojer
force-pushed
the
simplify-llm-api-key-config
branch
from
April 7, 2026 22:15
c781a30 to
55444d3
Compare
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
KylinMountain
added a commit
that referenced
this pull request
May 24, 2026
Architectural review (4 parallel Opus auditors) found that the skill_runner core was already generic, but the deck SURFACE was still fused to Editorial Monocle. Fixed: * validator: now takes optional `grammar` param (DeckGrammar TypedDict); skill-agnostic by default (only checks file present, parses, ≥5 slides, self-contained). Third-party deck skills (guizang, swiss) now pass validation cleanly. Editorial-specific rules opt-in via `EDITORIAL_MONOCLE_GRAMMAR`. (finding #2) * skills/openkb-deck-editorial/SKILL.md: declares its grammar + output_path_template under `od:` frontmatter — `run_skill` reads these and applies them post-run. * run_skill: now honors frontmatter `od.mode`, `od.output_path_template`, `od.deck_grammar`. When mode=="deck" and template is set, the runner injects the path into intent, verifies the file exists post-run, and runs validate_deck with the skill's grammar. Validation result is returned via new SkillRunResult dataclass. (findings #4, #5) * `openkb deck new --skill <name>`: CLI flag accepts any installed deck skill (default openkb-deck-editorial). guizang and swiss now usable from the scripted CLI, not only freeform chat. (finding #1) * `/deck new --skill <name>` chat slash: same flag, parsed positionally alongside --critique. (finding #1) * tests/test_read_kb_file.py: 13 new tests mirroring test_write_kb_file for the read-side allow-list. Pins refusal of `.openkb/config.yaml`, `.env`, `raw/`, `..` traversal, absolute paths. (finding #6) * Generator deck branch: no longer calls validate_deck directly; just propagates run_deck_create's SkillRunResult.validation up. Validation is now a property of "this skill declared mode=deck", not of "this CLI path was taken". Existing tests updated: * tests/test_deck_validator.py: explicit grammar arg on Editorial- specific tests; added test_guizang_shape_passes_generic_mode + test_missing_cover_ignored_in_generic_mode to pin both modes. * tests/test_deck_creator.py: mocks return SkillRunResult; new test_run_deck_create_honors_skill_name_override for --skill flag. * tests/test_generator.py: deck dispatch test mocks SkillRunResult. Below-threshold findings deferred: * Generator if/else → registry (score 70) — works, just not extensible via plugin; future. * Iteration backup in chat freeform path (score 75) — needs write_kb_file hook; separate change. * run_skill / scan_local_skills / _handle_slash_critique direct tests (scores 60-70) — covered indirectly by integration; can add later. Regression: 538 tests pass (was 523 pre-fix; net +15 = 13 new read_kb_file tests + 2 new validator-mode tests).
KylinMountain
added a commit
that referenced
this pull request
Jul 21, 2026
…es (#198) * feat(api): delete a knowledge base (POST /api/v1/kb/delete + openkb delete-kb) Physical, irreversible KB deletion with a type-the-name confirmation. - config.delete_kb: rmtree the KB directory + unregister it from the global registry (known_kbs / kb_aliases / default_kb, via the new unregister_kb). Guards against deleting a non-KB path; tolerates a ghost registry entry (directory already gone) by just unregistering. - POST /api/v1/kb/delete: confirm_name must equal kb, re-checked server-side. Lives in a new api_kbs_router (sibling of the config/graph/output routers) so api.py stays under the 800-line gate; delete_kb self-locks (config lock), no create_app closure needed. - openkb delete-kb NAME: type-the-name prompt (or --yes) then delete. Tests: endpoint (success + unregister, confirm mismatch, non-KB target) in test_api.py; config (physical delete + unregister, refuse non-KB, ghost tolerance) in test_config.py. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * feat(api): delete a wiki page (POST /api/v1/page/delete) with backlink impact Delete a concept/entity page and degrade references cleanly instead of leaving them dangling: - page_ops.delete_wiki_page: dry_run reports the backlink pages (whose inbound [[wikilinks]] would be demoted) without touching anything; execute removes the page under the KB ingest lock, strips its index.md entry outright (compiler.remove_doc_from_index — the entry is a [[link]] line), and demotes the now-dangling inbound [[links]] to plain text in ONLY those backlink pages (lint.fix_broken_links restrict_to). The page ref is "section/stem", validated to concepts/ or entities/ only (path-traversal-safe). - New api_pages_router hosts POST /api/v1/page (read — relocated here from api.py to keep it under the 800-line gate) + POST /api/v1/page/delete. Tests: dry-run impact + execute (page gone, inbound link demoted to text, index entry removed, sibling kept), 404 on missing page, and rejection of unsafe / non-editable refs. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * feat(api): edit a wiki page (PUT /api/v1/page) + link context (POST /api/v1/page/links) - page_ops.edit_wiki_page: replace a concept/entity page's BODY while preserving its code-managed OKF frontmatter (type/description/sources) verbatim; any frontmatter in the submitted content is dropped. Dead [[wikilinks]] the user typed are demoted to plain text on save (OpenKB keeps the wiki broken-link-free) and returned as ghosts_stripped so the UI can warn. Atomic write under the KB ingest lock. - page_ops.page_link_context (POST /api/v1/page/links): outbound + inbound links for the edit-impact panel. Editing the body does not break either (links are path-based), so this is context, not a blocker — the honest impact model. - PUT /api/v1/page added to api_pages_router. Tests: edit preserves frontmatter + keeps a resolvable link + demotes a dead one (reports ghosts_stripped) + replaces the body; 404 on a missing page; links reports out/backlinks. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * feat(web): delete a knowledge base from the settings sheet (type-name confirm) Adds a Danger Zone section to KbSettingsSheet: a type-the-name confirmation gates deleteKb() (POST /api/v1/kb/delete); on success KbDetail navigates back to the KB list. New deleteKb client + kbSettings danger* keys + common cancel (zh + en, identical key sets). Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * feat(web): in-reader page edit + delete with impact preview (F2/F3) For an open concept/entity page, the reader header now shows Edit and Delete: - Delete: a dry-run first (deletePage dryRun) surfaces the backlink pages whose [[links]] will demote to plain text; a red confirm card lists them, then the real delete closes the page and refreshes the inventory. - Edit: a body textarea (frontmatter is preserved server-side), Save via PUT /api/v1/page, a links panel (getPageLinks: out/backlinks), a "recompile may overwrite" note, and a toast listing any dead links demoted to text. Adds the deletePage/editPage/getPageLinks wiki clients and kb:pageOps.* locale keys (zh + en, identical key sets). Build green (i18n guard + tsc + vite). Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * fix(workbench): address code-review findings (content-mgmt correctness + concurrency) Backend (page_ops / kb_admin / lint): - delete/edit now do their read-modify-write INSIDE the KB ingest lock and re-check the page exists under it: no stale backlink snapshot, no resurrecting a concurrently-deleted page, no 500 on a racing unlink (missing_ok=True). [#4,#5,#6] - delete_kb serializes rmtree under the ingest lock (was fully unlocked). Moved delete_kb/unregister_kb into a new openkb/kb_admin.py so config.py drops back under the per-file line gate (751 lines). [#1,#15] - page deletion no longer scans or rewrites lint's _EXCLUDED_FILES (AGENTS.md/SCHEMA.md/log.md); fix_broken_links(restrict_to) also refuses them, which protects `openkb remove` too. [#2] - edit_wiki_page treats `content` as the body verbatim — no naive frontmatter split that silently dropped a body opening with a '---' block. [#3] - delete uses the real on-disk stem for the exact-match index-entry removal (case-insensitive FS), and adds index.md to the demotion set so a [[target]] embedded in another entry's brief no longer dangles. [#7,#9] API / CLI: - delete-KB endpoint & CLI can now clean a ghost registry entry (registered name whose directory was removed by hand), catch delete_kb's ValueError -> 400, and the endpoint is typed `-> KbDeleteResponse`. [#8,#13,E5] Frontend (KbDetail): - Deleting a KB dispatches `openkb:reload-kbs` so the sidebar refreshes immediately (was stale until a manual reload) — live-test fix. - Removed the dead `pageReloadSeq` path (editPage always returns content). [#14] Tests: excluded-doc-untouched, '---'-body preservation, ghost-KB unregister; non-KB-target test updated to the new resolution model. Backend 1234 passed, mypy/ruff clean, frontend build green. Not changed (intentional): pages_linking_to is NOT folded into build_graph (that excludes explorations/ backlinks) [#10]; the preview+confirm delete inherently scans per request [#11]; EDITABLE_SECTIONS stays duplicated in the frontend, which can't import the Python constant [#12]. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * feat(workbench): allow editing summary pages (edit ⊇ delete section sets) Summaries are compiled markdown like concepts/entities (a recompile regenerates them), so they are now editable via PUT /api/v1/page. They remain NOT independently deletable — a summary is removed by deleting its source document. Split page_ops into EDITABLE_SECTIONS (concepts/entities/summaries) vs DELETABLE_SECTIONS (concepts/entities); validate_page_ref takes the allowlist. Frontend ReaderBody gates Edit on canEdit (incl. summaries), Delete on canDelete. Test: summary editable (frontmatter preserved) + summary delete rejected (400). Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * fix(api): cross-platform delete_kb + endpoint OSError handling (xhigh review #1,#2) - delete_kb no longer holds the ingest lock DURING rmtree (the lock file lives inside the KB dir; Windows cannot delete an open file — the prior review-fix regressed this). It now takes the lock as a BARRIER (drain + wait out any in-flight mutation), releases it, re-checks existence, then rmtrees. [#1] - delete-KB endpoint maps FileNotFoundError to an idempotent success (a concurrent delete already removed the tree) and other OSError to a clean 500 with a message, instead of an uncaught 500 stack trace. [#2] Tests: endpoint OSError -> clean 500, FileNotFoundError -> 200 deleted. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG
KylinMountain
added a commit
that referenced
this pull request
Jul 22, 2026
…y-list inherit, silent reads, set-diff - config: define DEFAULT_ENTITY_TYPES before DEFAULT_CONFIG and add `entity_types` to DEFAULT_CONFIG so the key layers like every other GLOBAL_SCALAR_KEY and always appears in the effective config (#1). - config: resolve_effective_config treats an empty entity_types list the same as null → inherit, so a KB that cleared its override doesn't pin an empty vocabulary (#2). - config: resolve_entity_types(config, *, warn=True); config-read paths pass warn=False so a plain GET doesn't spam coercion warnings (#4). - api_config: both read paths call resolve_entity_types(..., warn=False). - KbSettingsSheet: inherited badge shows the effective list, not `globalValue ?? effective`; drop the now-unused globalValue prop (#3). - Settings: global entity_types diff is order-insensitive — the vocabulary is a set, so re-adding a removed type is not a change (#6). Backend pytest 1240 passed; ruff/format/mypy clean; frontend build green. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG
KylinMountain
added a commit
that referenced
this pull request
Jul 22, 2026
…ane (#197) (#199) * feat(web): read a document's converted source text in the Documents pane (#197) 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 <doc_name>.md; long docs concatenate the per-page <doc_name>.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/<doc_name>.{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. * fix(web): address xhigh code-review findings for the document reader (#197) 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 <button> and the delete control is a SIBLING, not nested. Keyboard-activating delete no longer bubbles into opening the reader, and the invalid nested-interactive markup is gone [#1, #5]. - Resolve a source file by the doc's own type (long → .json first, else .md), so two docs sharing a doc_name each resolve to their own file rather than whichever extension is tried first [#2]. - Guard source reads: skip non-dict page entries, reject non-list JSON, and return a controlled 500 on corrupt/unreadable sources instead of an unhandled exception [#3]. - Invalidate the per-hash content cache when the inventory changes, so a reopen after recompile refetches instead of serving stale text [#6]. Fetch/cache/memoized body moved to DocumentsPane so they survive the drawer's unmount-on-close. #8 (frontmatter stripping) intentionally not applied: source docs render verbatim (a user's own frontmatter is content, unlike wiki-page OKF metadata). Adds tests for the collision and malformed-JSON paths.
KylinMountain
added a commit
that referenced
this pull request
Jul 22, 2026
…verride) (#200) * feat(api): configurable entity_types (global + per-KB) via the config API entity_types (the entity-extraction vocabulary) is now surfaced through the config read/write API, layering global.yaml -> KB config.yaml like the other scalars: a KB list overrides the global list wholesale, an explicit null inherits, and unset falls back to DEFAULT_ENTITY_TYPES. The compiler already consumed config["entity_types"] via resolve_entity_types; this just exposes it. - GLOBAL_SCALAR_KEYS gains "entity_types" (layering + per-key `sources` tracking; the value-not-None-wins rule is type-agnostic, so it works for a list). - _KbConfigWritable / GlobalConfigValues / KbConfigResponse / GlobalConfigResponse carry entity_types; read_kb_config/read_global_config report the cleaned EFFECTIVE list (resolve_entity_types) plus the raw global value for the badge. - PATCH /api/v1/kb/config and PATCH /api/v1/config accept entity_types. Tests: KB override (cleaned + source 'kb') + null revert, global patch, global inheritance; updated the global-defaults shape assertion. Frontend UI follows. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * feat(web): entity-types config UI — chips editor (global default + per-KB override) - EntityTypesEditor: shared controlled chips editor (Enter/comma to add, x to remove; "other" is a fixed always-included chip; IME-safe composition). - KbSettingsSheet: an EntityTypesRow with the same inherit/override Switch as the scalar rows — turning override on seeds+persists the KB's own list, off reverts via null; inherited state shows the global/default list as a badge. Each chip change persists and adopts the server-cleaned response. - Settings (general tab): a global entity-types chips editor, order-sensitive diff into the save patch (joins the existing dirty/SaveBar flow). - "changes affect future recompiles only" note on both surfaces. New keys in common/kbSettings/settings (zh + en, identical sets). Build green (i18n guard OK). Backend was committed in 92f8f41. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG * fix(entity-types): address xhigh review — DEFAULT_CONFIG parity, empty-list inherit, silent reads, set-diff - config: define DEFAULT_ENTITY_TYPES before DEFAULT_CONFIG and add `entity_types` to DEFAULT_CONFIG so the key layers like every other GLOBAL_SCALAR_KEY and always appears in the effective config (#1). - config: resolve_effective_config treats an empty entity_types list the same as null → inherit, so a KB that cleared its override doesn't pin an empty vocabulary (#2). - config: resolve_entity_types(config, *, warn=True); config-read paths pass warn=False so a plain GET doesn't spam coercion warnings (#4). - api_config: both read paths call resolve_entity_types(..., warn=False). - KbSettingsSheet: inherited badge shows the effective list, not `globalValue ?? effective`; drop the now-unused globalValue prop (#3). - Settings: global entity_types diff is order-insensitive — the vocabulary is a set, so re-adding a removed type is not a change (#6). Backend pytest 1240 passed; ruff/format/mypy clean; frontend build green. Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
llm_api_key_envfrom config, hardcode readingLLM_API_KEYenv varLLM_API_KEYin.env, no config field neededOPENAI_API_KEY,ANTHROPIC_API_KEY, etc.) still work via LiteLLM auto-detectionokb inithas one less interactive promptTest plan
test_config.py6/6)LLM_API_KEYworks with OpenAI, Anthropic, and GeminiLLM_API_KEY