refactor(core): make spaCy opt-in + demand-gate the Python runtime (#5056) - #5101
Conversation
…inyhumansai#5056) Fresh installs spawned the runtime Python server on every launch and provisioned a spaCy venv + en_core_web_sm model on first run, even when no local NLP was configured (tinyhumansai#5056 / tinyhumansai#4814). Two changes stop that: - `memory_tree.spacy_enabled` now defaults to `false` (was `true`). With spaCy off (and Kompress already off by default), `enabled_backends()` is empty, so the runtime Python server no longer launches at boot and the spaCy venv/model is never downloaded. Query-entity extraction degrades to the existing in-Rust regex+LLM extractor; opt in via config or `OPENHUMAN_MEMORY_TREE_SPACY_ENABLED=1`. - The `python_runtime` harness-init step is now demand-gated via `python_needed_eagerly()` (Python enabled AND a backend needs it), so the managed CPython download no longer runs speculatively at boot. Lazy consumers (Python tools / skills / MCP) still resolve the interpreter on first use. Node + Kompress + server steps unchanged (already derive correctly). Coordinates with tinyhumansai#5055 (spaCy provider reliability) — this makes spaCy opt-in, not removed. Rank 4 of the tinyhumansai#5056 cleanup audit.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesRuntime opt-in gating
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant HarnessInit
participant Config
participant enabled_backends
participant python_runtime
HarnessInit->>Config: Read runtime_python.enabled
HarnessInit->>enabled_backends: Resolve enabled Python backends
alt Python enabled and a backend is enabled
HarnessInit->>python_runtime: Probe or provision eagerly
else No enabled backend
HarnessInit->>python_runtime: Report done and skip run
end
Suggested labels: Poem
Comment |
senamakel
left a comment
There was a problem hiding this comment.
Automated technical review: approved for the inspected head commit.
Summary of analysis
This PR changes default_memory_tree_spacy_enabled() from true to false, making spaCy opt-in, and introduces python_needed_eagerly() to demand-gate the managed-CPython provision step so that the interpreter is not speculatively downloaded at boot when no Python backend (spaCy/Kompress) is enabled.
What is correct
- The
python_needed_eagerlypredicate correctly composesruntime_python.enabledwithenabled_backendsbeing non-empty, so the eager provision only fires when a backend actually needs it. - The existing
spacy_stepalready independently gates onspacy_enabled, andruntime_python_server_stepalready gates onenabled_backendsbeing non-empty, so they correctly reflect the new default without additional wiring. - Serde's
#[serde(default)]means persisted configs retain their stored value; only fresh installs seefalse. Migration is seamless. - Test coverage:
spacy_enabled_defaults_to_false(config),enabled_backends_is_empty_by_default(runtime_python_server),python_runtime_step_is_done_by_default_with_no_backend_enabledandpython_needed_eagerly_true_when_spacy_backend_enabled(harness_init). All hermetic, no disk/network. - All CI hard gates pass: Rust Quality, Feature-Gate Smoke, Rust Core Coverage, PR CI Gate. The only red check (PR Submission Checklist) is a soft informational check.
One minor documentation finding (not blocking)
In src/openhuman/config/schema/storage_memory.rs at line 358, the doc comment on spacy_enabled still reads "When true (default)". Since the default is now false, the (default) parenthetical is stale and could mislead human readers of the API docs. Recommend changing to "When true (optional, default false)" or similar. The function-level comment above default_memory_tree_spacy_enabled() already correctly describes the new opt-in behavior — this is just the field-level docstring that needs alignment.
|
| Filename | Overview |
|---|---|
| src/openhuman/config/schema/storage_memory.rs | Changed default_memory_tree_spacy_enabled() from true to false; field-level doc comment still says "(default)" next to true — stale, but only a doc issue. |
| src/openhuman/harness_init/registry.rs | Introduced python_needed_eagerly(config) to gate managed CPython provisioning; python_is_done/python_run now early-return when no backend is enabled. Tests cover both the default no-op case and the enabled-backend case correctly. |
| src/openhuman/runtime_python_server/registry.rs | No functional code changes; adds enabled_backends_is_empty_by_default test that validates the new default-off spaCy behaviour end-to-end through the backend registry. |
Comments Outside Diff (1)
-
src/openhuman/config/schema/storage_memory.rs, line 357-363 (link)The field-level doc comment still describes
trueas the default, but the default is nowfalse. Any developer reading the rustdoc or IDE hover will get the wrong information about whether spaCy activates on a fresh install.
Reviews (1): Last reviewed commit: "refactor(core): make spaCy opt-in + dema..." | Re-trigger Greptile
Summary
memory_tree.spacy_enabledtofalse(wastrue) — spaCy becomes opt-in.python_runtimeharness-init step so managed CPython is not downloaded speculatively at boot.en_core_web_smmodel on first run, when no local NLP is configured.Problem
On a fresh install,
spacy_enableddefaultedtrue, soenabled_backends()returned[Spacy]and the runtime Python server launched on every boot, with the first run building a spaCy venv + downloadingen_core_web_sm. Separately, thepython_runtimestep resolved/downloaded managed CPython wheneverruntime_python.enabled(default true), even when nothing needed Python. This is the core of #5056's "unnecessary runtimes" item (and relates to #4814). spaCy only improves person/org recall in the E2GraphRAG query-entity extractor, which already has a full in-Rust regex+LLM fallback.Solution
default_memory_tree_spacy_enabled()→false. With spaCy off (Kompress already off by default),enabled_backends()is empty → the runtime Python server never launches at boot and the spaCy venv/model is never provisioned. Query-entity extraction degrades toscore::extract. Opt in via config orOPENHUMAN_MEMORY_TREE_SPACY_ENABLED=1.python_needed_eagerly(config)=runtime_python.enabled && !enabled_backends(config).is_empty(). Thepython_runtimestep'sis_done/rungate on it, so managed CPython is provisioned eagerly only when a Python backend actually needs it. Lazy consumers (Python tools / skills / MCP) still resolve the interpreter on first use.CREATE_NO_WINDOWcoverage is untouched (already complete); this PR changes whether work happens at boot, not how processes spawn.Coordinates with #5055 (spaCy provider reliability): this makes spaCy opt-in, it does not remove it.
Submission Checklist
spacy_enableddefaults false;enabled_backends(&Config::default())is empty;python_runtimestep is auto-done / no-op by default and provisions when a backend is enabled. Updated existing harness_init / runtime_python_server / config-default tests for the new default.N/A: behaviour-default change(no feature row add/remove; spaCy path unchanged, only its default).## RelatedN/A: retrieval works identically via the regex fallback; opt-in spaCy path unchanged.## Related(does not close Remove unused/redundant features: screen intelligence, order complete, desktop automation, autocomplete, Whisper, unnecessary runtimes #5056).Impact
spacy_enabled: truekeep spaCy on (serde default applies only to absent fields) — no behaviour change for current users; only fresh installs get the new default.cargo checkpasses (media ON + disabled gate); harness_init 12/12, runtime_python_server 17/17, memory_tree 210/210, config:: 472/472.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fixes/spacy-off-lazy-runtimesValidation Run
pnpm --filter openhuman-app format:check— N/A (no app changes)pnpm typecheck— N/A (no TS changes)cargo fmt --checkclean;cargo checkmedia ON + disabled gate passBehavior Changes
Parity Contract
enabled_backendsderivation unchanged; server/kompress/node steps unchangedDuplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes