Add .kgmdignore exclusions and remove files that leave the corpus - #5
Merged
Conversation
`ruff check kgmd/ tests/` was already failing on main with two E501 violations that predate this branch: a long DELETE statement in kgmd/resolve.py and a long SELECT in tests/test_resolve.py. Both are wrapped as adjacent string literals, so the SQL they build is byte identical. `make format` also had pending drift in kgmd/llm.py and tests/test_docs.py. CI enforces `ruff check` but not `ruff format`, so the drift never failed a build and accumulated instead. It is applied here, isolated from feature work, to keep the pre-submit gate clean. No behaviour change.
Closes #3. A corpus had no way to exclude anything. `corpus.include` is an allowlist of literal paths, so skipping one folder meant enumerating every other folder and keeping that list current as the corpus grew. Every indexed file is chunked and each chunk is one model call, so indexing an archive or a tree of templates is repeated spend that buys nothing and fills entity resolution with junk mentions. A `.kgmdignore` file at the corpus root now subtracts paths, with a documented gitignore subset: comments, blank lines, `*` (never crossing `/`), `?`, `**`, trailing-`/` directory rules, leading-`/` anchoring, and `!` negation resolved last-match-wins. Precedence is fixed: `corpus.include` scopes the candidates, `.kgmdignore` subtracts and negations re-add, and the dot-path rule is applied last and cannot be overridden by any pattern. An absent file changes nothing. Patterns are compiled to anchored regexes in a new stdlib-only leaf module rather than taking a dependency. `fnmatch`'s `*` crosses `/`, and `PurePath.match` is right-anchored with single-segment `**` on the supported runtimes, so neither can express these semantics; `pathspec` faithfully reproduces git's refusal to re-include a file inside an excluded directory, which is the one behaviour we deliberately diverge from, so it could not satisfy the requirement either. Exclusion alone would have been a no-op for every corpus that already has a graph, so ingest now also removes documents whose recorded path has left the resolved file set. Newly-ignored, deleted, and renamed files are one state and are handled identically, which closes the long-standing gap where a deleted note was never removed. Cascades do not cover that removal. `relations.evidence_chunk_id` is ON DELETE SET NULL, so relations bound to removed evidence are deleted explicitly rather than surviving with no provenance. The sqlite-vec tables have no foreign keys, and `chunks.id` and `entity_mentions.id` are reused after deletes while `embed_new_chunks` skips any chunk that already has a vector row -- so a leftover vector would make a future unrelated chunk look embedded and search would answer from deleted text. Both vector tables are cleared for the removed ids. Entities left with no mention and no relation are swept, scoped to the entities this removal orphaned so pre-existing orphans are not collected as a side effect. Unconditional pruning makes a stray `*` destructive, so an ignore ruleset that resolves to an empty set while the graph holds documents aborts before any write instead of emptying the graph. `kgmd build --dry-run` reports the resolved file set, the exclusion counts, and how many indexed documents would be removed. It takes no lock, calls no model, and runs ahead of `init_db` so it creates no database. `--json` gives the machine-readable form and requires `--dry-run`, because a `--json` that implied it would mean `kgmd build --json` quietly not building. The payload lives in `ingest.dry_run_report` so the substance is testable without invoking the CLI, keeping `cli.py` a thin renderer. `kgmd init` writes a starter `.kgmdignore` whose every line is a comment, so a fresh corpus indexes exactly what it did before, and an existing file is never overwritten. No new config key: `corpus.exclude` was rejected rather than shipping two mechanisms for one job, which also leaves DEFAULT_CONFIG and the configuration reference's key count untouched. Documentation is part of the change. Seven statements across the docs set became false -- the "File deleted from disk | nothing" row, the "Deleted notes are not removed from the graph" limitation, and an architecture claim that ingest.py imports nothing from kgmd among them -- and are corrected here.
Matches the convention set by specs/001-project-documentation: the spec, the Phase 0 research record, the Phase 1 design artifacts, and the task breakdown are committed alongside the change they describe. research.md is the part worth keeping. Every decision is recorded with the evidence that settled it, including the probe output showing fnmatch's `*` crosses `/` and PurePath.match's `**` spans one segment, the verification that real git refuses to re-include a file inside an excluded directory, and R11's line-by-line list of documented statements this change falsifies.
Greptile SummaryThis PR adds
Confidence Score: 4/5The pruning commit should be moved to the end of successful ingestion before merging, because an ordinary file-processing failure can otherwise leave the corpus partially and destructively updated. Missing-document cleanup is committed before surviving files are read, statted, and chunked, and neither build nor extract can roll that commit back when later ingestion fails. Files Needing Attention: kgmd/ingest.py Reviews (1): Last reviewed commit: "Add Spec Kit planning artifacts for .kgm..." | Re-trigger Greptile |
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
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
corpus.includeis an allowlist of literal paths, so skipping one folder meant enumerating every other folder and keeping that list current as the corpus grew — and a folder added outside the list was silently never indexed..kgmdignorefile at the corpus root now subtracts paths, using a documented gitignore subset with no new runtime dependency. Ingest also now removes documents whose path has left the corpus, so the feature is not a no-op for corpora that already have a graph — which closes the long-standing gap where a deleted note was never removed.kgmd build --dry-runmakes the rules inspectable before any spend.Closes #3.
Changes
New capability
kgmd/ignore.py(new, stdlib-only leaf) — parses.kgmdignoreinto ordered rules and compiles each pattern to an anchored regex at parse time. Supported:#comments, blank lines,*(never crossing/),?,**, trailing-/directory rules, leading-/anchoring, and!negation resolved last-match-wins. Character classes and escapes are unsupported and match literally, so an unsupported pattern excludes nothing rather than too much.ingest.scan_corpus_files— the single source of the resolved file set, so the preview cannot disagree with what a build does. Precedence is fixed:corpus.includescopes the candidates →.kgmdignoresubtracts and negations re-add → the dot-path rule is applied last and cannot be overridden by any pattern.ingest.prune_missing_documents— removes documents whose recorded path is no longer in the resolved set. Newly-ignored, deleted, and renamed files are one state and handled identically.kgmd build --dry-run [--json]— reports the resolved set, exclusion counts, and pending removals. No lock, no model call, and it runs ahead ofinit_dbso it creates no database.kgmd initwrites a starter.kgmdignorewhose every line is a comment, and never overwrites an existing file.Why no dependency was added
fnmatch's*crosses/(translate("*.md")→(?s:.*\.md)) andPurePath.matchis right-anchored with single-segment**on the supported runtimes (full_matchis 3.13+, the floor is 3.10) — verified by probe, so neither can express these semantics.pathspecwas also rejected on merit, not just policy: it faithfully reproduces git's refusal to re-include a file inside an excluded directory, which is the one behaviour we deliberately diverge from, so it could not have satisfied the requirement. Confirmed against real git.Correctness work the cascades do not cover
relations.evidence_chunk_idisON DELETE SET NULL, so relations bound to removed evidence are deleted explicitly rather than surviving with no provenance.sqlite-vectables have no foreign keys,chunks.id/entity_mentions.idare reused after deletes, andembed_new_chunksskips any chunk that already has a vector row. A leftover vector would therefore make a future, unrelated chunk look already-embedded and search would answer from deleted text. Both vector tables are cleared for the removed ids, with a regression test on the id-reuse path.extract --forceare not collected as an unrequested side effect.IN (...)rather than aTEMP TABLE, because a temp table would be DDL outsidekgmd/schema.py.Safety guard added beyond the issue
Unconditional pruning makes a stray
*destructive, so an ignore ruleset that resolves to an empty set while the graph holds documents aborts before any write instead of emptying the graph.Deliberately not added
No
corpus.excludeconfig key — two mechanisms for one job.DEFAULT_CONFIGis untouched, so the configuration reference's key count and its bidirectional coverage test need no changes.Documentation
Seven documented statements became false and are corrected, including the
File deleted from disk | **nothing**row in the maintenance guide, theDeleted notes are not removed from the graphlimitation in the personal-notes example, and an architecture claim thatingest.pyimports nothing fromkgmd.Testing
tests/test_ignore.py, 30 pattern-semantics tests with no filesystemtests/test_ingest.py(25) for discovery, pruning, idempotency, the guard, and the stale-vector regression;tests/test_cli.py(6) for the two CLI-only guaranteesmake format,make lint, andmake testare all clean. Every commit on this branch was verified green in isolation, not just the tip.Tests are hermetic per Principle V:
tmp_pathcorpora, hand-packedstruct.packvectors, no embedding backend, no network.tests/fixtures/*.mdis untouched, so no exact-count assertion moves.Smoke-tested against a real corpus, not only in tests:
Real ingest on that corpus: newly-ignored → 1 document removed; deleted file → 1 removed; re-run → all zeros (idempotent); bare
*→ raised with the document still present and nothing deleted. Resolved 1,000 files in 0.051s against the spec's 5s budget.Related issues
kgmd reset,kgmd reset --hard,kgmd extract --force) leave the same class of stale state behind. That includes a previously unreported defect whereextract --forceleaves stale mention vectors and silently corrupts entity resolution. Out of scope here; the prune path added in this PR is the pattern to copy.This repo does not use Jira or Linear.
Additional Notes
.kgmdignoreproduces exactly the previous file set, asserted as a regression test.PRAGMA user_versionbump; this change only deletes rows from existing tables.kgmd extracttoo since it shares the ingest path.archive/followed by!archive/2024-decisions.mdre-includes that file. git refuses this. It is the motivating example from Add a .kgmdignore file to exclude paths from indexing #3, it is documented in the configuration reference, and it is why the directory walk is not pruned.make lintwas already red onmainwith two pre-existingE501violations, andmake formathad pending drift. Both are isolated in the first commit so the feature diff stays readable.