feat(import): add conversation-export import command#442
Open
dale053 wants to merge 2 commits into
Open
Conversation
seed prior agent history into the review queue without bypassing approval. this adds format-specific importers with dry-run, proposal caps, and dedup against approved claims. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesConversation-export import feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as import_cmd
participant Importer as run_import
participant Store as KBStore
participant Proposals as proposals.propose_claim/propose_page
User->>CLI: vouch import <format> <path> [--dry-run] [--max-proposals N]
CLI->>Importer: run_import(store, format, path, dry_run, max_proposals, actor)
Importer->>Importer: load_candidates(format, path)
Importer->>Store: _register_file_source() (chat-json/memory-export)
Importer->>Importer: dedup claims/pages
Importer->>Proposals: propose_claim/propose_page per candidate
Proposals-->>Importer: proposal_id
Importer-->>CLI: ImportResult(counts, proposal_ids, warnings, cap_hit)
CLI-->>User: JSON or text summary
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/test_import.py (1)
177-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider broader CLI test coverage.
The CLI integration test only covers the JSON happy path. Adding tests for text output mode,
CorpusImportError→ClickExceptionmapping, and--dry-run/--max-proposalsthrough the CLI would verify the command's full surface and error handling paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_import.py` around lines 177 - 192, The CLI import test currently only validates the JSON success path in test_cli_import_json_output, so expand coverage for the import command’s other behaviors. Add tests around the cli invoke flow for plain text output, error mapping from CorpusImportError to ClickException, and option handling for --dry-run and --max-proposals, using the existing cli, CliRunner, and import/chat-json command path as the entry points.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/cli.py`:
- Around line 2053-2058: The --max-proposals option in the click option
definition currently accepts 0 and negative integers because it uses plain
type=int with default=None. Update the CLI validation in the option decorator
for the run_import-related command to use a positive integer constraint such as
click.IntRange(min=1), while keeping None as the default so the cap remains
optional. This change should be made in the click.option declaration for
--max-proposals so invalid values are rejected before run_import is called.
In `@src/vouch/corpus_import.py`:
- Around line 333-335: Dry-run still mutates state because
`_register_file_source()` calls `store.put_source()` before proposals are
skipped. Update the import flow in `corpus_import.py` so the `json_source_id`
registration happens only when `dry_run` is false, or make
`_register_file_source()` a no-op for dry runs. Apply the same guard in both the
chat-json/memory-export path and the other duplicate call site, using the
existing `dry_run` flag and `_register_file_source` helper to keep imports fully
non-mutating.
- Line 339: The proposal cap handling in corpus_import should not treat negative
max_proposals values as unlimited; update the logic around the cap assignment in
the import path so that only None means no cap and any negative value is
explicitly rejected or clamped as invalid. Use the existing corpus_import
proposal limit flow and the cap/max_proposals check to enforce validation before
continuing, so callers cannot bypass flood protection by passing -1.
- Around line 378-388: The page proposal path in propose_pages currently only
attaches source_ids from candidate.source_path, so JSON-backed chat imports lose
their registered source. Update the candidate handling in propose_pages to
preserve and pass through the JSON source identifier for generated pages as
well, reusing the existing _register_file_source and source_ids flow so both
markdown and chat-json candidates attach sources consistently.
- Around line 102-106: The markdown/frontmatter parsing path in corpus_import
should convert raw read/decode/YAML failures into CorpusImportError so the CLI
sees a consistent import error contract. Update the logic around the
path.read_text, _FRONTMATTER_RE.match, and yaml.safe_load flow to catch
unreadable file, decoding, and invalid frontmatter exceptions, then re-raise
them as CorpusImportError with the original exception chained.
---
Nitpick comments:
In `@tests/test_import.py`:
- Around line 177-192: The CLI import test currently only validates the JSON
success path in test_cli_import_json_output, so expand coverage for the import
command’s other behaviors. Add tests around the cli invoke flow for plain text
output, error mapping from CorpusImportError to ClickException, and option
handling for --dry-run and --max-proposals, using the existing cli, CliRunner,
and import/chat-json command path as the entry points.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 366d2a7b-7fad-4ebb-aa49-2740d5242ebd
📒 Files selected for processing (4)
CHANGELOG.mdsrc/vouch/cli.pysrc/vouch/corpus_import.pytests/test_import.py
Comment on lines
+2053
to
+2058
| @click.option( | ||
| "--max-proposals", | ||
| type=int, | ||
| default=None, | ||
| help="Cap proposals filed in one run (claims + pages combined).", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate --max-proposals against non-positive values.
type=int with default=None accepts 0 and negative values, which could cause confusing behavior depending on how run_import handles them. Using click.IntRange(min=1) rejects invalid values at the CLI layer with a clear error message, while still allowing None (no cap) as the default.
🛡️ Proposed fix
`@click.option`(
"--max-proposals",
- type=int,
+ type=click.IntRange(min=1),
default=None,
help="Cap proposals filed in one run (claims + pages combined).",
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @click.option( | |
| "--max-proposals", | |
| type=int, | |
| default=None, | |
| help="Cap proposals filed in one run (claims + pages combined).", | |
| ) | |
| `@click.option`( | |
| "--max-proposals", | |
| type=click.IntRange(min=1), | |
| default=None, | |
| help="Cap proposals filed in one run (claims + pages combined).", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/cli.py` around lines 2053 - 2058, The --max-proposals option in the
click option definition currently accepts 0 and negative integers because it
uses plain type=int with default=None. Update the CLI validation in the option
decorator for the run_import-related command to use a positive integer
constraint such as click.IntRange(min=1), while keeping None as the default so
the cap remains optional. This change should be made in the click.option
declaration for --max-proposals so invalid values are rejected before run_import
is called.
Comment on lines
+102
to
+106
| text = path.read_text(encoding="utf-8").replace("\r\n", "\n") | ||
| m = _FRONTMATTER_RE.match(text) | ||
| if m: | ||
| meta = yaml.safe_load(m.group(1)) or {} | ||
| if not isinstance(meta, dict): |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wrap markdown parse failures in CorpusImportError.
Invalid frontmatter YAML, unreadable files, or decode failures currently escape as raw exceptions, bypassing the import error contract used by the CLI.
Suggested fix
def parse_markdown_file(path: Path) -> tuple[str, str, dict[str, Any]]:
"""Return ``(title, body, frontmatter)`` for one markdown file."""
- text = path.read_text(encoding="utf-8").replace("\r\n", "\n")
- m = _FRONTMATTER_RE.match(text)
- if m:
- meta = yaml.safe_load(m.group(1)) or {}
+ try:
+ text = path.read_text(encoding="utf-8").replace("\r\n", "\n")
+ m = _FRONTMATTER_RE.match(text)
+ if not m:
+ return path.stem, text, {}
+ meta = yaml.safe_load(m.group(1)) or {}
+ except (OSError, UnicodeDecodeError, yaml.YAMLError) as e:
+ raise CorpusImportError(f"cannot parse markdown file at {path}: {e}") from e
+ if m:
if not isinstance(meta, dict):
meta = {}
body = m.group(2)
title = str(meta.get("title") or path.stem)
return title, body, meta
- return path.stem, text, {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| text = path.read_text(encoding="utf-8").replace("\r\n", "\n") | |
| m = _FRONTMATTER_RE.match(text) | |
| if m: | |
| meta = yaml.safe_load(m.group(1)) or {} | |
| if not isinstance(meta, dict): | |
| try: | |
| text = path.read_text(encoding="utf-8").replace("\r\n", "\n") | |
| m = _FRONTMATTER_RE.match(text) | |
| if not m: | |
| return path.stem, text, {} | |
| meta = yaml.safe_load(m.group(1)) or {} | |
| except (OSError, UnicodeDecodeError, yaml.YAMLError) as e: | |
| raise CorpusImportError(f"cannot parse markdown file at {path}: {e}") from e | |
| if m: | |
| if not isinstance(meta, dict): | |
| meta = {} | |
| body = m.group(2) | |
| title = str(meta.get("title") or path.stem) | |
| return title, body, meta |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/corpus_import.py` around lines 102 - 106, The markdown/frontmatter
parsing path in corpus_import should convert raw read/decode/YAML failures into
CorpusImportError so the CLI sees a consistent import error contract. Update the
logic around the path.read_text, _FRONTMATTER_RE.match, and yaml.safe_load flow
to catch unreadable file, decoding, and invalid frontmatter exceptions, then
re-raise them as CorpusImportError with the original exception chained.
Comment on lines
+333
to
+335
| json_source_id: str | None = None | ||
| if format in {"chat-json", "memory-export"}: | ||
| json_source_id = _register_file_source(store, resolved) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep dry-run fully non-mutating.
_register_file_source() writes via store.put_source(), so dry_run=True still persists imported source files before proposals are skipped. This violates the dry-run objective.
Suggested fix
json_source_id: str | None = None
- if format in {"chat-json", "memory-export"}:
+ if not dry_run and format in {"chat-json", "memory-export"}:
json_source_id = _register_file_source(store, resolved)
@@
source_ids: list[str] = []
- if candidate.source_path is not None:
+ if not dry_run and candidate.source_path is not None:
source_ids = [_register_file_source(store, candidate.source_path, root=vault_root)]Also applies to: 378-380
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/corpus_import.py` around lines 333 - 335, Dry-run still mutates
state because `_register_file_source()` calls `store.put_source()` before
proposals are skipped. Update the import flow in `corpus_import.py` so the
`json_source_id` registration happens only when `dry_run` is false, or make
`_register_file_source()` a no-op for dry runs. Apply the same guard in both the
chat-json/memory-export path and the other duplicate call site, using the
existing `dry_run` flag and `_register_file_source` helper to keep imports fully
non-mutating.
|
|
||
| vault_root = resolved if resolved.is_dir() else resolved.parent | ||
| proposed = 0 | ||
| cap = max_proposals if max_proposals is not None and max_proposals >= 0 else None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject negative proposal caps instead of disabling the cap.
max_proposals=-1 currently becomes unlimited, which can bypass the flood-protection cap if called outside a validating CLI layer.
Suggested fix
- cap = max_proposals if max_proposals is not None and max_proposals >= 0 else None
+ if max_proposals is not None and max_proposals < 0:
+ raise CorpusImportError("max_proposals must be non-negative")
+ cap = max_proposals📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cap = max_proposals if max_proposals is not None and max_proposals >= 0 else None | |
| if max_proposals is not None and max_proposals < 0: | |
| raise CorpusImportError("max_proposals must be non-negative") | |
| cap = max_proposals |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/corpus_import.py` at line 339, The proposal cap handling in
corpus_import should not treat negative max_proposals values as unlimited;
update the logic around the cap assignment in the import path so that only None
means no cap and any negative value is explicitly rejected or clamped as
invalid. Use the existing corpus_import proposal limit flow and the
cap/max_proposals check to enforce validation before continuing, so callers
cannot bypass flood protection by passing -1.
Comment on lines
+378
to
+388
| source_ids: list[str] = [] | ||
| if candidate.source_path is not None: | ||
| source_ids = [_register_file_source(store, candidate.source_path, root=vault_root)] | ||
|
|
||
| page = propose_page( | ||
| store, | ||
| title=title, | ||
| body=candidate.body or "", | ||
| page_type="concept", | ||
| source_ids=source_ids, | ||
| tags=candidate.tags, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Attach JSON import sources to generated pages.
Long chat-json assistant messages are converted to page proposals, but only markdown candidates get source_ids; the registered JSON source is dropped for those pages.
Suggested fix
source_ids: list[str] = []
- if candidate.source_path is not None:
+ if json_source_id is not None:
+ source_ids = [json_source_id]
+ elif candidate.source_path is not None:
source_ids = [_register_file_source(store, candidate.source_path, root=vault_root)]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| source_ids: list[str] = [] | |
| if candidate.source_path is not None: | |
| source_ids = [_register_file_source(store, candidate.source_path, root=vault_root)] | |
| page = propose_page( | |
| store, | |
| title=title, | |
| body=candidate.body or "", | |
| page_type="concept", | |
| source_ids=source_ids, | |
| tags=candidate.tags, | |
| source_ids: list[str] = [] | |
| if json_source_id is not None: | |
| source_ids = [json_source_id] | |
| elif candidate.source_path is not None: | |
| source_ids = [_register_file_source(store, candidate.source_path, root=vault_root)] | |
| page = propose_page( | |
| store, | |
| title=title, | |
| body=candidate.body or "", | |
| page_type="concept", | |
| source_ids=source_ids, | |
| tags=candidate.tags, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/corpus_import.py` around lines 378 - 388, The page proposal path in
propose_pages currently only attaches source_ids from candidate.source_path, so
JSON-backed chat imports lose their registered source. Update the candidate
handling in propose_pages to preserve and pass through the JSON source
identifier for generated pages as well, reusing the existing
_register_file_source and source_ids flow so both markdown and chat-json
candidates attach sources consistently.
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.
What changed
added a new
vouch import <format> <path>cli command for conversation-export ingestion with three formats:chat-json,markdown-vault, andmemory-export. the implementation lives in a newsrc/vouch/corpus_import.pymodule that normalizes each format into candidates, routes everything throughproposals.propose_claim/proposals.propose_page, supports--dry-run, enforces--max-proposals, and reports dedup/cap outcomes. tests were added intests/test_import.py, andCHANGELOG.mdwas updated under[Unreleased].Why
this fixes #431 by giving users a migration path from existing agent history into vouch without bypassing the review gate. users can now seed a kb from prior exports while keeping the same propose-only workflow, with controls (
--dry-run,--max-proposals) to avoid flooding review and dedup behavior to reduce repeated claims/pages.What might break
no breaking surface change is expected for existing
.vouch/directories: no file moves, no on-disk schema migration, and no changes to existingkb.*method behavior. this adds a new cli entrypoint and writes pending proposals/sources using existing storage + proposal flows.VEP
not required for this change. no object-model,
kb.*method list, on-disk layout, bundle format, or audit-log shape change was introduced.Tests
make checkpasses locally (lint + mypy + pytest)CHANGELOG.mdupdated under## [Unreleased]Summary by CodeRabbit
New Features
vouch importcommand to import conversation exports from supported formats.--max-proposalscaps, and--jsonoutput for automation.Bug Fixes