Skip to content

feat: compile pipeline, query agent, and multimodal improvements - #10

Merged
rejojer merged 12 commits into
devfrom
bugfix/compile-clean
Apr 10, 2026
Merged

feat: compile pipeline, query agent, and multimodal improvements#10
rejojer merged 12 commits into
devfrom
bugfix/compile-clean

Conversation

@rejojer

@rejojer rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member

Summary

  • Concept dedup & compile refactor: concept plan/update/related paths, bidirectional backlinks, shared _compile_concepts
  • Brief system & unified query: per-page JSON sources via pymupdf, doc_type + full_text frontmatter, get_page_content tool
  • Config & environment: default model gpt-5.4-mini, API key warning, warning suppression, test isolation
  • CLI polish: improved init prompts with explicit defaults, American English, output formatting
  • Multimodal query: get_image tool with ToolOutputImage for viewing figures/charts in source documents
  • Image path unification: all image paths use sources/images/ prefix, pymupdf replaces PageIndex for page content extraction

Test plan

  • All 184 tests pass
  • openkb init shows correct prompts with defaults
  • openkb add short doc: clean frontmatter, images at sources/images/
  • openkb add long doc: per-page JSON from pymupdf, correct image paths
  • openkb query on short doc: reads source via read_file
  • openkb query on long doc: uses get_page_content with targeted pages
  • openkb query about figures: calls get_image tool
KylinMountain and others added 6 commits April 10, 2026 08:04
…klinks

- Add concept dedup with briefs and _read_concept_briefs context
- Add concepts plan and update prompt templates with create/update/related paths
- Extract shared _compile_concepts from compile_short_doc and compile_long_doc
- Add bidirectional backlinks between summaries and concepts
- Code review fixes: security, robustness, tests, and CI hardening

Co-authored-by: Ray <mailtangyu@gmail.com>
- Add get_page_content tool and parse_pages helper for page-level access
- Store long doc sources as per-page JSON extracted by pymupdf
- Unify summary frontmatter to doc_type + full_text fields
- Update schema and tree renderer for new frontmatter format
- All image paths use sources/images/ prefix relative to wiki root

Co-authored-by: Ray <mailtangyu@gmail.com>
- Change default model to gpt-5.4-mini
- Warn when no LLM API key found instead of failing silently
- Fix CI publish workflow and test isolation

Co-authored-by: Ray <mailtangyu@gmail.com>
- Move warning suppression after imports to avoid markitdown override
- Improve init prompts with explicit defaults
- Use American English throughout (initialized, normalized, Synthesize)
- Replace unicode ellipsis with ASCII
- Remove empty explorations/reports dirs from init
- Fix test isolation for _find_kb_dir
- Add get_image tool for viewing images referenced in source documents
- Use ToolOutputImage for proper image content in LLM context
- Update prompt: use full_text field, restrict get_page_content to pageindex
- Add self-talk before tool calls, enforce concise answers
- Prevent duplicate frontmatter in LLM-generated content via schema update
- Add convert_pdf_to_pages for per-page content+image extraction
- All image paths use sources/images/ prefix relative to wiki root
- Remove page marker comments from short doc source markdown
@rejojer
rejojer force-pushed the bugfix/compile-clean branch from 9f652d0 to 44bf83e Compare April 10, 2026 00:09
@rejojer

rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. _write_concept appends LLM-rewritten content instead of replacing the body during concept updates, causing content duplication. The _CONCEPT_UPDATE_USER prompt (line 106) instructs the LLM to "Rewrite the full page incorporating the new information naturally -- do not just append" and return "The rewritten full concept page." However, _write_concept appends the complete rewrite to the existing body (existing += f"\n\n{clean}" at line 341) rather than replacing the body with it. Every concept update with a new source document will duplicate all existing content.

