Skip to content

Commit f201aca

Browse files
KylinMountainXinyan Zhou
andauthored
fix: collision-resistant doc_name — same-stem documents no longer overwrite each other (#96)
* feat(state): index registry entries by path for doc identity * feat(state): match legacy registry entries by stem for path backfill * docs(state): document first-match-wins + truthy-path semantics in find_legacy_by_stem * feat(converter): portable registry path key * feat(converter): collision-resistant doc_name resolution (Scheme A) * fix(converter,state): NFKC-normalize both sides of name comparisons + edge tests * feat(converter): name all artifacts by collision-resistant doc_name * fix(converter): registry-authoritative names; read-only dedup skip Unclaimed on-disk artifacts no longer force a suffix (fixes doc_name drift on retry after a failed compile), and the dedup early-return now derives the name from the stored entry without invoking the resolver (no legacy path backfill from duplicate copies). * feat(indexer): accept explicit doc_name for long-doc artifacts * feat(cli): persist path identity metadata on add * fix(cli): construct registry after convert so legacy backfills aren't clobbered * fix(lint): resolve raw files through the registry before stem matching * fix(cli): remove locates raw copies via recorded raw_path * fix(cli): restrict raw-name fallback to legacy entries without raw_path * credit: collision-resistant doc_name groundwork The identity model (path-keyed registry metadata, collision-resistant naming) builds on the approach pioneered in PR #30. Co-authored-by: Xinyan Zhou <xinyanzhou938@gmail.com> --------- Co-authored-by: Xinyan Zhou <xinyanzhou938@gmail.com>
1 parent 25c3ffd commit f201aca

11 files changed

Lines changed: 791 additions & 28 deletions

File tree

openkb/cli.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def filter(self, record: logging.LogRecord) -> bool:
4141
from dotenv import load_dotenv
4242

4343
from openkb.config import DEFAULT_CONFIG, load_config, save_config, load_global_config, register_kb
44-
from openkb.converter import convert_document
44+
from openkb.converter import _registry_path, convert_document
4545
from openkb.log import append_log
4646
from openkb.schema import AGENTS_MD, INDEX_SEED, PAGE_CONTENT_DIRS
4747

@@ -284,7 +284,6 @@ def add_single_file(file_path: Path, kb_dir: Path) -> Literal["added", "skipped"
284284
config = load_config(openkb_dir / "config.yaml")
285285
_setup_llm_key(kb_dir)
286286
model: str = config.get("model", DEFAULT_CONFIG["model"])
287-
registry = HashRegistry(openkb_dir / "hashes.json")
288287

289288
# 2. Convert document
290289
click.echo(f"Adding: {file_path.name}")
@@ -299,15 +298,15 @@ def add_single_file(file_path: Path, kb_dir: Path) -> Literal["added", "skipped"
299298
click.echo(f" [SKIP] Already in knowledge base: {file_path.name}")
300299
return "skipped"
301300

302-
doc_name = file_path.stem
301+
doc_name = result.doc_name or file_path.stem
303302
index_result = None # populated only on the long-doc branch
304303

305304
# 3/4. Index and compile
306305
if result.is_long_doc:
307306
click.echo(f" Long document detected — indexing with PageIndex...")
308307
try:
309308
from openkb.indexer import index_long_document
310-
index_result = index_long_document(result.raw_path, kb_dir)
309+
index_result = index_long_document(result.raw_path, kb_dir, doc_name=doc_name)
311310
except Exception as exc:
312311
click.echo(f" [ERROR] Indexing failed: {exc}")
313312
logger.debug("Indexing traceback:", exc_info=True)
@@ -347,17 +346,31 @@ def add_single_file(file_path: Path, kb_dir: Path) -> Literal["added", "skipped"
347346

348347
# Register hash only after successful compilation
349348
if result.file_hash:
349+
# Construct the registry NOW, not earlier: convert_document may have
350+
# backfilled a legacy entry (doc_name/path) on disk via its own
351+
# instance, and an earlier snapshot would clobber that backfill on
352+
# the full rewrite in add().
353+
registry = HashRegistry(openkb_dir / "hashes.json")
350354
doc_type = "long_pdf" if result.is_long_doc else file_path.suffix.lstrip(".")
351355
meta = {
352356
"name": file_path.name,
353357
"doc_name": doc_name,
354358
"type": doc_type,
359+
"path": _registry_path(file_path, kb_dir),
355360
}
361+
if result.raw_path is not None:
362+
meta["raw_path"] = _registry_path(result.raw_path, kb_dir)
363+
if result.source_path is not None:
364+
meta["source_path"] = _registry_path(result.source_path, kb_dir)
356365
# For long PDFs we also persist the PageIndex doc_id so `openkb
357366
# remove` can later call ``Collection.delete_document(doc_id)``
358367
# to free the managed PDF copy + SQLite row.
359368
if index_result is not None:
360369
meta["doc_id"] = index_result.doc_id
370+
# An edited document arrives with a new content hash; drop the
371+
# stale entry for the same doc_name so the registry keeps exactly
372+
# one entry per document.
373+
registry.remove_by_doc_name(doc_name)
361374
registry.add(result.file_hash, meta)
362375

363376
append_log(kb_dir / "wiki", "ingest", file_path.name)
@@ -910,7 +923,15 @@ def remove(ctx, identifier, keep_raw, keep_empty, dry_run, yes):
910923
raw_path = None
911924
if not keep_raw:
912925
raw_dir = kb_dir / "raw"
913-
candidate = raw_dir / name
926+
# Raw copies are named by doc_name since the collision fix: use the
927+
# recorded raw_path when present. Only pre-upgrade entries (no
928+
# raw_path field) fall back to the original filename — a recorded
929+
# path that no longer exists must NOT fall through, or it could
930+
# delete a same-named raw file belonging to another document.
931+
if meta.get("raw_path"):
932+
candidate = kb_dir / meta["raw_path"]
933+
else:
934+
candidate = raw_dir / name
914935
if candidate.exists():
915936
raw_path = candidate
916937
actions.append(("DELETE", str(candidate.relative_to(kb_dir))))

openkb/converter.py

Lines changed: 104 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
"""Document conversion pipeline for OpenKB."""
22
from __future__ import annotations
33

4+
import hashlib
45
import logging
6+
import re
57
import shutil
8+
import unicodedata
69
from dataclasses import dataclass
710
from pathlib import Path
811

@@ -25,6 +28,81 @@ class ConvertResult:
2528
is_long_doc: bool = False
2629
skipped: bool = False
2730
file_hash: str | None = None # For deferred hash registration
31+
doc_name: str | None = None # Stable wiki name (collision-resistant)
32+
33+
34+
def _registry_path(path: Path, kb_dir: Path) -> str:
35+
"""Portable path string used as the registry's identity key.
36+
37+
Relative-to-KB posix when the file lives inside the KB (stable across
38+
machines/checkouts), absolute posix otherwise. Both paths are fully
39+
resolved (symlinks followed) before comparison.
40+
"""
41+
resolved_path = path.resolve()
42+
resolved_kb = kb_dir.resolve()
43+
if resolved_path.is_relative_to(resolved_kb):
44+
return resolved_path.relative_to(resolved_kb).as_posix()
45+
return resolved_path.as_posix()
46+
47+
48+
_SAFE_STEM_RE = re.compile(r"[^\w\-]+")
49+
_SUFFIX_LEN = 8
50+
51+
52+
def _sanitize_stem(stem: str) -> str:
53+
normalized = unicodedata.normalize("NFKC", stem)
54+
return _SAFE_STEM_RE.sub("-", normalized).strip("-") or "document"
55+
56+
57+
def _name_taken(candidate: str, registry: HashRegistry) -> bool:
58+
"""True when ``candidate`` is claimed by another registered document.
59+
60+
The registry is the single authority on ownership: artifacts on disk
61+
without a registry entry are either leftovers of a failed ingest of
62+
this same source (must be adoptable so a retry keeps its clean name)
63+
or out-of-contract manual drops — both are overwritten, matching
64+
pre-collision-fix behaviour for unclaimed files.
65+
"""
66+
for meta in registry.all_entries().values():
67+
entry_name = meta.get("doc_name") or Path(meta.get("name", "")).stem
68+
if unicodedata.normalize("NFKC", entry_name) == candidate:
69+
return True
70+
return False
71+
72+
73+
def resolve_doc_name(src: Path, kb_dir: Path, registry: HashRegistry) -> str:
74+
"""Resolve the stable wiki name for ``src`` (Scheme A).
75+
76+
Identity is keyed by path: a source we've seen before (same path, even
77+
with new content) keeps its name so re-ingest overwrites in place.
78+
Legacy registry entries (written before the path index) are matched by
79+
stem and backfilled with the path. A brand-new source keeps the clean
80+
sanitized stem unless another document already owns that name, in which
81+
case it gets a deterministic ``-{sha256(path)[:8]}`` suffix.
82+
"""
83+
path_key = _registry_path(src, kb_dir)
84+
85+
known = registry.get_by_path(path_key)
86+
if known is not None:
87+
stored = known.get("doc_name") or Path(known.get("name", "")).stem
88+
if stored:
89+
return stored
90+
91+
legacy = registry.find_legacy_by_stem(src.stem)
92+
if legacy is not None:
93+
file_hash, meta = legacy
94+
meta = dict(meta)
95+
name = meta.get("doc_name") or Path(meta.get("name", "")).stem
96+
meta["doc_name"] = name
97+
meta["path"] = path_key
98+
registry.add(file_hash, meta) # backfill + persist
99+
return name
100+
101+
candidate = _sanitize_stem(src.stem)
102+
if _name_taken(candidate, registry):
103+
digest = hashlib.sha256(path_key.encode("utf-8")).hexdigest()[:_SUFFIX_LEN]
104+
return f"{candidate}-{digest}"
105+
return candidate
28106

29107

30108
def get_pdf_page_count(path: Path) -> int:
@@ -53,20 +131,29 @@ def convert_document(src: Path, kb_dir: Path) -> ConvertResult:
53131
registry = HashRegistry(openkb_dir / "hashes.json")
54132

55133
# ------------------------------------------------------------------
56-
# 1. Hash check
134+
# 1. Hash check + identity resolution
57135
# ------------------------------------------------------------------
58136
file_hash = HashRegistry.hash_file(src)
59137
if registry.is_known(file_hash):
60138
logger.info("Skipping already-known file: %s", src.name)
61-
return ConvertResult(skipped=True)
139+
stored = registry.get(file_hash) or {}
140+
return ConvertResult(
141+
skipped=True,
142+
file_hash=file_hash,
143+
doc_name=stored.get("doc_name") or Path(stored.get("name", src.name)).stem,
144+
)
145+
doc_name = resolve_doc_name(src, kb_dir, registry)
62146

63147
# ------------------------------------------------------------------
64148
# 2. Copy to raw/
65149
# ------------------------------------------------------------------
66150
raw_dir = kb_dir / "raw"
67151
raw_dir.mkdir(parents=True, exist_ok=True)
68-
raw_dest = raw_dir / src.name
69-
if raw_dest.resolve() != src.resolve():
152+
if src.resolve().is_relative_to(raw_dir.resolve()):
153+
# Watch mode: the file already lives in raw/ — don't copy/rename.
154+
raw_dest = src
155+
else:
156+
raw_dest = raw_dir / f"{doc_name}{src.suffix.lower()}"
70157
shutil.copy2(src, raw_dest)
71158

72159
# ------------------------------------------------------------------
@@ -81,18 +168,21 @@ def convert_document(src: Path, kb_dir: Path) -> ConvertResult:
81168
threshold,
82169
src.name,
83170
)
84-
return ConvertResult(raw_path=raw_dest, is_long_doc=True, file_hash=file_hash)
171+
return ConvertResult(
172+
raw_path=raw_dest,
173+
is_long_doc=True,
174+
file_hash=file_hash,
175+
doc_name=doc_name,
176+
)
85177

86178
# ------------------------------------------------------------------
87179
# 4/5. Convert to Markdown
88180
# ------------------------------------------------------------------
89181
sources_dir = kb_dir / "wiki" / "sources"
90182
sources_dir.mkdir(parents=True, exist_ok=True)
91-
images_dir = kb_dir / "wiki" / "sources" / "images" / src.stem
183+
images_dir = kb_dir / "wiki" / "sources" / "images" / doc_name
92184
images_dir.mkdir(parents=True, exist_ok=True)
93185

94-
doc_name = src.stem
95-
96186
if src.suffix.lower() == ".md":
97187
markdown = src.read_text(encoding="utf-8")
98188
markdown = copy_relative_images(markdown, src.parent, doc_name, images_dir)
@@ -109,4 +199,9 @@ def convert_document(src: Path, kb_dir: Path) -> ConvertResult:
109199
dest_md = sources_dir / f"{doc_name}.md"
110200
dest_md.write_text(markdown, encoding="utf-8")
111201

112-
return ConvertResult(raw_path=raw_dest, source_path=dest_md, file_hash=file_hash)
202+
return ConvertResult(
203+
raw_path=raw_dest,
204+
source_path=dest_md,
205+
file_hash=file_hash,
206+
doc_name=doc_name,
207+
)

openkb/indexer.py

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,15 @@ def _convert_pdf_to_pages(pdf_path: Path, doc_name: str, images_dir: Path) -> li
8686
return convert_pdf_to_pages(pdf_path, doc_name, images_dir)
8787

8888

89-
def index_long_document(pdf_path: Path, kb_dir: Path) -> IndexResult:
90-
"""Index a long PDF document using PageIndex and write wiki pages."""
89+
def index_long_document(
90+
pdf_path: Path, kb_dir: Path, doc_name: str | None = None
91+
) -> IndexResult:
92+
"""Index a long PDF document using PageIndex and write wiki pages.
93+
94+
``doc_name`` is the collision-resistant wiki name used for all written
95+
artifacts; defaults to the PDF's stem for backward compatibility.
96+
"""
97+
source_name = doc_name or pdf_path.stem
9198
openkb_dir = kb_dir / ".openkb"
9299
config = load_config(openkb_dir / "config.yaml")
93100

@@ -123,7 +130,7 @@ def index_long_document(pdf_path: Path, kb_dir: Path) -> IndexResult:
123130

124131
# Fetch complete document (metadata + structure + text)
125132
doc = col.get_document(doc_id, include_text=True)
126-
doc_name: str = doc.get("doc_name", pdf_path.stem)
133+
indexed_doc_name: str = doc.get("doc_name", pdf_path.stem)
127134
description: str = doc.get("doc_description", "")
128135
structure: list = doc.get("structure", [])
129136

@@ -132,15 +139,15 @@ def index_long_document(pdf_path: Path, kb_dir: Path) -> IndexResult:
132139
logger.info("page_count from doc: %s", doc.get("page_count", "NOT PRESENT"))
133140

134141
tree = {
135-
"doc_name": doc_name,
142+
"doc_name": indexed_doc_name,
136143
"doc_description": description,
137144
"structure": structure,
138145
}
139146

140147
# Write wiki/sources/ — per-page content
141148
sources_dir = kb_dir / "wiki" / "sources"
142149
sources_dir.mkdir(parents=True, exist_ok=True)
143-
images_dir = sources_dir / "images" / pdf_path.stem
150+
images_dir = sources_dir / "images" / source_name
144151

145152
all_pages: list[dict[str, Any]] = []
146153
if pageindex_api_key:
@@ -155,19 +162,19 @@ def index_long_document(pdf_path: Path, kb_dir: Path) -> IndexResult:
155162
if not all_pages:
156163
if pageindex_api_key:
157164
logger.warning("Cloud returned no pages for %s; falling back to local pymupdf", pdf_path.name)
158-
all_pages = _normalize_page_content(_convert_pdf_to_pages(pdf_path, pdf_path.stem, images_dir))
165+
all_pages = _normalize_page_content(_convert_pdf_to_pages(pdf_path, source_name, images_dir))
159166

160167
if not all_pages:
161168
raise RuntimeError(f"No page content extracted for {pdf_path.name}")
162169

163-
(sources_dir / f"{pdf_path.stem}.json").write_text(
170+
(sources_dir / f"{source_name}.json").write_text(
164171
json_mod.dumps(all_pages, ensure_ascii=False, indent=2), encoding="utf-8",
165172
)
166173

167174
# Write wiki/summaries/ (no images, just summaries)
168175
summaries_dir = kb_dir / "wiki" / "summaries"
169176
summaries_dir.mkdir(parents=True, exist_ok=True)
170-
summary_md = render_summary_md(tree, pdf_path.stem, doc_id)
171-
(summaries_dir / f"{pdf_path.stem}.md").write_text(summary_md, encoding="utf-8")
177+
summary_md = render_summary_md(tree, source_name, doc_id)
178+
(summaries_dir / f"{source_name}.md").write_text(summary_md, encoding="utf-8")
172179

173180
return IndexResult(doc_id=doc_id, description=description, tree=tree)

openkb/lint.py

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,15 +336,25 @@ def find_orphans(wiki: Path) -> list[str]:
336336
return sorted(orphans)
337337

338338

339-
def find_missing_entries(raw: Path, wiki: Path) -> list[str]:
339+
def find_missing_entries(
340+
raw: Path, wiki: Path, *, kb_dir: Path | None = None,
341+
) -> list[str]:
340342
"""Find files in raw/ that have no corresponding wiki entries.
341343
342-
A file is considered "present" if it has either a sources/ or summaries/
343-
page with the same stem.
344+
When ``kb_dir`` is provided, each raw file is first resolved through
345+
the hash registry (``kb_dir/.openkb/hashes.json``): registered files
346+
are checked against their ``doc_name`` artifacts, which can differ
347+
from the raw stem (watch-mode/URL ingests keep the original filename
348+
in raw/ — e.g. ``2509.11420.pdf`` → ``2509-11420.md`` — and collision
349+
suffixes rename artifacts too). Files with no registry entry fall back
350+
to the stem heuristic: "present" means a sources/ or summaries/ page
351+
with the same stem.
344352
345353
Args:
346354
raw: Path to the raw documents directory.
347355
wiki: Path to the wiki root directory.
356+
kb_dir: Root of the knowledge base. When None, only the legacy
357+
stem heuristic is used.
348358
349359
Returns:
350360
List of filenames in raw/ with no wiki entry.
@@ -356,10 +366,39 @@ def find_missing_entries(raw: Path, wiki: Path) -> list[str]:
356366
summary_stems = {p.stem for p in summaries_dir.glob("*.md")} if summaries_dir.exists() else set()
357367
known_stems = sources_stems | summary_stems
358368

369+
registry = None
370+
if kb_dir is not None:
371+
registry_file = kb_dir / ".openkb" / "hashes.json"
372+
if registry_file.exists():
373+
# Deferred import: converter pulls in pymupdf/markitdown,
374+
# which lint doesn't otherwise need at import time.
375+
from openkb.converter import _registry_path
376+
from openkb.state import HashRegistry
377+
378+
registry = HashRegistry(registry_file)
379+
359380
missing: list[str] = []
360381
if raw.exists():
361382
for f in raw.iterdir():
362-
if f.is_file() and f.stem not in known_stems:
383+
if not f.is_file():
384+
continue
385+
if registry is not None:
386+
meta = registry.get_by_path(_registry_path(f, kb_dir))
387+
if meta is not None:
388+
# Registered file — the registry's doc_name is the
389+
# single source of truth for artifact names.
390+
doc_name = meta.get("doc_name") or Path(
391+
meta.get("name", f.name)
392+
).stem
393+
present = (
394+
(sources_dir / f"{doc_name}.md").exists()
395+
or (sources_dir / f"{doc_name}.json").exists()
396+
or (summaries_dir / f"{doc_name}.md").exists()
397+
)
398+
if not present:
399+
missing.append(f.name)
400+
continue
401+
if f.stem not in known_stems:
363402
missing.append(f.name)
364403

365404
return sorted(missing)
@@ -460,7 +499,7 @@ def run_structural_lint(kb_dir: Path) -> str:
460499

461500
broken = find_broken_links(wiki)
462501
orphans = find_orphans(wiki)
463-
missing = find_missing_entries(raw, wiki)
502+
missing = find_missing_entries(raw, wiki, kb_dir=kb_dir)
464503
sync_issues = check_index_sync(wiki)
465504
bad_frontmatter = find_invalid_frontmatter(wiki)
466505

0 commit comments

Comments
 (0)