feat: Phase 8 — Embedded maintenance operations (rebuild, compact, WAL prune)

[tazarov]

Summary

Phase 8: Embedded Maintenance
Goal: Expose 5 embedded-mode maintenance operations (rebuild, compact-collection, compact-all, prune-wal-collection, prune-wal-all) in the Java surface — EmbeddedSession callback slots, JNA + Panama backend wiring, and integration tests.
Status: Verified ✓ | UAT: 5/5 passed

Expands EmbeddedSession from a 2-parameter constructor to a 7-parameter constructor with 5 BiFunction<Long, String, T> callback slots for maintenance operations. Both JNA and Panama backends bind all 5 chroma_embedded_* FFI symbols and inject callFfiJson-based lambdas at construction time. 14 integration tests (7 per backend) verify the wiring end-to-end.

Changes

Plan 08-01: EmbeddedSession Callback Slots + Backend Wiring

Expanded EmbeddedSession with 5 BiFunction callback slots and 7 public maintenance methods. Wired both JNA (JnaBindings + lambdas) and Panama (Ffi MethodHandle + Arena-scoped lambdas) backends.

Key files modified:

• java/core/src/main/java/tech/amikos/chroma/local/core/EmbeddedSession.java
• java/jna/src/main/java/tech/amikos/chroma/local/jna/JnaChromaRuntime.java
• java/panama/src/main/java/tech/amikos/chroma/local/panama/PanamaChromaRuntime.java
• java/core/src/test/java/tech/amikos/chroma/local/core/EmbeddedSessionTest.java

Plan 08-02: Integration Tests

14 integration tests (7 JNA + 7 Panama) covering smoke tests, error paths, input validation, and closed-session guards for all 5 maintenance operations.

Key files created:

• java/jna/src/test/java/tech/amikos/chroma/local/jna/JnaEmbeddedMaintenanceTest.java
• java/panama/src/test/java/tech/amikos/chroma/local/panama/PanamaEmbeddedMaintenanceTest.java

Requirements Addressed

• EMNT-01: EmbeddedSession.rebuildCollection(name, options) returns RebuildCollectionResult in both backends
• EMNT-02: EmbeddedSession.compactCollection(request) and compactAll(request) return CompactionResult in both backends
• EMNT-03: EmbeddedSession.pruneCollectionWAL(name, options) and pruneAllWAL(options) return WALPruneResult in both backends
• EMNT-04: Option builders validate inputs at build time
• EMNT-05: Integration tests verify each embedded maintenance operation in both backends

Verification

• Automated verification: 5/5 must-haves passed
• UAT: 5/5 tests passed (compilation, JNA tests, Panama tests, API surface, lint)
• Regression gate: all prior-phase tests pass

Test plan

• cd java && gradle :core:compileJava :jna:compileJava :panama:compileJava — all modules compile
• CHROMA_LIB_PATH=<shim> gradle :jna:test :panama:test — all 19 tests pass
• gradle :core:check :jna:check :panama:check — lint clean

[tazarov]

@claude PTAL

[claude]

Claude finished @tazarov's task in 2m 39s —— View job

---

Code Review — Phase 8: Embedded Maintenance Operations

• Gather context and understand request
• Read key changed files
• Review EmbeddedSession API design
• Review JNA backend wiring
• Review Panama backend wiring
• Review integration tests
• Post review findings

---

Overall the implementation is solid. The callback-injection pattern is consistent with ServerSession, the FFI wiring is correct in both backends, and the test suite covers the key cases (null input, closed-session guard, smoke paths, nonexistent-collection errors). A few findings below.

Bugs / Stale State

ServerSession stub messages are out of date — rebuildCollection and pruneCollectionWAL were correctly updated to Phase 10, but three stubs were missed:

// ServerSession.java:92
throw new UnsupportedOperationException("compactCollection will be wired in Phase 8");
// ServerSession.java:97
throw new UnsupportedOperationException("compactAll will be wired in Phase 8");
// ServerSession.java:111
throw new UnsupportedOperationException("pruneAllWAL will be wired in Phase 8");

Phase 8 shipped without wiring server-mode operations (by design), so these should be updated to reflect the correct future phase (Phase 10, same as the others, or whatever the roadmap says). Fix this →

---

API Design Observations

WALPruneOptions is shared by both pruneCollectionWAL and pruneAllWAL — the name field has no meaning in the all-WAL context, but there's no enforcement preventing callers from passing a named option to pruneAllWAL. The name may be silently ignored by the shim. A small Javadoc note or assertion (if (options.name() != null) throw ...) on pruneAllWAL would prevent accidental misuse.

Asymmetric convenience overloads — rebuildCollection(String name) and pruneCollectionWAL(String name) have string shorthands, but compactCollection does not. A compactCollection(String name) → new CompactCollectionRequest.Builder(name).build() overload would complete the set and reduce boilerplate for the common case. Not a blocker, but worth considering.

---

Test Coverage

No positive smoke test for rebuildCollection or compactCollection — both JNA and Panama tests only verify the error path (nonexistent collection → ChromaException). A happy-path test that creates a collection first (even via the Chroma client) and then calls rebuildCollection would give stronger confidence in the full wiring. compactAll does have a smoke test (empty database → 0 collections), which is good. Missing smoke tests for the remaining two operations is a gap.

Integration test boilerplate is repetitive — each of the 7 JNA tests and 7 Panama tests re-initializes the runtime and session inline. A @BeforeEach/@AfterEach pair (or a JUnit 5 @TempDir-based fixture) would halve the setup noise. Not a functional issue, but readability would benefit.

