Reconsider the nlp-engine/agent crate boundary for LLM connectivity code #1978

Overview

This surfaced while adding prompt/response dump instrumentation for debugging LLM conversations (see #1956, where a user asked to see the exact prompt/response sent to and received from the LLM at every point in the agent loop). The fix required two separate dump mechanisms in two different crates for what is conceptually one thing — "the exact conversation sent to/from an LLM" — which is a direct symptom of a crate-boundary question worth revisiting deliberately, not folding into an unrelated PR.

Problem Statement

• packages/nlp-engine/src/chat/prompt_dump.rs, wired into ChatEngine::generate_blocking — the single chokepoint for the local/native GGUF path (Stage-1 routing, Stage-2 ReAct turns, resolve_query, the routing probe — everything that calls into llama.cpp).
• packages/agent/src/local_agent/openai_compat_prompt_dump.rs, wired directly into OpenAiCompatInferenceEngine::generate in packages/agent/src/local_agent/openai_compat_inference.rs — because that implementation is a pure HTTP client (reqwest) that never touches nlp-engine at all.

Both implement the same ChatInferenceEngine trait and are functionally interchangeable inference backends, but live in different crates because one needs a network dependency and the other doesn't.

Current split, all inside packages/agent/src/local_agent/: the ReAct loop (agent_loop.rs), the native inference bridge (inference.rs, wrapping nlp-engine::ChatEngine), the OpenAI-compatible HTTP client (openai_compat_inference.rs, openai_compat_discovery.rs), and the model manager (model_manager.rs) all live in the agent crate. Only the actual llama.cpp wrapper (ChatEngine, generate_streaming) lives in nlp-engine.

Proposed Solution

Current architecture (../nodespace-docs/architecture/crate-boundaries.md): nlp-engine is documented as a leaf crate scoped to "LLM inference and embedding computation" only, explicitly "no internal deps," with its decision tree routing anything that "orchestrates AI agents" to the agent crate instead. Under this rule, OpenAiCompatInferenceEngine lives in agent specifically because it needs reqwest/futures, and giving a leaf crate a networking dependency conflicts with that document's stated design.

Alternative framing (raised during #1956's investigation): the split should be app/orchestration vs. NLP-domain, not local-compute vs. everything-else. Under that framing, "how a conversation reaches a model" — whether via local llama.cpp or a remote HTTP endpoint — is one cohesive NLP-domain concern ("LLM connectivity"), regardless of backend, and belongs together in nlp-engine (accepting a reqwest dependency there) rather than split by which backend happens to need a network call. The ReAct loop, tool execution, and skill routing (agent_loop.rs) would still belong in agent under either framing — this is specifically about the inference engine implementations, not the orchestration logic built on top of them.

Why this matters going forward: additional remote-model backends beyond OpenAI-compatible endpoints are expected. Under the current split, each new backend's HTTP client code keeps landing in agent for the same leaf-crate-purity reason, while the domain concept of "talking to an LLM" stays fragmented across two crates. Deciding this now, before more backends exist, avoids compounding the split.

Decide, as an explicit architecture decision, whether inference-engine implementations (both local and remote) should consolidate into nlp-engine, stay split as they are today, or use some other boundary.

Architecture Impact

This issue is explicitly about deciding whether to reverse or extend ../nodespace-docs/architecture/crate-boundaries.md's current rule (nlp-engine = LLM inference/embedding computation only, leaf crate, no deps; agent = orchestration, including any HTTP-based inference backend). That document was written specifically to prevent misplaced code ("agent-authored code was incorrectly placed in desktop-app... these rules prevent that class of mistake") — so any resolution here should either reaffirm the current rule with clearer rationale, or formally supersede it via a new ADR, not just move files without updating the doc.

If consolidation is chosen: nlp-engine gains a reqwest/HTTP dependency (currently absent — it is a "no internal deps" leaf crate); crate-boundaries.md's "Contains" / "Does not contain" sections for both crates need updating; the two now-separate prompt-dump mechanisms could merge into one.

Acceptance Criteria

• Explicit decision made on whether inference-engine implementations (inference.rs, openai_compat_inference.rs, openai_compat_discovery.rs) consolidate into nlp-engine, stay in agent, or use a different boundary
• Decision recorded as an ADR in ../nodespace-docs/decisions/ (or an edit to crate-boundaries.md directly, if the resolution is narrow enough not to warrant a full ADR)
• crate-boundaries.md updated to reflect whichever boundary is decided, so it stays the accurate reference for future placement decisions
• If consolidation is chosen, a follow-up issue is filed for the actual code move (out of scope here)

Technical Specifications

Reference Files

• packages/nlp-engine/src/chat/mod.rs — ChatEngine::generate_streaming/generate_blocking, the local inference chokepoint
• packages/nlp-engine/src/chat/prompt_dump.rs — the local-path dump mechanism added in Expose Gemma 4 12B as a selectable native model (once verified on adequate hardware) #1956
• packages/agent/src/local_agent/inference.rs — LlamaChatInferenceEngine, the native bridge implementing ChatInferenceEngine
• packages/agent/src/local_agent/openai_compat_inference.rs — OpenAiCompatInferenceEngine, the remote HTTP-based implementation of the same trait
• packages/agent/src/local_agent/openai_compat_prompt_dump.rs — the remote-path dump mechanism added in Expose Gemma 4 12B as a selectable native model (once verified on adequate hardware) #1956
• packages/agent/src/local_agent/openai_compat_discovery.rs — endpoint model discovery for the remote path
• ../nodespace-docs/architecture/crate-boundaries.md — the current documented rule this issue evaluates

Non-Goals

• Actually performing the crate move — this issue is about making the decision, not executing it.
• Changing agent_loop.rs (ReAct loop, tool execution, skill routing) — out of scope regardless of which way this is decided.

Related Issues

• Expose Gemma 4 12B as a selectable native model (once verified on adequate hardware) #1956 (where the two-dump-mechanism symptom surfaced)