From 028907554cc05e0b746382430ee5913d3f935d0c Mon Sep 17 00:00:00 2001 From: Henry Park Date: Sat, 27 Jun 2026 19:57:56 -0700 Subject: [PATCH] =?UTF-8?q?docs(reborn-itest):=20descope=20Slice=209=20emb?= =?UTF-8?q?eddings=20fake=20=E2=80=94=20seam=20unreachable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 9 set out to add a Tier-1 fake `EmbeddingProvider` for the Reborn integration-test harness. Investigation at base 6a3b10fa5 found there is no embeddings seam to intercept in the Reborn memory path, so the fake is descoped. This records the verdict in the design spec (§3.6/§3.7) and the reborn test-support CLAUDE.md so the analysis isn't repeated. Evidence: - Reborn first-party memory tools dispatch only through NativeMemoryService (host_runtime first_party_tools/memory.rs:161); the test override is #[cfg(test)] and unreachable from the cross-crate tests/support/reborn/. - NativeMemoryService::search hardcodes .with_vector(false) (memory_native/src/service.rs:104) — embeddings never consulted on search. - build_native_backend wires embedding_provider: None and never embeds on write (build_chunk_writes persists text-only chunks). - The harness wires no EmbeddingProvider/MemoryService at all; memory_search returns correct membership via FTS with zero embeddings wiring. - Trait mismatch: the Reborn path uses ironclaw_memory_native::EmbeddingProvider, not ironclaw_embeddings::EmbeddingProvider (the latter is v1 Workspace only). Docs-only; no production or test code changes. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-26-reborn-integration-test-framework-design.md | 14 ++++++++++++-- tests/support/reborn/CLAUDE.md | 9 +++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md b/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md index bf729fbf6e3..e9458bd6f74 100644 --- a/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md +++ b/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md @@ -126,7 +126,7 @@ The LLM is one egress; a real turn crosses several. The contract is **every netw | Shell / process | `RuntimeProcessPort` | **inert `RecordingProcessPort`** (scripted output, no real process) — **NEW, see below** | real shell only via explicit `.with_live_shell()` | | Secrets / OAuth | `SecretStore` | `StaticSecretStore` (fixture handles); refresh worker not spawned | `.with_secret(handle, value)` to seed | | Approval gates | approval store | in-memory auto-approve | `.deny_capability` / explicit gate resolution | -| Embeddings | `EmbeddingProvider` | none wired; `InMemoryBackend` linear-scan | **caveat:** semantic-ordering assertions are unreliable — assert membership, not vector rank | +| Embeddings | `EmbeddingProvider` | none wired, **and unreachable from the harness path — no fake built (Slice 9 verdict, see below)** | memory_search is pure FTS; **assert membership, not vector rank** | | Trace Commons (telemetry) | `ContributionHttpSink` (`ironclaw_reborn_traces`) | **already captured** — the agent path (`HostEgressContributionSink`) routes through `RuntimeHttpEgress`, so `RecordingRuntimeHttpEgress` records it; no new sink needed | assert by filtering `recorded_egress.requests()` by the contribution URL | | MCP servers | MCP client | not wired by default | opt-in: `.with_mock_mcp(server)` wiring the existing `MockMcpServer` — **P1 ergonomics** | @@ -134,6 +134,16 @@ The LLM is one egress; a real turn crosses several. The contract is **every netw Injection seam: `RebornBuildInput` → `build_local_runtime` does not expose a process-port override today (`apply_runtime_process_binding` handles only `TenantSandbox`/`None`). Wiring the recording port needs a small `#[cfg(any(test, feature = "test-support"))]` accessor in `ironclaw_reborn_composition::factory` accepting an injectable `Arc` — the same visibility-promotion pattern Step 4 uses for `mount_local_dev_database_roots`. Scoped into Step 5b. +**Embeddings — no fake (Slice 9 verdict, seam unreachable).** Investigation for the planned embeddings fake found **no seam to intercept in the Reborn harness path**, so the fake is descoped. Evidence (base `6a3b10fa5`): + +- The Reborn first-party memory tools (`builtin.memory_search`/`memory_write`/`memory_read`/`memory_tree`) dispatch through `NativeMemoryService` only. `MemoryCapabilityState::service_for` (`crates/ironclaw_host_runtime/src/first_party_tools/memory.rs:161`) *always* builds `NativeMemoryService::from_filesystem(filesystem, …)`; the sole override (`memory_service_for_test`) is `#[cfg(test)]`, internal to that crate's own unit tests and unreachable from the cross-crate `tests/support/reborn/` harness. +- `NativeMemoryService::search` (`crates/ironclaw_memory_native/src/service.rs:104`) hardcodes `.with_vector(false)` on every request — there is no parameter or opt-in — so vector/embedding search is unconditionally off and `EmbeddingProvider::embed` is never called on a search. +- `build_native_backend` (`service.rs:415`) wires `ChunkingMemoryDocumentIndexer::new(repository)` with `embedding_provider: None` and never calls `.with_embedding_provider`; on write, `build_chunk_writes(…, None)` persists text-only chunks and the backend even swallows indexer errors — so `embed` is never called on a write either. +- The harness wires no `EmbeddingProvider`/`MemoryService` at all (grep of `tests/support/reborn/` is empty), and memory_search returns correct **membership** results via FTS with zero embeddings wiring. +- Trait mismatch compounds it: the Reborn path uses `ironclaw_memory_native::EmbeddingProvider`, a *different* trait from the `ironclaw_embeddings::EmbeddingProvider` the build order referenced. The latter is wired only into the v1 `Workspace` (`src/workspace/mod.rs`, `src/app.rs::init_tools`), which the Reborn harness does not exercise. + +Conclusion: a fake `EmbeddingProvider` would have nothing to intercept. Membership assertions over memory_search are covered for free by FTS through the real `NativeMemoryService` path (the future memory-coverage slice should assert membership there, not vector rank). Re-scope a fake only if/when the Reborn memory path gains a vector-search path that consults `EmbeddingProvider` (e.g. `NativeMemoryService` stops forcing `with_vector(false)` *and* a cross-crate provider-injection accessor is added). + **Trace Commons — no new type.** `ironclaw_reborn_traces::ContributionHttpSink` looks like a separate egress, but the agent-invoked path (`HostEgressContributionSink`, `trace_commons.rs`) delegates to `RuntimeHttpEgress` — so it is *already* captured by the default `RecordingRuntimeHttpEgress`. A test asserts contribution behavior by filtering `recorded_egress.requests()` for the contribution URL. (The CLI/background-worker path uses a direct client, but that worker is not spawned in the harness — §3.6 "Secrets/OAuth" note.) No `RecordingContributionSink` is built. **P1 ergonomics (not blockers):** a URL-keyed scripting layer over `RecordingRuntimeHttpEgress` for multi-step tool-HTTP flows, and a `.with_mock_mcp(...)` constructor wiring the existing `MockMcpServer` into the Reborn `ExtensionRegistry`. Both are additive; the default capture matrix above is the required floor. @@ -142,7 +152,7 @@ Injection seam: `RebornBuildInput` → `build_local_runtime` does not expose a p There is **no single outbound chokepoint** — production has four distinct seam families, so the harness uses a **two-tier** interception model (matching the industry split: trait-level fakes for orchestration logic, request-matcher mocks for HTTP adapters): -- **Tier 1 — trait-level fakes** (for logic that runs *above* the call): `LlmProvider` (scripted `TraceLlm`, §3.1), `EmbeddingProvider` (fake), `RuntimeProcessPort` (inert `RecordingProcessPort`), and **channel delivery** (`OutboundDeliverySink` — captured by `RecordingOutboundDeliverySink` at its own trait boundary, *not* through `RuntimeHttpEgress`; see §3.6). LLM/embeddings hold their own `reqwest` clients and are deliberately **not** mocked at HTTP — mocking at the provider trait runs the real chain/loop (the "FakeChatModel" lesson: HTTP-level mocks silently pass when the SDK/loop layer above them changes). +- **Tier 1 — trait-level fakes** (for logic that runs *above* the call): `LlmProvider` (scripted `TraceLlm`, §3.1), ~~`EmbeddingProvider` (fake)~~ **— descoped: the embeddings seam is unreachable in the Reborn memory path, see §3.6 "Embeddings — no fake"**, `RuntimeProcessPort` (inert `RecordingProcessPort`), and **channel delivery** (`OutboundDeliverySink` — captured by `RecordingOutboundDeliverySink` at its own trait boundary, *not* through `RuntimeHttpEgress`; see §3.6). LLM/embeddings hold their own `reqwest` clients and are deliberately **not** mocked at HTTP — mocking at the provider trait runs the real chain/loop (the "FakeChatModel" lesson: HTTP-level mocks silently pass when the SDK/loop layer above them changes). - **Tier 2 — recording interceptor over the HTTP-egress family**: MCP, OAuth, OAuth-refresh, first-party HTTP tools (and Trace Commons) route through the single `RuntimeHttpEgress::execute(RuntimeHttpEgressRequest{ runtime, capability_id, url, method, … })` trait; WASM/network-policy tool calls go through the sibling `NetworkHttpEgress`. The harness wires both recorders (`RecordingRuntimeHttpEgress` + `RecordingNetworkHttpEgress`) — see §3.6 for the authoritative per-boundary list. Today they record a scripted FIFO body; the P1 ergonomics extension (§3.6) is a URL/`capability_id`-keyed matcher over the same recorders. **Rejected: a single HTTP interceptor for *everything*.** Routing LLM/embeddings/process through one HTTP layer would rip provider auth/retry/circuit out of `ironclaw_llm`, force non-HTTP boundaries (process, future CLI) into an HTTP shape, lose type safety (match on serialized bodies), and reintroduce the per-provider HTTP fixtures §3.1 rejects. It contradicts the locked SDK-seam requirement. diff --git a/tests/support/reborn/CLAUDE.md b/tests/support/reborn/CLAUDE.md index 92b8c4305ec..c6f62b6fa8c 100644 --- a/tests/support/reborn/CLAUDE.md +++ b/tests/support/reborn/CLAUDE.md @@ -88,3 +88,12 @@ Slice 1 ships the spine + one text-reply test. Slice 2 (this PR) ships matrix; inert process port + `.with_live_shell()` / `.with_live_http_egress()` opt-ins; outbound/HTTP/secrets/MCP capture wiring; a dedicated `assertions.rs` once the `assert_*` family grows; the pre-commit test-style check. + +**Descoped (not planned): the embeddings fake.** Slice 9 verified there is no +embeddings seam to intercept in the Reborn memory path — `NativeMemoryService` +(the only memory service the first-party memory tools dispatch through) hardcodes +`.with_vector(false)` on every search and wires no `EmbeddingProvider` on write, +so `EmbeddingProvider::embed` is never called. memory_search returns correct +**membership** via FTS with zero embeddings wiring; a future memory-coverage +test should assert membership there, not build a fake. Full evidence: design spec +§3.6 "Embeddings — no fake (Slice 9 verdict)".