---

Minor Observations

WALPruneResult.warning vs RebuildCollectionResult.warnings — one is a single String, the other is a List<String>. If this mirrors the FFI response shape this is correct, but worth confirming the models are consistent with the Rust shim's JSON output.

collectionCount typed as int in CompactionResult and WALPruneResult while all other aggregate fields use long. Given these originate from Go uint64, an extreme value would silently overflow. Practically this is a non-issue (no one has 2B+ collections), but it's a minor inconsistency with the rest of the numeric fields.

Panama lambda verbosity — the five maintenance lambdas in PanamaChromaRuntime.doStartEmbedded() are each ~8 lines of identical Arena/invokeExact boilerplate. The existing comment (line 288) explains the invokeExact constraint that prevents abstraction. This is an accepted cost; just noting it for completeness.

---

Summary

One actionable bug (stale Phase 8 references in ServerSession stubs) and a few small gaps in test coverage. The core FFI wiring, null-safety, and close-guard logic are all correct and consistent with the existing patterns in the codebase.

[tazarov]

@claude PTAL again

[claude]

Claude finished @tazarov's task in 1m 57s —— View job

---

Re-review — Phase 8: Embedded Maintenance Operations

• Gather context and understand request
• Check what changed since previous review
• Review ServerSession stubs (stale Phase 8 references)
• Review API surface changes
• Review test coverage updates
• Post re-review findings

---

All actionable items from the first review have been addressed across two fix commits (a8a49fc, 99c2304). Summary below.

Resolved

Stale Phase 8 stub messages — all three UnsupportedOperationException messages in ServerSession.java (lines 92, 101, 106) now correctly read "Phase 10". ✅

collectionCount typed as int — changed to long in both CompactionResult and WALPruneResult, consistent with the Rust shim's u64. ✅

WALPruneResult.warning (single String) — changed to List<String> to match the Rust shim's Vec<String> at lines 614, 768–841. ✅

Asymmetric convenience overloads — compactCollection(String name) overload added to both EmbeddedSession (line 83) and ServerSession (line 95). ✅

pruneAllWAL / name field confusion — Javadoc note added to both EmbeddedSession.pruneAllWAL (line 107) and ServerSession.pruneAllWAL (line 113). ✅

Null-returning test stubs — replaced with UnsupportedOperationException throws in EmbeddedSessionTest, and 13 unit tests added (5 constructor null-rejections, 5 input validations, 2 convenience overload delegation tests). ✅

---

Remaining Minor Observations

These were noted in the first review and are not blockers; just carried forward for completeness:

• No delegation test for compactCollection(String) — rebuildCollectionConvenienceOverloadDelegates and pruneCollectionWalConvenienceOverloadDelegates exist in EmbeddedSessionTest, but there is no equivalent for the newly added compactCollection(String) overload. A symmetrical test would complete the set.

• No positive smoke test for rebuildCollection or compactCollection — integration tests still only cover error paths (nonexistent collection → ChromaException). compactAll and pruneAllWAL have smoke tests. Acknowledged gap; happy-path coverage can come with a future phase that creates collections.

• Integration test setup boilerplate — each of the 14 integration tests re-initializes the runtime inline. @BeforeEach/@AfterEach would reduce noise. Style-only; not a functional concern.

---

Summary

The implementation is clean. All bugs identified in the first pass are fixed. The two remaining items (missing delegation test, missing smoke tests) are cosmetic/coverage gaps — they do not affect correctness of the FFI wiring or the closed-session guard logic. LGTM.

[tazarov]

@claude

[claude]

Claude finished @tazarov's task in 1m 0s —— View job

---

Re-review — Phase 8: Embedded Maintenance Operations

• Gather context and understand request
• Check what changed since previous review
• Review remaining minor observations from last pass
• Post re-review findings

---

All three remaining minor observations from the second review have been addressed in 5a52a46.

Resolved

Missing delegation test for compactCollection(String) — compactCollectionConvenienceOverloadDelegates() added to EmbeddedSessionTest, verifying the convenience overload captures the correct handle and JSON payload. Now symmetric with the rebuildCollection and pruneCollectionWAL delegation tests. ✅

No smoke tests for rebuildCollection and compactCollection convenience overloads — embeddedRebuildCollectionConvenienceOverloadSmoke and embeddedCompactCollectionConvenienceOverloadSmoke added to both JnaEmbeddedMaintenanceTest and PanamaEmbeddedMaintenanceTest. These verify the full FFI path is reached (nonexistent collection → ChromaException), confirming the wiring is live in both backends. ✅

---

Still Pending (acknowledged, not blockers)

• Integration test setup boilerplate — each of the 19 integration tests still re-initializes the runtime inline. @BeforeEach/@AfterEach would reduce noise. Style-only; not a functional concern.

• No true happy-path smoke test for rebuildCollection or compactCollection — the new tests confirm the FFI wiring reaches the shim, but the only observable outcome is a ChromaException for a missing collection. A test that pre-creates a collection and observes a successful result would give stronger end-to-end confidence. This requires collection management APIs; reasonable to defer to a later phase as previously noted.

---

Summary

The three actionable items from the second review are all closed. The implementation is correct, the callback wiring is consistent, the delegation tests are now symmetric, and both backends have smoke coverage for all five maintenance operations including the convenience overloads. LGTM.