feat: single-fetch build lifecycle, fragments_on_skip, idempotent FTS, sibling imports (zeeker 0.8.0) - #9
Merged
Merged
Conversation
Defect 1 — single-fetch lifecycle: the fragments phase no longer reloads the resource module or re-invokes fetch_data(). The main phase threads the loaded module and the raw fetch_data() output through typed ValidationResult fields (module, raw_data), and the builder passes both into _process_fragments_for_resource as main_data_context. fetch_data() now runs exactly once per build; downstream PID sentinels / marker files are no longer needed. _process_fragments_for_resource keeps a backward-compatible signature (new params optional, legacy reload+refetch fallback when omitted). Defect 2 — new opt-in per-resource zeeker.toml flag fragments_on_skip: when fragments = true and fragments_on_skip = true, the fragments phase also runs on steady-state builds where fetch_data() returned no new rows (status "skipped"), with main_data_context=[] and the already-loaded module. Flag absent preserves existing behavior exactly. Defect 3 — idempotent --setup-fts: enable_fts(..., replace=True) so repeated setup on an existing database (incremental --sync-from-s3 builds) succeeds; the index is refreshed via rebuild_fts() (FTS5 'rebuild', idempotent) instead of populate_fts() (raw INSERT that double-indexes already-populated tables), followed by an explicit commit so the build connection releases its write lock. Defect 4 — sibling imports: _load_resource_module prepends the resources/ directory to sys.path before exec_module, so resource modules can import sibling helper modules without a manual shim. Also: document fragments_on_skip and the single-fetch lifecycle in CLAUDE.md; bump zeeker to 0.8.0; add 8 unit tests covering fetch-once, module-loaded-once, fragments_on_skip on/off, FTS idempotency (unit + end-to-end), and sibling imports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cuh5eJDfVowgpp22s7siE1
… aliasing, sys.path scoping, fetch-cache keying Addresses four verified defects from code review of the build-lifecycle change: 1. Pre-transform snapshot for main_data_context (processor.py). raw_data aliased the same list/dicts later handed to transform_data(), so an in-place transform (e.g. renaming/popping the heavy content column) mutated the "raw" context the fragments phase receives, violating the documented pre-transform contract. raw_data is now deep-copied before the transform runs (only when a transform_data function exists, so transform-less resources pay nothing). 2. Single-load/single-fetch guarantee now holds under --parallel (processor.py, builder.py). _prewarm_fetches imported its own module instance and ran fetch_data there, while the sequential loop imported a second, virgin instance that the fragments phase received — losing module-level state and double-running module import side effects. ResourceProcessor now keeps a per-build module cache keyed by resource name, so prewarm and the sequential loop share one instance; the cache is cleared in build_database's finally via clear_build_caches(). 3. Scoped, lowest-precedence sibling-import path (processor.py). The sys.path.insert(0, resources_dir) was permanent and took precedence over stdlib/site-packages, so resources/openai.py self-imported on `import openai`, and any helper named after a lazily-imported package (statistics, csv, requests, ...) shadowed it process-wide. The dir is now APPENDED for the duration of the module load only and removed in a finally block. Sibling helper modules registered in sys.modules during the load are tracked and purged at build end, so same-named helpers in different projects no longer leak between builds in one process. Sibling imports must be at module top level (documented in CLAUDE.md). 4. Build-stable fetch-cache key (async_executor.py). The cache key included the live row count, so a user migrate_schema() that changes the row count between the schema-check fetch and the insert fetch caused a cache miss and a second fetch_data invocation in the same build (double-spending API budgets / advancing checkpoints). The key is now the resource name alone; the cache is cleared between builds. Rejected one finding (builder.py:157, "context flip enables duplicate fragment insertion in dedup-pattern resources"): the non-empty main_data_context is the intended, versioned behavior of 0.8.0 — the [] context under 0.7.0 was an artifact of the duplicate fetch this release removes, never a contract. Verified against the cited pdpc project: its fetch_fragments_data is a `return []` stub that ignores context, so no duplicate insertion occurs. Adds 5 regression tests (parallel module state, in-place transform, sys.path cleanup, cross-project sibling leak, migrate_schema row-count change). Full suite: 386 passed, 5 skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cuh5eJDfVowgpp22s7siE1
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
2 participants
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.
Why
Every downstream data project independently works around the same four build-lifecycle defects: env-var PID sentinels and marker files to detect zeeker's duplicate
fetch_datacall, raw-SQL fragment insertion because the fragments phase never runs on steady-state builds, a hand-rolledsetup_fts.pybecause--setup-ftsisn't idempotent, andsys.pathshims in every resource file for sibling imports.Changes
1. Single-load, single-fetch lifecycle. The fragments phase no longer reloads the resource module or re-calls
fetch_data. The main phase's loaded module and raw fetch output (pre-transform_data, snapshotted via deepcopy when a transform exists) are threaded through asmain_data_context.fetch_datanow runs exactly once per build — including under--parallel, where a per-build module cache ensures the pre-warm and sequential phases share one module instance._process_fragments_for_resourcekeeps its old reload+refetch behavior when the new optional params are absent (backward-compatible for external callers).2.
fragments_on_skip = true(new opt-in per-resource flag inzeeker.toml): runs the fragments phase withmain_data_context=[]even whenfetch_datareturns no new rows — what enrichment-style pipelines need on steady-state builds. Flag absent → behavior identical to 0.7.0 (regression-tested).3. Idempotent
--setup-fts.enable_fts(..., replace=True), and the follow-up population switched frompopulate_fts()torebuild_fts()+ explicit commit. This also fixes a latent pre-existing double-indexing bug (enable_ftspopulated internally, thenpopulate_ftsinserted every row again) and a write-lock leak that could deadlock the next incremental build.--setup-ftsis now safe on--sync-from-s3builds.4. Sibling imports in
resources/. The resources directory is appended tosys.path(lowest precedence — stdlib/site-packages always win name clashes) only for the duration of module load, then removed. Resource modules canimport helperwithout shims.Version bumped 0.7.0 → 0.8.0 (uv.lock updated); CLAUDE.md documents the new lifecycle guarantee and flag.
Review process
Implemented, then adversarially reviewed by two independent reviewers (correctness + backward-compat against the documented downstream workaround patterns). 6 distinct findings; 5 confirmed and fixed:
main_data_context(in-place transforms were mutating the fragments context)--parallel(pre-warm loaded a second module instance whose state never reached fragments)sys.pathshadowing (prepend → append; scoped to load; removed after)1 rejected with verification: the non-empty
main_data_contexton new-row builds is the intended, documented contract of this release — the old[]context was an artifact of the duplicate call this PR removes.Tests
New
test_build_lifecycle.py: fetch-count == 1 on fresh and incremental fragments builds, module loaded exactly once,fragments_on_skipon/off paths, FTS idempotency (double setup, no duplicate index entries, triggers alive), sibling-import build. Full suite: 381 passed, 5 skipped (all pre-existing skips), coverage 73.98%.Downstream note
Existing workarounds (PID sentinels, marker files, module caches) remain harmless under 0.8.0 — they detect a second call that no longer happens. Follow-up PRs will remove them per repo once this releases.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Cuh5eJDfVowgpp22s7siE1
Generated by Claude Code