Arkive/arkive

Fork 0

Implement deterministic full-text index generation #70

Merged

JackFrostbyte merged 3 commits from feature/issue-22-index-builder into develop

2026-10-02 22:39:41 -04:00

McJuniorstein commented

2026-09-19 11:44:58 -04:00

Member

Summary

Adds arkive.index_build, installed as arkive-build-index, which builds the SQLite FTS5 lexical index and its Issue #21 manifest for the bundled core from validated records. The command takes an explicit --scope (v0.1: bundled_core only) and --output directory outside data/; the domain is never inferred.
Eight fail-closed stages, each named in the error output: environment (FTS5 compiled in, no fallback), records (the canonical validator must accept the whole tree, so schema-invalid, cross-record-invalid, and user_local-under-data/ trees are refused), inputs (documented eligibility rules, duplicate chunk identifiers and duplicate insertion keys refused), integrity (every included document's bytes match its recorded checksum and size, every chunk's text equals its [char_start, char_end) slice; otherwise the inputs are stale and refused), build (one table, one transaction, total insertion order, no SQLite side files), manifest (every digest through arkive.index_manifest), verify (the canonical validator checks the staged manifest against the records), commit (atomic per-file rename with rollback of both previous files).
Eligibility: a document is indexed iff its scope is the build scope, its source is accepted and bundleable, and it has chunk records. Everything else in scope is recorded under inputs.excluded with a validator-checked reason. Excluding a source covers its documents; including a document includes all its chunks per the v0.1 membership contract. The rules are not configurable.
Determinism as ADR-0004 bounds it: same records, same SQLite version → identical artifact bytes and identical manifest outside provenance. The builder reproduces the committed fixture artifact byte for byte; the suite asserts this when the SQLite version matches and skips otherwise.
The pilot-core fixture is now produced by the builder. make_pilot_core_fixture.py only fixes the arguments, so the fixture can no longer drift from production output. The committed artifact is unchanged; only build.builder.tool and the platform string in the manifest changed.
Documentation: new docs/index-build.md (command, stages, eligibility table, build configuration, determinism claims and non-claims, output handling, relationship to #23/#25/#48), a command section in docs/development.md, docs/index-manifest.md updated where it said building was future work, and a CHANGELOG entry.

Design notes

Input boundary is the validator. The builder runs validate_tree over the record root and refuses on any problem before loading anything. It has no rule of its own about what a valid record is, so it cannot accept what the validator rejects and cannot drift from it.
Stale means bytes, not records. The validator compares records to records; the builder adds the byte check because an index promises that offsets land on cited text. A build from a file that changed under its record would cite passages that are not there.
Output must be outside data/ and must not contain the record tree, so a generated index cannot be read back as reviewed content or committed by accident. The repository root itself is therefore refused as an output; build/index is the documented location and is already gitignored.
What is not atomic: the two renames at commit. A crash between them leaves a manifest whose recorded checksum does not match the artifact, which the validator reports as checksum-invalid and #25 must treat as "do not query". Documented in docs/index-build.md §6.
No new dependency. SQLite is standard library; the FTS5 check is the only environment requirement (ADR-0004 offline and capability boundary).

Verification

git diff --check
python pipeline/validate/validate_records.py and python pipeline/validate/validate_records.py data/sources tests/fixtures/index/pilot-core
python -m unittest discover -s tests -v — 427 tests pass (49 new)
python -m ruff check src pipeline tests
python -m ruff format --check src pipeline tests
Rendered Markdown reviewed
Mutation check: 21 guards disabled one at a time (FTS5 check, validator verdict, each eligibility rule, duplicate and insertion-key guards, SHA and slice checks, rollback, insertion order, both output-location guards, staging cleanup, side-file check, exit-code mapping, metadata-only and no-chunk exclusions, replaced flag); every one is caught by at least one test.
A complete build runs with socket creation disabled.
Coverage of src/arkive/index_build.py after adding tests for the reachable error paths; the remaining uncovered lines are defensive branches unreachable with a validated tree (unreadable JSON after the validator parsed it, a non-sha256 algorithm the schema forbids, an out-of-scope document under data/, a pre-existing file in a fresh mkdtemp directory, a sqlite3.Error during the write, and rollback errors that need a failing disk).

Scope confirmation

No unrelated production behavior changed
No metadata contract changed
No unreviewed source or generated data was added (the regenerated fixture manifest differs only in builder identity and platform string; the artifact bytes are identical)
Out of scope untouched: no search UI or query path, no vector or hybrid index, no automatic source processing, no deployment bundle, no large-corpus work, no user_local build

Linked issue

Closes #22

🤖 Generated with Claude Code

## Summary - Adds `arkive.index_build`, installed as `arkive-build-index`, which builds the SQLite FTS5 lexical index and its Issue #21 manifest for the bundled core from validated records. The command takes an explicit `--scope` (v0.1: `bundled_core` only) and `--output` directory outside `data/`; the domain is never inferred. - Eight fail-closed stages, each named in the error output: `environment` (FTS5 compiled in, no fallback), `records` (the canonical validator must accept the whole tree, so schema-invalid, cross-record-invalid, and `user_local`-under-`data/` trees are refused), `inputs` (documented eligibility rules, duplicate chunk identifiers and duplicate insertion keys refused), `integrity` (every included document's bytes match its recorded checksum and size, every chunk's text equals its `[char_start, char_end)` slice; otherwise the inputs are stale and refused), `build` (one table, one transaction, total insertion order, no SQLite side files), `manifest` (every digest through `arkive.index_manifest`), `verify` (the canonical validator checks the staged manifest against the records), `commit` (atomic per-file rename with rollback of both previous files). - Eligibility: a document is indexed iff its scope is the build scope, its source is accepted and bundleable, and it has chunk records. Everything else in scope is recorded under `inputs.excluded` with a validator-checked reason. Excluding a source covers its documents; including a document includes all its chunks per the v0.1 membership contract. The rules are not configurable. - Determinism as ADR-0004 bounds it: same records, same SQLite version → identical artifact bytes and identical manifest outside `provenance`. The builder reproduces the committed fixture artifact byte for byte; the suite asserts this when the SQLite version matches and skips otherwise. - The `pilot-core` fixture is now produced by the builder. `make_pilot_core_fixture.py` only fixes the arguments, so the fixture can no longer drift from production output. The committed artifact is unchanged; only `build.builder.tool` and the platform string in the manifest changed. - Documentation: new `docs/index-build.md` (command, stages, eligibility table, build configuration, determinism claims and non-claims, output handling, relationship to #23/#25/#48), a command section in `docs/development.md`, `docs/index-manifest.md` updated where it said building was future work, and a CHANGELOG entry. ## Design notes - **Input boundary is the validator.** The builder runs `validate_tree` over the record root and refuses on any problem before loading anything. It has no rule of its own about what a valid record is, so it cannot accept what the validator rejects and cannot drift from it. - **Stale means bytes, not records.** The validator compares records to records; the builder adds the byte check because an index promises that offsets land on cited text. A build from a file that changed under its record would cite passages that are not there. - **Output must be outside `data/`** and must not contain the record tree, so a generated index cannot be read back as reviewed content or committed by accident. The repository root itself is therefore refused as an output; `build/index` is the documented location and is already gitignored. - **What is not atomic:** the two renames at commit. A crash between them leaves a manifest whose recorded checksum does not match the artifact, which the validator reports as checksum-invalid and #25 must treat as "do not query". Documented in `docs/index-build.md` §6. - No new dependency. SQLite is standard library; the FTS5 check is the only environment requirement (ADR-0004 offline and capability boundary). ## Verification - [x] `git diff --check` - [x] `python pipeline/validate/validate_records.py` and `python pipeline/validate/validate_records.py data/sources tests/fixtures/index/pilot-core` - [x] `python -m unittest discover -s tests -v` — 427 tests pass (49 new) - [x] `python -m ruff check src pipeline tests` - [x] `python -m ruff format --check src pipeline tests` - [x] Rendered Markdown reviewed - [x] Mutation check: 21 guards disabled one at a time (FTS5 check, validator verdict, each eligibility rule, duplicate and insertion-key guards, SHA and slice checks, rollback, insertion order, both output-location guards, staging cleanup, side-file check, exit-code mapping, metadata-only and no-chunk exclusions, replaced flag); every one is caught by at least one test. - [x] A complete build runs with socket creation disabled. - [x] Coverage of `src/arkive/index_build.py` after adding tests for the reachable error paths; the remaining uncovered lines are defensive branches unreachable with a validated tree (unreadable JSON after the validator parsed it, a non-`sha256` algorithm the schema forbids, an out-of-scope document under `data/`, a pre-existing file in a fresh `mkdtemp` directory, a `sqlite3.Error` during the write, and rollback errors that need a failing disk). ## Scope confirmation - [x] No unrelated production behavior changed - [x] No metadata contract changed - [x] No unreviewed source or generated data was added (the regenerated fixture manifest differs only in builder identity and platform string; the artifact bytes are identical) - [x] Out of scope untouched: no search UI or query path, no vector or hybrid index, no automatic source processing, no deployment bundle, no large-corpus work, no `user_local` build ## Linked issue Closes #22 🤖 Generated with [Claude Code](https://claude.com/claude-code)

McJuniorstein added 1 commit

2026-09-19 11:44:58 -04:00

Implement deterministic lexical index generation 55f6617796

Add arkive.index_build (installed as arkive-build-index), which builds the
SQLite FTS5 index and its manifest for the bundled core from validated
records. The builder takes an explicit distribution scope, refuses any record
tree the canonical validator rejects, applies documented eligibility rules with
validator-checked exclusion reasons, refuses duplicate identifiers and stale
inputs by checking document bytes and chunk offsets, writes the ADR-0004 table
in one transaction in a total insertion order, verifies the staged manifest
with the canonical validator, and only then replaces an existing index through
atomic renames with rollback. Repeated builds on the same SQLite version are
byte-identical; provenance is the only section that varies.

The pilot-core fixture is now produced by the builder; the fixture script only
fixes the arguments. The artifact bytes are unchanged.

Closes #22

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

McJuniorstein referenced this pull request

2026-09-19 11:44:59 -04:00

Implement deterministic full-text index generation #22

JackFrostbyte commented

2026-09-19 21:20:03 -04:00

Owner

I found one storage-boundary issue that I think should be fixed before merge:

1. Symlinked parent directories can bypass the path boundary

_check_output_dir() uses abspath() and rejects the final output path if it is itself a symlink, but it does not resolve symlinks in parent components.

For example, if:

/tmp/build/link -> <repo>/data

then an output such as:

/tmp/build/link/index

is not itself a symlink and can pass the current lexical checks, while mkdir() would actually create/write the index under <repo>/data.

There is a similar issue in _document_file(): the lexical relative_to(records_root) check plus path.is_symlink() protects against absolute/.. traversal and a symlinked final file, but a parent directory inside the record tree could itself be a symlink resolving outside the tree.

Please make these boundary checks operate on the effective/resolved filesystem location (or explicitly reject symlinked path components) and add regression tests for:

an output path whose parent is a symlink into data/;
a document path whose parent inside data/sources is a symlink outside the record tree.

2. Please also handle pre-existing SQLite side files

The implementation states that index.sqlite-wal, index.sqlite-shm, and index.sqlite-journal must never remain in a finished index directory, but the current side-file check only checks the staging database. A pre-existing side file in the destination appears able to survive a successful rebuild.

Please either fail closed on these files or handle their cleanup safely, and add a rebuild regression test proving the final output cannot retain stale SQLite sidecars.

Everything else I reviewed looks aligned with Issue #22 and the existing manifest/storage contracts. Once these boundary cases are covered, this should be in very good shape for merge.

The implementation is very solid overall and covers Issue #22 thoroughly: deterministic FTS5 generation, canonical record validation, eligibility/exclusion handling, stale-byte and chunk-offset checks, manifest generation/verification, rollback, reproducibility tests, and network-disabled execution all look well designed. I found one storage-boundary issue that I think should be fixed before merge: **1. Symlinked parent directories can bypass the path boundary** `_check_output_dir()` uses `abspath()` and rejects the final output path if it is itself a symlink, but it does not resolve symlinks in parent components. For example, if: `/tmp/build/link -> <repo>/data` then an output such as: `/tmp/build/link/index` is not itself a symlink and can pass the current lexical checks, while `mkdir()` would actually create/write the index under `<repo>/data`. There is a similar issue in `_document_file()`: the lexical `relative_to(records_root)` check plus `path.is_symlink()` protects against absolute/`..` traversal and a symlinked final file, but a parent directory inside the record tree could itself be a symlink resolving outside the tree. Please make these boundary checks operate on the effective/resolved filesystem location (or explicitly reject symlinked path components) and add regression tests for: * an output path whose parent is a symlink into `data/`; * a document path whose parent inside `data/sources` is a symlink outside the record tree. **2. Please also handle pre-existing SQLite side files** The implementation states that `index.sqlite-wal`, `index.sqlite-shm`, and `index.sqlite-journal` must never remain in a finished index directory, but the current side-file check only checks the staging database. A pre-existing side file in the destination appears able to survive a successful rebuild. Please either fail closed on these files or handle their cleanup safely, and add a rebuild regression test proving the final output cannot retain stale SQLite sidecars. Everything else I reviewed looks aligned with Issue #22 and the existing manifest/storage contracts. Once these boundary cases are covered, this should be in very good shape for merge.

McJuniorstein referenced this pull request from a commit

2026-09-27 13:59:46 -04:00

Close the symlink and SQLite side-file gaps in index output

McJuniorstein added 1 commit

2026-09-27 13:59:46 -04:00

Close the symlink and SQLite side-file gaps in index output 91d68fbfdd

Address the two review findings on PR #70.

Path boundary: _check_output_dir() now checks the output against data/
on both the spelled and the symlink-resolved path of each side, and
returns the resolved path so every later write goes where the check
looked. A link that resolves somewhere safe is still accepted.
_document_file() refuses any symbolic link on the path below the
repository root, not only a symlinked final file. The canonical
validator already refuses symlinks in the core store, but it keys on
its own DATA_DIR, so a --repository-root naming another checkout
bypassed it; the builder no longer relies on that.

Side files: a destination holding index.sqlite-journal, -wal or -shm
(including a dangling link) is refused before the build and again at
commit. Arkive never creates these files, so one means something else
wrote to the index; it is neither deleted nor built beside, since
whether its owner is still running cannot be determined reliably.

Adds 11 regression tests; each new guard was mutation-checked.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

McJuniorstein commented

2026-09-27 14:13:37 -04:00

Author

Member

Thanks, both fixed in 91d68fb:

Symlink boundary. _check_output_dir() now checks the path as given and the symlink-resolved path, for both the output and data/, and returns the resolved one so writes go where the check looked. A link that resolves somewhere safe (e.g. /tmp on macOS) is still accepted. _document_file() walks every component below the repository root and refuses any symlink. Regression tests cover both of your cases, plus a symlinked data/sources, a data/ that is itself a link, and a parent that is a symlink containing the record tree.
SQLite side files. A destination holding index.sqlite-{journal,wal,shm} (including a dangling symlink) is refused before the build and again at commit. Arkive never creates these files (the builder writes only in staging, and the index uses the rollback journal), so one means something else wrote to the index. I chose refusing over cleanup because there's no reliable way to tell whether that process is still running: an idle SQLite connection holds no lock. There is a rebuild test for each suffix and one for a side file appearing mid-build.

Each new guard was mutation-checked: 9 mutations, all caught; 437 tests pass.

One thing to flag, not fixed here: the validator's core-only rules (symlink refusal, user_local-under-data/) key on its own DATA_DIR, so they don't apply when --repository-root names another checkout. That's how the document-symlink case actually got through. Eligibility still keeps user_local records out of the index, but it might be worth a follow-up issue to pass the core root into validate_tree.

Thanks, both fixed in 91d68fb: 1. **Symlink boundary.** `_check_output_dir()` now checks the path as given and the symlink-resolved path, for both the output and `data/`, and returns the resolved one so writes go where the check looked. A link that resolves somewhere safe (e.g. `/tmp` on macOS) is still accepted. `_document_file()` walks every component below the repository root and refuses any symlink. Regression tests cover both of your cases, plus a symlinked `data/sources`, a `data/` that is itself a link, and a parent that is a symlink containing the record tree. 2. **SQLite side files.** A destination holding `index.sqlite-{journal,wal,shm}` (including a dangling symlink) is refused before the build and again at commit. Arkive never creates these files (the builder writes only in staging, and the index uses the rollback journal), so one means something else wrote to the index. I chose refusing over cleanup because there's no reliable way to tell whether that process is still running: an idle SQLite connection holds no lock. There is a rebuild test for each suffix and one for a side file appearing mid-build. Each new guard was mutation-checked: 9 mutations, all caught; 437 tests pass. One thing to flag, not fixed here: the validator's core-only rules (symlink refusal, `user_local`-under-`data/`) key on its own `DATA_DIR`, so they don't apply when `--repository-root` names another checkout. That's how the document-symlink case actually got through. Eligibility still keeps `user_local` records out of the index, but it might be worth a follow-up issue to pass the core root into `validate_tree`.

JackFrostbyte added 1 commit

2026-10-02 22:36:39 -04:00