if is_update and path.exists():
existing = path.read_text(encoding="utf-8")
if source_file not in existing:
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
fm = existing[:end + 3]
body = existing[end + 3:]
if "sources:" in fm:
fm = fm.replace("sources: [", f"sources: [{source_file}, ")
else:
fm = fm.replace("---\n", f"---\nsources: [{source_file}]\n", 1)
existing = fm + body
else:
existing = f"---\nsources: [{source_file}]\n---\n\n" + existing
# Strip frontmatter from LLM content to avoid duplicate blocks
clean = content
if clean.startswith("---"):
end = clean.find("---", 3)
if end != -1:
clean = clean[end + 3:].lstrip("\n")
existing += f"\n\n{clean}"
if brief and existing.startswith("---"):

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

rejojer added 5 commits April 10, 2026 08:33
The _CONCEPT_UPDATE_USER prompt asks the LLM for a full rewrite, but
_write_concept was appending the rewrite to the existing body, causing
content duplication on every concept update.
Replace hand-rolled fence stripping with json_repair to handle
malformed JSON, missing fences, and prose-wrapped responses from LLMs.
Also fixes str.index() ValueError on fenced blocks without newlines.
@rejojer

rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. _write_concept silently discards LLM-generated content on re-compilation of the same document. When is_update=True and the concept page already exists, the body-replacement logic (lines 339-353) is entirely nested inside if source_file not in existing:. On re-compile, source_file is already present in the frontmatter, so this condition is false and the LLM rewrite is silently dropped -- the API call is made and paid for, but the result is never written. Only the brief field gets updated (lines 354-363, outside the gate). The source_file not in existing check should only guard the "add source to frontmatter" block, not the body replacement.

if is_update and path.exists():
existing = path.read_text(encoding="utf-8")
if source_file not in existing:
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
fm = existing[:end + 3]
body = existing[end + 3:]
if "sources:" in fm:
fm = fm.replace("sources: [", f"sources: [{source_file}, ")
else:
fm = fm.replace("---\n", f"---\nsources: [{source_file}]\n", 1)
existing = fm + body
else:
existing = f"---\nsources: [{source_file}]\n---\n\n" + existing
# Strip frontmatter from LLM content to avoid duplicate blocks
clean = content
if clean.startswith("---"):
end = clean.find("---", 3)
if end != -1:
clean = clean[end + 3:].lstrip("\n")
# Replace body with LLM rewrite (prompt asks for full rewrite, not delta)
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
existing = existing[:end + 3] + "\n\n" + clean
else:
existing = clean
else:
existing = clean
if brief and existing.startswith("---"):

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@rejojer

rejojer commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. _write_concept silently discards LLM-generated concept rewrites on re-compilation. When is_update=True and source_file already appears in the existing concept page (in sources: [...] frontmatter), the source_file not in existing guard at line 326 causes the entire body-replacement block (lines 327-353) to be skipped. Only the brief frontmatter field is updated; the full page rewrite from _CONCEPT_UPDATE_USER is thrown away. This means re-adding a modified document leaves concept pages stale. (bug: body replacement wrongly gated inside source-dedup check)

if is_update and path.exists():
existing = path.read_text(encoding="utf-8")
if source_file not in existing:
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
fm = existing[:end + 3]
body = existing[end + 3:]
if "sources:" in fm:
fm = fm.replace("sources: [", f"sources: [{source_file}, ")
else:
fm = fm.replace("---\n", f"---\nsources: [{source_file}]\n", 1)
existing = fm + body
else:
existing = f"---\nsources: [{source_file}]\n---\n\n" + existing
# Strip frontmatter from LLM content to avoid duplicate blocks
clean = content
if clean.startswith("---"):
end = clean.find("---", 3)
if end != -1:
clean = clean[end + 3:].lstrip("\n")
# Replace body with LLM rewrite (prompt asks for full rewrite, not delta)
if existing.startswith("---"):
end = existing.find("---", 3)
if end != -1:
existing = existing[:end + 3] + "\n\n" + clean
else:
existing = clean
else:
existing = clean
if brief and existing.startswith("---"):

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@rejojer
rejojer merged commit 85eaebf into dev Apr 10, 2026
@rejojer rejojer mentioned this pull request Apr 10, 2026
4 tasks
rejojer added a commit that referenced this pull request Apr 11, 2026
feat: compile pipeline, query agent, and multimodal improvements
@rejojer
rejojer deleted the bugfix/compile-clean branch April 11, 2026 17:45
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants