Skip to content

feat(cli): add migrate-images for KBs ingested before note-relative links#182

Open
Aldominguez12 wants to merge 2 commits into
VectifyAI:mainfrom
Aldominguez12:feat/migrate-image-links
Open

feat(cli): add migrate-images for KBs ingested before note-relative links#182
Aldominguez12 wants to merge 2 commits into
VectifyAI:mainfrom
Aldominguez12:feat/migrate-image-links

Conversation

@Aldominguez12

Copy link
Copy Markdown
Contributor

Follow-up to #181, as offered there and welcomed in the review.

Problem

KBs ingested before #181 keep wiki-root-relative sources/images/... links in their wiki/sources/*.md pages. The read_wiki_image fallback keeps agent flows working, but renderers (Obsidian, GitHub, VS Code) still resolve those links against the containing file and show every figure broken.

Fix

New one-time migration command:

openkb migrate-images            # rewrite old links in place
openkb migrate-images --dry-run  # preview files and link counts
  • Rewrites the ](sources/images/ prefix to ](images/ — the form md_image_ref() emits at ingest since fix(images): write note-relative image links in sources pages #181. Only the prefix is swapped, so path tails never need parsing and image embeds / plain links are handled alike.
  • Scope matches the writers fix(images): write note-relative image links in sources pages #181 changed: only .md files directly under wiki/sources/. Long-doc per-page JSON intentionally keeps wiki-root-relative paths (internal metadata), and pages outside sources/ never carried the old prefix.
  • Idempotent (already-migrated links don't match), takes the ingest lock, reports per file, and appends to wiki/log.md — same conventions as lint --fix.
  • Logic lives in a new openkb/migrate.py so the command doesn't drag in pymupdf via images.py.

Verification

  • New tests/test_migrate.py: 11 tests covering rewrite/alt-preservation, idempotency, dry-run, JSON and non-sources pages left untouched, sorted multi-file output, and the CLI surface (no KB, nothing to migrate, apply + log, dry-run).
  • ruff check/format and mypy clean on the touched files.
  • End-to-end against two real pre-fix(images): write note-relative image links in sources pages #181 KBs (32 sources pages, 2,061 old links): the command's output is byte-identical to a previously hand-verified migration of the same KBs, and a second run is a no-op.

🤖 Generated with Claude Code

…inks

Follow-up to VectifyAI#181: already-ingested KBs keep wiki-root-relative
sources/images/... links in their wiki/sources/*.md pages, which
render broken in Obsidian, GitHub, and VS Code.

`openkb migrate-images` rewrites them to the note-relative images/...
form now emitted at ingest (`--dry-run` to preview). Scope matches
the writers VectifyAI#181 changed: only .md files directly under wiki/sources/
— long-doc JSON metadata intentionally keeps wiki-root-relative paths,
and pages outside sources/ never carried the old prefix. Idempotent,
takes the ingest lock, and logs to wiki/log.md.

Verified against two real pre-VectifyAI#181 KBs (32 files, 2,061 links): output
is byte-identical to a hand-verified migration, and a second run is a
no-op.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c11d107dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread openkb/cli.py Outdated
verb = "Would rewrite" if dry_run else "Rewrote"
click.echo(f"{verb} {total} image link(s) across {len(changed)} file(s).")
if not dry_run:
append_log(wiki, "migrate-images", f"{total} link(s) across {len(changed)} file(s)")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep migration log writes under the KB lock

When openkb migrate-images runs concurrently with another mutating command, this append_log executes after the exclusive kb_ingest_lock block has already been released. Since append_log writes wiki/log.md, this leaves a wiki write outside the repo's locking invariant from AGENTS.md and can interleave with another process's locked log update; keep the log append inside the same exclusive lock as the file rewrites.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 29da160. The append_log call now runs inside the same kb_ingest_lock block as the file rewrites, matching the pattern used by the lint command. tests/test_migrate.py still passes (11/11).

Codex review flagged that append_log ran after kb_ingest_lock was
released, leaving a wiki/log.md write outside the locking invariant.
Move it inside the exclusive block, matching the lint command.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant