Skip to content

feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) - #3183

Merged
nickpismenkov merged 4 commits into
feat/reborn-memory-pr3-libsql-behaviorfrom
feat/reborn-memory-pr4-postgres-behavior
May 4, 2026
Merged

nickpismenkov merged 4 commits into
feat/reborn-memory-pr3-libsql-behaviorfrom
feat/reborn-memory-pr4-postgres-behavior

Conversation

@nickpismenkov

Copy link
Copy Markdown
Contributor

Summary

PR 4 of the stack for #3118. Stacks on PR 3 (#3182). Replace the "not yet implemented" stubs in RebornPostgresMemoryDocumentRepository with a full read/write/list/search/version/chunk implementation against the reborn_memory_* Postgres substrate.

Every query carries the full (tenant_id, user_id, agent_id, project_id) scope tuple. Absent agent/project IDs use the empty-string DB sentinel; the constructor rejects empty / _none user-supplied IDs, so equality (agent_id = $3) is unambiguous and NULL/IS NULL gymnastics are unnecessary.

Behavior implemented

Operation What it does on the native Postgres schema
read_document Filter by full scope tuple + path; return content column as bytes.
read_document_metadata Filter by full scope tuple + path; return metadata JSONB column.
list_documents Scoped SELECT path of all rows under the scope.
write_document / write_document_with_options BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE -> list_paths_for_scope -> file/directory conflict check -> upsert with content_hash; archives prior content into reborn_memory_document_versions when content changed and skip_versioning != Some(true).
write_document_metadata Scoped UPDATE of the metadata JSONB column.
search_documents FTS branch joins through reborn_memory_chunks with ts_rank_cd over the GIN-indexed content_tsv; vector branch uses pgvector <=> ordering. Both branches are pre-fusion ranked; results fuse through the shared search::fuse_memory_search_results, honoring FusionStrategy.
replace_document_chunks_if_current SELECT … FOR UPDATE row lock, hash drift check, replace chunks with optional pgvector embedding.
delete_document_chunks Scoped DELETE through the reborn_memory_documents join.
run_migrations Wrapped in a session-level pg_advisory_lock(REBORN_MIGRATION_LOCK_ID) so concurrent processes (parallel test runs) serialize on CREATE EXTENSION pgcrypto/vector.

plainto_tsquery('english', \$5) already tolerates arbitrary punctuation, so the Postgres path does not need a manual FTS-escape helper — the punctuation contract is preserved at the query layer instead.

Test plan

13 new behavioral tests in tests/reborn_native_postgres_repository_contract.rs, at parity with the libSQL contract from PR 3:

  • round_trips_a_document_within_full_scope
  • returns_none_when_document_is_missing
  • upsert_replaces_content_for_same_full_scope_and_path
  • full_scope_isolates_user_agent_project_independently
  • top_level_projects_path_is_a_normal_user_path_not_project_scope
  • rejects_file_directory_prefix_conflicts_within_scope
  • writes_metadata_and_reads_it_back
  • write_with_options_creates_version_row_only_when_not_skipped
  • version_numbers_are_monotonic_and_content_hash_matches_archived_content
  • replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  • full_text_search_returns_only_chunks_within_full_scope
  • fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  • full_text_search_uses_rrf_when_only_full_text_branch_returns_results

Tests use the standard DATABASE_URL=postgres://localhost/ironclaw_test env-var pattern with try_connect skipping cleanly when Postgres is unreachable (matches tests/workspace_integration.rs); each test scopes a unique tenant prefix and cleans up via cleanup_tenant so parallel runs do not collide.

Verification (all green):

  • `cargo fmt --all -- --check` — clean
  • `cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings` — clean
  • `cargo clippy --all --tests --examples --all-features -- -D warnings` — clean
  • `cargo test -p ironclaw_memory --features libsql` — green
  • `cargo test -p ironclaw_memory --features postgres` — green (skips when no DB)
  • `cargo test -p ironclaw_memory --all-features` — green
  • `cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold` — pass
  • `python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD` — clean
  • `git diff --check origin/reborn-integration` — clean

Stack

🤖 Generated with Claude Code

Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e482).
Replace the `not yet implemented` stubs in
`RebornPostgresMemoryDocumentRepository` with a full
read/write/list/search/version/chunk implementation against the
`reborn_memory_*` Postgres substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and
NULL/IS NULL gymnastics are unnecessary.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinels back to `None`).
- write_document, write_document_with_options:
  BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE ->
  reborn_postgres_list_paths_for_scope -> conflict check ->
  upsert -> optional version row -> COMMIT. Computes content_hash on
  every write; archives prior content into
  `reborn_memory_document_versions` if the content changed and
  `skip_versioning` is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through `reborn_memory_chunks`
  with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector
  branch uses pgvector `<=>` ordering. Both branches are pre-fusion
  ranked; results are fused through the shared
  `search::fuse_memory_search_results`, honoring `FusionStrategy`.
- replace_document_chunks_if_current: SELECT … FOR UPDATE row lock,
  hash drift check (no-op if the document was rewritten under it),
  delete-then-insert chunks with optional pgvector embedding.
- delete_document_chunks: scoped DELETE through
  `reborn_memory_documents` join.
- run_migrations: wrapped in a session-level
  `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent
  processes (parallel test runs) serialize on
  `CREATE EXTENSION pgcrypto/vector`.

Behavioral coverage in
`tests/reborn_native_postgres_repository_contract.rs` (13 tests at
parity with the libSQL contract). Tests use the standard
`DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern
with `try_connect` skipping cleanly when Postgres is unreachable
(matches `tests/workspace_integration.rs`); each test scopes a
unique tenant prefix and cleans up via `cleanup_tenant` so parallel
runs do not collide:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

`plainto_tsquery('english', $5)` already tolerates arbitrary
punctuation, so the Postgres FTS-escape test asserts the same
contract as the libSQL counterpart against the unescaped path.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --features postgres
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `try_connect` so CI without a database
remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Comment thread crates/ironclaw_memory/src/repo/native_postgres.rs Outdated
nickpismenkov and others added 3 commits May 4, 2026 15:09
…R 5 of #3118) (#3184)

* test(reborn-memory): port pure-behavior contract over native repos

Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a
behavior contract for `RepositoryMemoryBackend` layered over both
`RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`. The semantics under test
live above the bare repository — they belong to the
`RepositoryMemoryBackend` composition (`.config` inheritance, schema
validation, indexer best-effort, capability fail-closed,
embedding-dimension guard).

These behaviors already exist in production code; PR 5 is purely
test coverage that ports the matching contract from the legacy
`db_memory_repository_contract.rs` over to the native
`reborn_memory_*` substrate. No production-code changes.

New file `tests/reborn_native_repository_backend_contract.rs`:

libSQL (10 tests, in-process temp DB):
  - libsql_backend_inherits_config_metadata_for_skip_indexing
  - libsql_backend_closer_config_overrides_parent_config
  - libsql_backend_honors_skip_versioning_from_config
  - libsql_backend_validates_schema_from_config_before_write
  - libsql_backend_reports_write_success_when_indexer_fails_after_persist
  - libsql_backend_search_fails_closed_for_unsupported_vector_search
  - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch
  - libsql_backend_hybrid_search_fuses_full_text_and_vector_results
  - libsql_backend_weighted_score_fusion_orders_results_by_weights
  - libsql_backend_search_honors_limit

Postgres (4 tests, skip-on-unreachable per
`tests/workspace_integration.rs:26-33`; each test scopes a unique
tenant prefix and cleans up via `pg_cleanup_tenant`):
  - postgres_backend_validates_schema_from_config_before_write
  - postgres_backend_honors_skip_versioning_from_config
  - postgres_backend_reports_write_success_when_indexer_fails_after_persist
  - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch

Coverage delta vs. PR 4:

  | Behavior in #3118 phase 6                              | PR 5 |
  |--------------------------------------------------------|------|
  | `.config` metadata inheritance precedence              |  ✓  |
  | Closer `.config` overrides parent `.config`            |  ✓  |
  | `skip_indexing` from `.config`                         |  ✓  |
  | `skip_versioning` from `.config`                       |  ✓  |
  | Schema validation before persistence                   |  ✓  |
  | Indexer failure after persist => write success         |  ✓  |
  | Capability fail-closed for vector search               |  ✓  |
  | Query embedding dimension mismatch fails closed        |  ✓  |
  | RRF fusion (hybrid)                                    |  ✓  |
  | WeightedScore fusion                                   |  ✓  |
  | `limit` honored                                        |  ✓  |
  | FTS query escaping (already covered in PR 3 / PR 4)    | n/a |
  | `version` hash semantics (already covered in PR 3/4)   | n/a |

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185)

* test(reborn-memory): vertical integration through public seams

Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add
caller-facing vertical-integration coverage that drives the full
public-seam stack against the Reborn-native repositories:

  reborn-native repository
    -> ChunkingMemoryDocumentIndexer
    -> RepositoryMemoryBackend
    -> MemoryBackendFilesystemAdapter
    -> CompositeRootFilesystem mounted at /memory

These behaviors already exist in production code; PR 6 is purely
test coverage that proves the Phase 7 invariants hold against the
native substrate. No production-code changes.

`tests/reborn_native_filesystem_vertical_integration.rs` —
public-seam vertical:

libSQL (6 tests, in-process temp DB):
  - libsql_round_trips_authorized_read_write_through_composite_mount
  - libsql_lists_direct_children_through_composite_mount
  - libsql_search_through_composite_mount_returns_only_same_scope_results
  - libsql_capability_denied_search_fails_closed_before_repository_is_called
  - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error
  - libsql_composite_mount_rejects_duplicate_root_at_registration

Postgres (2 tests, skip-on-unreachable; per-tenant cleanup):
  - postgres_round_trips_authorized_read_write_through_composite_mount
  - postgres_search_through_composite_mount_returns_only_same_scope_results

This amend additionally closes thoroughness gaps identified in a
Required-Tests audit of #3118. None of these are production
behavior changes — every gap is a missing assertion on behavior
the production code already implements:

`tests/reborn_native_repository_backend_contract.rs`:
  - Strengthen skip_indexing test to assert chunk-count = 0 in DB
    instead of indexer-call-count = 1 (uses real
    ChunkingMemoryDocumentIndexer that respects skip_indexing).
    Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing`
    -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`.
  - Add `libsql_backend_document_metadata_overrides_inherited_config`:
    parent .config says skip_versioning=true, doc-level metadata
    overrides to false; overwrite must produce a version row.
  - Strengthen weighted-score test to assert ordering changes when
    weights flip (FT-heavy vs vector-heavy produces different top
    result, proving weights actually steer the score).
  - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch`
    asserting result count is bounded by an effective
    pre_fusion_limit.
  - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test
    locking the clamp invariant of `with_pre_fusion_limit`).

`tests/reborn_native_libsql_repository_contract.rs`:
  - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`:
    two `tokio::join!`-launched writes serialize via `BEGIN
    IMMEDIATE`; assert exactly one row in `list_documents`.
  - Add `fts_query_with_only_stopwords_does_not_error` covering the
    issue's "empty/stopword-ish" FTS case.

`tests/reborn_native_postgres_repository_contract.rs`:
  - Add `same_path_in_different_tenants_stores_separate_rows`
    (Postgres parity for the tenant axis the libSQL contract
    already covered).
  - Add Postgres mirrors of the concurrent-writes and stopword-FTS
    tests above.

`src/search.rs`:
  - Add unit tests for `fuse_memory_search_results` deterministic
    tiebreak: tied scores must sort by relative path ascending,
    independent of insertion order.

Coverage delta for issue #3118 "Required tests":

  | Required test (issue)                                        | Status |
  |--------------------------------------------------------------|--------|
  | Same path different tenants/users/agents/projects            |  ✓    |
  | Search/list scope isolation                                  |  ✓    |
  | projects/ prefix as user path                                |  ✓    |
  | Same scope+path upserts                                      |  ✓    |
  | Different scope+same path -> different docs                  |  ✓    |
  | File/directory prefix conflict                               |  ✓    |
  | Concurrent writes -> exactly one row                         |  ✓    |
  | Indexer failure after persist -> write succeeds              |  ✓    |
  | .config applies to children                                  |  ✓    |
  | Closer .config overrides parent                              |  ✓    |
  | Document metadata overrides inherited .config                |  ✓    |
  | skip_indexing -> chunk count = 0                             |  ✓    |
  | skip_versioning -> no version row                            |  ✓    |
  | Schema validation rejects before persistence                 |  ✓    |
  | Write -> chunk -> FTS end-to-end                             |  ✓    |
  | FTS escaping (punct + stopwords)                             |  ✓    |
  | Vector dim mismatch                                          |  ✓    |
  | Hybrid ranking deterministic (path tiebreak)                 |  ✓    |
  | RRF + WeightedScore fusion                                   |  ✓    |
  | limit + pre_fusion_limit                                     |  ✓    |
  | Version monotonic + content hash                             |  ✓    |
  | Adapter / RootFilesystem vertical integration                |  ✓    |
  | Unsupported capability fails closed before side effects      |  ✓    |

Acceptance criteria from #3118 closed by this PR:

  - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` /
        `RootFilesystem`) have vertical integration coverage against
        native repositories.

Verification (all green; second run confirms reliability — first
run hit a pre-existing parallel-execution flake in the legacy
`db_memory_repository_contract` target, unrelated to this PR; passes
in isolation and on re-run):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3185 review findings

Three medium-severity issues raised on the vertical integration PR:

1. Postgres vertical tests in reborn_native_filesystem_vertical_integration
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported them as passed even when Postgres was
   unreachable. Replace the silent-skip helper with `pg_require_connection`
   that panics with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the
   `ironclaw_memory` guardrail that Postgres behavioral coverage must
   be real, not compile/skip coverage.

2. The Postgres vertical search test built `RepositoryMemoryBackend`
   without a `ChunkingMemoryDocumentIndexer`, so writes through
   `CompositeRootFilesystem` never populated `reborn_memory_chunks`.
   Search returned an empty Vec, and the assertion
   `paths.iter().all(|p| p == "visible.md")` was vacuously true even
   without proving anything about the write -> index -> search flow.
   Wire the same chunking indexer the libSQL vertical stack uses
   (FTS-only — no embedding provider), and add an explicit non-empty
   assertion before the scope check. Verified end-to-end against a
   `pgvector/pgvector:pg16` container.

3. The `pre_fusion_limit_is_clamped_up_to_limit` test in
   reborn_native_repository_backend_contract claimed the invariant
   `pre_fusion_limit >= limit` held regardless of caller order, but
   only exercised `with_limit().with_pre_fusion_limit()`. The reverse
   case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left
   `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch
   SQL `LIMIT` shrink below the requested final limit. Make
   `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order
   regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3184 review findings

Three medium-severity issues raised on the behavior-contract PR:

1. Postgres backend tests in reborn_native_repository_backend_contract
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported all Postgres tests as passed without
   running migrations or exercising any of the schema validation,
   versioning, indexer, or search behavior against Postgres. Replace
   the silent-skip helper with `pg_require_connection` that panics
   with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring
   the `ironclaw_memory` guardrail that Postgres repository coverage
   must be real, not compile/skip coverage.

2. The `libsql_backend_search_honors_limit` and
   `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests
   only asserted `results.len() <= 2`, which is vacuously satisfied
   by 0 or 1 results — a regression that broke FTS or indexing
   entirely would still pass. Establish the precondition with a
   larger-limit baseline search asserting the full match count is
   visible, then assert the bounded search returns exactly 2.

3. The new search/fusion/limit contract was libSQL-only, while
   `RebornPostgresMemoryDocumentRepository` has its own FTS
   (`tsvector`) and pgvector query implementations. Add Postgres
   counterparts for hybrid search, weighted-score fusion,
   pre-fusion-limit per-branch, and the `limit` truncation cases.
   The Postgres weighted-score test uses asymmetric FTS content
   (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks
   the FTS leader unambiguously above the hybrid match — without
   this, `ts_rank_cd` ties non-deterministically. Generalize
   `RecordingEmbeddingProvider` to be parametric on `DIM` so the
   same provider semantics work against libSQL (DIM=3, blob
   embeddings) and Postgres (DIM=1536, fixed-width pgvector column).

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 105 memory tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two review issues raised on the native Postgres repository PR:

1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional
   `delete_document_chunks` for both `skip_indexing == Some(true)` and
   the empty-chunks case. A stale reindex (older bytes, older
   skip_indexing flag) racing with a concurrent writer that has
   already produced fresh chunks for newer content could clobber the
   newer write's chunk rows, leaving the latest content unsearchable.

   Compute `content_hash_at_read` up front and route both paths
   through the same hash-guarded `replace_document_chunks_if_current`
   call. The hash guard turns a stale clear into a no-op once the
   document has moved on. `delete_document_chunks` is no longer
   reached by the indexer; the trait method stays in place to avoid
   churning the API across the stack.

   Add 3 regression tests in `mod tests` covering the skip_indexing
   path, the empty-chunks path, and the non-empty path, each
   asserting routing through `replace_document_chunks_if_current`
   with the read-time content hash.

2. The Postgres contract harness silently skipped every test when the
   pool could not hand out a connection — the binary reported all
   tests as passed without running migrations or exercising any
   read/write/search/chunk/version behavior, violating the
   `ironclaw_memory` guardrail that Postgres coverage must be real.
   Make `try_connect` panic with an actionable message unless the
   new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set.

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 108 memory tests pass, zero clippy warnings, fmt clean. Verified
fail-loud and opt-in-skip behavior with and without DATABASE_URL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous
fix routed the indexer through `replace_document_chunks_if_current` so
the live race the reviewer described (stale reindex clobbering chunks
from a newer write) was closed, but the unguarded
`delete_document_chunks` method was still on the trait and in 4
implementations as a footgun for any future caller.

Remove it entirely:

- Drop the trait method from `MemoryDocumentIndexRepository` and
  document at the trait level that chunk clearing is folded into
  `replace_document_chunks_if_current` (called with an empty `chunks`
  slice). A hash-guarded clear is the only safe shape here.
- Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`,
  `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the
  `libsql_document_id_and_content` helper that became dead with the
  legacy libsql impl.
- Remove the test mock's stub and the `IndexerCall::Delete` variant
  in the indexer regression tests; the recording repo now only
  implements the hash-checked path, so any future regression that
  starts calling something else fails to compile.

Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory
tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nickpismenkov
nickpismenkov merged commit 7558e97 into feat/reborn-memory-pr3-libsql-behavior May 4, 2026
14 checks passed
@nickpismenkov
nickpismenkov deleted the feat/reborn-memory-pr4-postgres-behavior branch May 4, 2026 22:28
nickpismenkov added a commit that referenced this pull request May 4, 2026
#3182)

* feat(reborn-memory): implement native libSQL repository behavior

Phase 4 of #3118, PR 3 of the stack. Stacks on PR 2 (#3181). Replace
the `not yet implemented` stubs in `RebornLibSqlMemoryDocumentRepository`
with a full read/write/list/search/version/chunk implementation
against the `reborn_memory_*` substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = ?`) is unambiguous and
NULL/IS NULL gymnastics are gone.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinel back to `None`).
- write_document, write_document_with_options:
  BEGIN IMMEDIATE -> reborn_libsql_list_paths_for_scope -> conflict
  check -> insert-or-update; computes content_hash on every write;
  writes prior content to reborn_memory_document_versions if the
  content changed and skip_versioning is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through reborn_memory_chunks_fts
  with full-scope WHERE; vector branch reads embeddings, computes
  cosine, sorts deterministically (path tiebreaker), then RRF /
  weighted-score fusion.
- replace_document_chunks_if_current: re-reads content_hash inside
  the transaction; no-op if it has drifted (prevents corrupting the
  index after a between-read-and-refresh rewrite).
- delete_document_chunks: scoped DELETE.

Reusable helpers added to `repo/mod.rs` and shared by both native
backends:

- REBORN_SCOPE_NONE_SENTINEL ("")
- reborn_agent_id_db_value, reborn_project_id_db_value
- reborn_memory_document_from_row(tenant, user, agent_db, project_db, db_path)

13 new behavioral tests in
`tests/reborn_native_libsql_repository_contract.rs`:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_tenant_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back_for_native_documents
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

The `reborn_libsql_repository_fails_closed_until_behavior_lands`
smoke test from PR 2 is removed (its purpose — proving stubs were
fail-closed before behavior landed — is now obsolete).

Postgres native repository remains stubbed; PR 4 implements it on
the same substrate with a behavioral testcontainer harness.

Verification (all green; 56 memory tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) (#3183)

* feat(reborn-memory): implement native Postgres repository behavior

Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e482).
Replace the `not yet implemented` stubs in
`RebornPostgresMemoryDocumentRepository` with a full
read/write/list/search/version/chunk implementation against the
`reborn_memory_*` Postgres substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and
NULL/IS NULL gymnastics are unnecessary.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinels back to `None`).
- write_document, write_document_with_options:
  BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE ->
  reborn_postgres_list_paths_for_scope -> conflict check ->
  upsert -> optional version row -> COMMIT. Computes content_hash on
  every write; archives prior content into
  `reborn_memory_document_versions` if the content changed and
  `skip_versioning` is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through `reborn_memory_chunks`
  with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector
  branch uses pgvector `<=>` ordering. Both branches are pre-fusion
  ranked; results are fused through the shared
  `search::fuse_memory_search_results`, honoring `FusionStrategy`.
- replace_document_chunks_if_current: SELECT … FOR UPDATE row lock,
  hash drift check (no-op if the document was rewritten under it),
  delete-then-insert chunks with optional pgvector embedding.
- delete_document_chunks: scoped DELETE through
  `reborn_memory_documents` join.
- run_migrations: wrapped in a session-level
  `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent
  processes (parallel test runs) serialize on
  `CREATE EXTENSION pgcrypto/vector`.

Behavioral coverage in
`tests/reborn_native_postgres_repository_contract.rs` (13 tests at
parity with the libSQL contract). Tests use the standard
`DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern
with `try_connect` skipping cleanly when Postgres is unreachable
(matches `tests/workspace_integration.rs`); each test scopes a
unique tenant prefix and cleans up via `cleanup_tenant` so parallel
runs do not collide:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

`plainto_tsquery('english', $5)` already tolerates arbitrary
punctuation, so the Postgres FTS-escape test asserts the same
contract as the libSQL counterpart against the unescaped path.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --features postgres
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `try_connect` so CI without a database
remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): port pure-behavior contract over native repos (PR 5 of #3118) (#3184)

* test(reborn-memory): port pure-behavior contract over native repos

Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a
behavior contract for `RepositoryMemoryBackend` layered over both
`RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`. The semantics under test
live above the bare repository — they belong to the
`RepositoryMemoryBackend` composition (`.config` inheritance, schema
validation, indexer best-effort, capability fail-closed,
embedding-dimension guard).

These behaviors already exist in production code; PR 5 is purely
test coverage that ports the matching contract from the legacy
`db_memory_repository_contract.rs` over to the native
`reborn_memory_*` substrate. No production-code changes.

New file `tests/reborn_native_repository_backend_contract.rs`:

libSQL (10 tests, in-process temp DB):
  - libsql_backend_inherits_config_metadata_for_skip_indexing
  - libsql_backend_closer_config_overrides_parent_config
  - libsql_backend_honors_skip_versioning_from_config
  - libsql_backend_validates_schema_from_config_before_write
  - libsql_backend_reports_write_success_when_indexer_fails_after_persist
  - libsql_backend_search_fails_closed_for_unsupported_vector_search
  - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch
  - libsql_backend_hybrid_search_fuses_full_text_and_vector_results
  - libsql_backend_weighted_score_fusion_orders_results_by_weights
  - libsql_backend_search_honors_limit

Postgres (4 tests, skip-on-unreachable per
`tests/workspace_integration.rs:26-33`; each test scopes a unique
tenant prefix and cleans up via `pg_cleanup_tenant`):
  - postgres_backend_validates_schema_from_config_before_write
  - postgres_backend_honors_skip_versioning_from_config
  - postgres_backend_reports_write_success_when_indexer_fails_after_persist
  - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch

Coverage delta vs. PR 4:

  | Behavior in #3118 phase 6                              | PR 5 |
  |--------------------------------------------------------|------|
  | `.config` metadata inheritance precedence              |  ✓  |
  | Closer `.config` overrides parent `.config`            |  ✓  |
  | `skip_indexing` from `.config`                         |  ✓  |
  | `skip_versioning` from `.config`                       |  ✓  |
  | Schema validation before persistence                   |  ✓  |
  | Indexer failure after persist => write success         |  ✓  |
  | Capability fail-closed for vector search               |  ✓  |
  | Query embedding dimension mismatch fails closed        |  ✓  |
  | RRF fusion (hybrid)                                    |  ✓  |
  | WeightedScore fusion                                   |  ✓  |
  | `limit` honored                                        |  ✓  |
  | FTS query escaping (already covered in PR 3 / PR 4)    | n/a |
  | `version` hash semantics (already covered in PR 3/4)   | n/a |

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185)

* test(reborn-memory): vertical integration through public seams

Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add
caller-facing vertical-integration coverage that drives the full
public-seam stack against the Reborn-native repositories:

  reborn-native repository
    -> ChunkingMemoryDocumentIndexer
    -> RepositoryMemoryBackend
    -> MemoryBackendFilesystemAdapter
    -> CompositeRootFilesystem mounted at /memory

These behaviors already exist in production code; PR 6 is purely
test coverage that proves the Phase 7 invariants hold against the
native substrate. No production-code changes.

`tests/reborn_native_filesystem_vertical_integration.rs` —
public-seam vertical:

libSQL (6 tests, in-process temp DB):
  - libsql_round_trips_authorized_read_write_through_composite_mount
  - libsql_lists_direct_children_through_composite_mount
  - libsql_search_through_composite_mount_returns_only_same_scope_results
  - libsql_capability_denied_search_fails_closed_before_repository_is_called
  - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error
  - libsql_composite_mount_rejects_duplicate_root_at_registration

Postgres (2 tests, skip-on-unreachable; per-tenant cleanup):
  - postgres_round_trips_authorized_read_write_through_composite_mount
  - postgres_search_through_composite_mount_returns_only_same_scope_results

This amend additionally closes thoroughness gaps identified in a
Required-Tests audit of #3118. None of these are production
behavior changes — every gap is a missing assertion on behavior
the production code already implements:

`tests/reborn_native_repository_backend_contract.rs`:
  - Strengthen skip_indexing test to assert chunk-count = 0 in DB
    instead of indexer-call-count = 1 (uses real
    ChunkingMemoryDocumentIndexer that respects skip_indexing).
    Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing`
    -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`.
  - Add `libsql_backend_document_metadata_overrides_inherited_config`:
    parent .config says skip_versioning=true, doc-level metadata
    overrides to false; overwrite must produce a version row.
  - Strengthen weighted-score test to assert ordering changes when
    weights flip (FT-heavy vs vector-heavy produces different top
    result, proving weights actually steer the score).
  - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch`
    asserting result count is bounded by an effective
    pre_fusion_limit.
  - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test
    locking the clamp invariant of `with_pre_fusion_limit`).

`tests/reborn_native_libsql_repository_contract.rs`:
  - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`:
    two `tokio::join!`-launched writes serialize via `BEGIN
    IMMEDIATE`; assert exactly one row in `list_documents`.
  - Add `fts_query_with_only_stopwords_does_not_error` covering the
    issue's "empty/stopword-ish" FTS case.

`tests/reborn_native_postgres_repository_contract.rs`:
  - Add `same_path_in_different_tenants_stores_separate_rows`
    (Postgres parity for the tenant axis the libSQL contract
    already covered).
  - Add Postgres mirrors of the concurrent-writes and stopword-FTS
    tests above.

`src/search.rs`:
  - Add unit tests for `fuse_memory_search_results` deterministic
    tiebreak: tied scores must sort by relative path ascending,
    independent of insertion order.

Coverage delta for issue #3118 "Required tests":

  | Required test (issue)                                        | Status |
  |--------------------------------------------------------------|--------|
  | Same path different tenants/users/agents/projects            |  ✓    |
  | Search/list scope isolation                                  |  ✓    |
  | projects/ prefix as user path                                |  ✓    |
  | Same scope+path upserts                                      |  ✓    |
  | Different scope+same path -> different docs                  |  ✓    |
  | File/directory prefix conflict                               |  ✓    |
  | Concurrent writes -> exactly one row                         |  ✓    |
  | Indexer failure after persist -> write succeeds              |  ✓    |
  | .config applies to children                                  |  ✓    |
  | Closer .config overrides parent                              |  ✓    |
  | Document metadata overrides inherited .config                |  ✓    |
  | skip_indexing -> chunk count = 0                             |  ✓    |
  | skip_versioning -> no version row                            |  ✓    |
  | Schema validation rejects before persistence                 |  ✓    |
  | Write -> chunk -> FTS end-to-end                             |  ✓    |
  | FTS escaping (punct + stopwords)                             |  ✓    |
  | Vector dim mismatch                                          |  ✓    |
  | Hybrid ranking deterministic (path tiebreak)                 |  ✓    |
  | RRF + WeightedScore fusion                                   |  ✓    |
  | limit + pre_fusion_limit                                     |  ✓    |
  | Version monotonic + content hash                             |  ✓    |
  | Adapter / RootFilesystem vertical integration                |  ✓    |
  | Unsupported capability fails closed before side effects      |  ✓    |

Acceptance criteria from #3118 closed by this PR:

  - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` /
        `RootFilesystem`) have vertical integration coverage against
        native repositories.

Verification (all green; second run confirms reliability — first
run hit a pre-existing parallel-execution flake in the legacy
`db_memory_repository_contract` target, unrelated to this PR; passes
in isolation and on re-run):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3185 review findings

Three medium-severity issues raised on the vertical integration PR:

1. Postgres vertical tests in reborn_native_filesystem_vertical_integration
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported them as passed even when Postgres was
   unreachable. Replace the silent-skip helper with `pg_require_connection`
   that panics with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the
   `ironclaw_memory` guardrail that Postgres behavioral coverage must
   be real, not compile/skip coverage.

2. The Postgres vertical search test built `RepositoryMemoryBackend`
   without a `ChunkingMemoryDocumentIndexer`, so writes through
   `CompositeRootFilesystem` never populated `reborn_memory_chunks`.
   Search returned an empty Vec, and the assertion
   `paths.iter().all(|p| p == "visible.md")` was vacuously true even
   without proving anything about the write -> index -> search flow.
   Wire the same chunking indexer the libSQL vertical stack uses
   (FTS-only — no embedding provider), and add an explicit non-empty
   assertion before the scope check. Verified end-to-end against a
   `pgvector/pgvector:pg16` container.

3. The `pre_fusion_limit_is_clamped_up_to_limit` test in
   reborn_native_repository_backend_contract claimed the invariant
   `pre_fusion_limit >= limit` held regardless of caller order, but
   only exercised `with_limit().with_pre_fusion_limit()`. The reverse
   case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left
   `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch
   SQL `LIMIT` shrink below the requested final limit. Make
   `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order
   regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3184 review findings

Three medium-severity issues raised on the behavior-contract PR:

1. Postgres backend tests in reborn_native_repository_backend_contract
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported all Postgres tests as passed without
   running migrations or exercising any of the schema validation,
   versioning, indexer, or search behavior against Postgres. Replace
   the silent-skip helper with `pg_require_connection` that panics
   with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring
   the `ironclaw_memory` guardrail that Postgres repository coverage
   must be real, not compile/skip coverage.

2. The `libsql_backend_search_honors_limit` and
   `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests
   only asserted `results.len() <= 2`, which is vacuously satisfied
   by 0 or 1 results — a regression that broke FTS or indexing
   entirely would still pass. Establish the precondition with a
   larger-limit baseline search asserting the full match count is
   visible, then assert the bounded search returns exactly 2.

3. The new search/fusion/limit contract was libSQL-only, while
   `RebornPostgresMemoryDocumentRepository` has its own FTS
   (`tsvector`) and pgvector query implementations. Add Postgres
   counterparts for hybrid search, weighted-score fusion,
   pre-fusion-limit per-branch, and the `limit` truncation cases.
   The Postgres weighted-score test uses asymmetric FTS content
   (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks
   the FTS leader unambiguously above the hybrid match — without
   this, `ts_rank_cd` ties non-deterministically. Generalize
   `RecordingEmbeddingProvider` to be parametric on `DIM` so the
   same provider semantics work against libSQL (DIM=3, blob
   embeddings) and Postgres (DIM=1536, fixed-width pgvector column).

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 105 memory tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3183 review findings

Two review issues raised on the native Postgres repository PR:

1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional
   `delete_document_chunks` for both `skip_indexing == Some(true)` and
   the empty-chunks case. A stale reindex (older bytes, older
   skip_indexing flag) racing with a concurrent writer that has
   already produced fresh chunks for newer content could clobber the
   newer write's chunk rows, leaving the latest content unsearchable.

   Compute `content_hash_at_read` up front and route both paths
   through the same hash-guarded `replace_document_chunks_if_current`
   call. The hash guard turns a stale clear into a no-op once the
   document has moved on. `delete_document_chunks` is no longer
   reached by the indexer; the trait method stays in place to avoid
   churning the API across the stack.

   Add 3 regression tests in `mod tests` covering the skip_indexing
   path, the empty-chunks path, and the non-empty path, each
   asserting routing through `replace_document_chunks_if_current`
   with the read-time content hash.

2. The Postgres contract harness silently skipped every test when the
   pool could not hand out a connection — the binary reported all
   tests as passed without running migrations or exercising any
   read/write/search/chunk/version behavior, violating the
   `ironclaw_memory` guardrail that Postgres coverage must be real.
   Make `try_connect` panic with an actionable message unless the
   new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set.

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 108 memory tests pass, zero clippy warnings, fmt clean. Verified
fail-loud and opt-in-skip behavior with and without DATABASE_URL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): remove unguarded delete_document_chunks API

Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous
fix routed the indexer through `replace_document_chunks_if_current` so
the live race the reviewer described (stale reindex clobbering chunks
from a newer write) was closed, but the unguarded
`delete_document_chunks` method was still on the trait and in 4
implementations as a footgun for any future caller.

Remove it entirely:

- Drop the trait method from `MemoryDocumentIndexRepository` and
  document at the trait level that chunk clearing is folded into
  `replace_document_chunks_if_current` (called with an empty `chunks`
  slice). A hash-guarded clear is the only safe shape here.
- Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`,
  `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the
  `libsql_document_id_and_content` helper that became dead with the
  legacy libsql impl.
- Remove the test mock's stub and the `IndexerCall::Delete` variant
  in the indexer regression tests; the recording repo now only
  implements the hash-checked path, so any future regression that
  starts calling something else fails to compile.

Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory
tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nickpismenkov added a commit that referenced this pull request May 4, 2026
#3181)

* docs(reborn-memory): reorient guardrails to native-isolated direction

Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112
and asks for a native, isolated Reborn memory subsystem with explicit
tenant/user/agent/project scope columns rather than synthetic
workspace user_id strings.

Update crates/ironclaw_memory/CLAUDE.md to reflect that decision:

- Drop reuse-first / preserve-existing-tables wording.
- State that src/workspace is reference material only and that
  ironclaw_memory must not depend on the main app crate.
- Spell out the full (tenant, user, agent, project, path) scope
  invariant on every read/list/search/write/version/chunk operation.
- Document the empty-string DB sentinel for absent agent/project
  columns and call out that _none is the virtual-path-only sentinel.
- Note that legacy migration/coexistence is explicitly deferred and
  that Postgres needs real (not compile-only) behavioral coverage.

No code changes; this PR is purely doc reorientation that unblocks
the native-substrate work in PR 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): split lib.rs into focused modules

Phase 2 of #3118. Make the crate agent-friendly before the
deeper native-substrate changes by splitting the 3801-line
crates/ironclaw_memory/src/lib.rs into 13 focused modules. No
behavior change: every public name keeps its identity, the four
test files under crates/ironclaw_memory/tests/ are untouched, and
all 42 existing tests still pass.

New layout:

  src/lib.rs         (34 LoC)  re-export hub + crate docs
  src/path.rs        scope, path, parsing, validation, error helpers
  src/metadata.rs    DocumentMetadata, .config inheritance
  src/embedding.rs   EmbeddingProvider + vector helpers
  src/schema.rs      JSON schema validation
  src/search.rs      search request/result + RRF/weighted fusion
  src/chunking.rs    ChunkConfig + chunk_document + content_sha256
  src/backend.rs     MemoryBackend + RepositoryMemoryBackend
  src/filesystem.rs  RootFilesystem adapters
  src/indexer.rs     MemoryDocumentIndexer + ChunkingIndexer
  src/repo/mod.rs    MemoryDocumentRepository + shared repo helpers
  src/repo/in_memory.rs
  src/repo/libsql.rs        (cfg(libsql))
  src/repo/postgres.rs      (cfg(postgres))

Visibility rules:

- Public items keep their public names (verified against the
  imports in all four crates/ironclaw_memory/tests/*.rs files).
- Helpers shared across modules become pub(crate).
- libSQL- and Postgres-only schema constants and helpers stay
  inside their respective repo files.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

This split is the foundation for the follow-up PRs that add the
native reborn_memory_* schema, libSQL/Postgres repositories on the
native schema, ported semantic/search/versioning tests, and host
wiring (#3118 phases 3-7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): add native reborn_memory_* schema + empty repo wiring

Phase 3 of #3118, PR 2 of the stack. Add the dedicated
reborn_memory_* substrate that the rest of the stack will build on,
without yet implementing any read/write/list/search behavior.

The native schema lives inside the crate (consistent with the
existing `LibSqlMemoryDocumentRepository::run_migrations` pattern
where the crate manages its own DDL in isolation from the main
app's migration system). The legacy `memory_documents` table is
deliberately untouched — Reborn memory must not depend on or
piggy-back on the legacy schema.

Added:

  src/repo/native_libsql.rs
    - RebornLibSqlMemoryDocumentRepository (compiles, errors on every op)
    - run_migrations() materializes the substrate
    - REBORN_LIBSQL_MEMORY_DOCUMENTS_SCHEMA (4 tables + indexes + triggers)

  src/repo/native_postgres.rs
    - RebornPostgresMemoryDocumentRepository (compiles, errors on every op)
    - run_migrations() materializes the substrate
    - REBORN_POSTGRES_MEMORY_DOCUMENTS_SCHEMA (4 tables + GIN/HNSW indexes)

  tests/reborn_native_schema_contract.rs
    - reborn_libsql_run_migrations_creates_native_substrate_idempotently
    - reborn_libsql_repository_fails_closed_until_behavior_lands

Schema design (matches issue #3118 exactly):

  reborn_memory_documents
    tenant_id TEXT NOT NULL
    user_id   TEXT NOT NULL
    agent_id  TEXT NOT NULL DEFAULT ''   -- empty string is the
    project_id TEXT NOT NULL DEFAULT ''  -- DB-only "absent" sentinel;
    path      TEXT NOT NULL              -- _none stays virtual-path-only
    UNIQUE (tenant_id, user_id, agent_id, project_id, path)

  reborn_memory_chunks         -- chunks with content_hash + optional embedding
  reborn_memory_chunks_fts     -- FTS5 virtual table on content (libSQL)
                               -- TSVECTOR + GIN + HNSW (Postgres)
  reborn_memory_document_versions  -- monotonic version per document, cascade

Indexes cover full-scope lookup, scope+path lookup, list/search by
scope, chunk lookup by document_id, FTS lookup, and version lookup
by (document_id, version DESC).

Repositories return a clear "reborn-native ... is not yet
implemented" `FilesystemError::Backend` for every read/write/list/
search/index operation. Callers fail closed instead of treating an
empty result as authoritative.

Verification (all green, 44 tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native libSQL repository behavior (PR 3 of #3118) (#3182)

* feat(reborn-memory): implement native libSQL repository behavior

Phase 4 of #3118, PR 3 of the stack. Stacks on PR 2 (#3181). Replace
the `not yet implemented` stubs in `RebornLibSqlMemoryDocumentRepository`
with a full read/write/list/search/version/chunk implementation
against the `reborn_memory_*` substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = ?`) is unambiguous and
NULL/IS NULL gymnastics are gone.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinel back to `None`).
- write_document, write_document_with_options:
  BEGIN IMMEDIATE -> reborn_libsql_list_paths_for_scope -> conflict
  check -> insert-or-update; computes content_hash on every write;
  writes prior content to reborn_memory_document_versions if the
  content changed and skip_versioning is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through reborn_memory_chunks_fts
  with full-scope WHERE; vector branch reads embeddings, computes
  cosine, sorts deterministically (path tiebreaker), then RRF /
  weighted-score fusion.
- replace_document_chunks_if_current: re-reads content_hash inside
  the transaction; no-op if it has drifted (prevents corrupting the
  index after a between-read-and-refresh rewrite).
- delete_document_chunks: scoped DELETE.

Reusable helpers added to `repo/mod.rs` and shared by both native
backends:

- REBORN_SCOPE_NONE_SENTINEL ("")
- reborn_agent_id_db_value, reborn_project_id_db_value
- reborn_memory_document_from_row(tenant, user, agent_db, project_db, db_path)

13 new behavioral tests in
`tests/reborn_native_libsql_repository_contract.rs`:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_tenant_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back_for_native_documents
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

The `reborn_libsql_repository_fails_closed_until_behavior_lands`
smoke test from PR 2 is removed (its purpose — proving stubs were
fail-closed before behavior landed — is now obsolete).

Postgres native repository remains stubbed; PR 4 implements it on
the same substrate with a behavioral testcontainer harness.

Verification (all green; 56 memory tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) (#3183)

* feat(reborn-memory): implement native Postgres repository behavior

Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e482).
Replace the `not yet implemented` stubs in
`RebornPostgresMemoryDocumentRepository` with a full
read/write/list/search/version/chunk implementation against the
`reborn_memory_*` Postgres substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and
NULL/IS NULL gymnastics are unnecessary.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinels back to `None`).
- write_document, write_document_with_options:
  BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE ->
  reborn_postgres_list_paths_for_scope -> conflict check ->
  upsert -> optional version row -> COMMIT. Computes content_hash on
  every write; archives prior content into
  `reborn_memory_document_versions` if the content changed and
  `skip_versioning` is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through `reborn_memory_chunks`
  with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector
  branch uses pgvector `<=>` ordering. Both branches are pre-fusion
  ranked; results are fused through the shared
  `search::fuse_memory_search_results`, honoring `FusionStrategy`.
- replace_document_chunks_if_current: SELECT … FOR UPDATE row lock,
  hash drift check (no-op if the document was rewritten under it),
  delete-then-insert chunks with optional pgvector embedding.
- delete_document_chunks: scoped DELETE through
  `reborn_memory_documents` join.
- run_migrations: wrapped in a session-level
  `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent
  processes (parallel test runs) serialize on
  `CREATE EXTENSION pgcrypto/vector`.

Behavioral coverage in
`tests/reborn_native_postgres_repository_contract.rs` (13 tests at
parity with the libSQL contract). Tests use the standard
`DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern
with `try_connect` skipping cleanly when Postgres is unreachable
(matches `tests/workspace_integration.rs`); each test scopes a
unique tenant prefix and cleans up via `cleanup_tenant` so parallel
runs do not collide:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

`plainto_tsquery('english', $5)` already tolerates arbitrary
punctuation, so the Postgres FTS-escape test asserts the same
contract as the libSQL counterpart against the unescaped path.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --features postgres
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `try_connect` so CI without a database
remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): port pure-behavior contract over native repos (PR 5 of #3118) (#3184)

* test(reborn-memory): port pure-behavior contract over native repos

Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a
behavior contract for `RepositoryMemoryBackend` layered over both
`RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`. The semantics under test
live above the bare repository — they belong to the
`RepositoryMemoryBackend` composition (`.config` inheritance, schema
validation, indexer best-effort, capability fail-closed,
embedding-dimension guard).

These behaviors already exist in production code; PR 5 is purely
test coverage that ports the matching contract from the legacy
`db_memory_repository_contract.rs` over to the native
`reborn_memory_*` substrate. No production-code changes.

New file `tests/reborn_native_repository_backend_contract.rs`:

libSQL (10 tests, in-process temp DB):
  - libsql_backend_inherits_config_metadata_for_skip_indexing
  - libsql_backend_closer_config_overrides_parent_config
  - libsql_backend_honors_skip_versioning_from_config
  - libsql_backend_validates_schema_from_config_before_write
  - libsql_backend_reports_write_success_when_indexer_fails_after_persist
  - libsql_backend_search_fails_closed_for_unsupported_vector_search
  - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch
  - libsql_backend_hybrid_search_fuses_full_text_and_vector_results
  - libsql_backend_weighted_score_fusion_orders_results_by_weights
  - libsql_backend_search_honors_limit

Postgres (4 tests, skip-on-unreachable per
`tests/workspace_integration.rs:26-33`; each test scopes a unique
tenant prefix and cleans up via `pg_cleanup_tenant`):
  - postgres_backend_validates_schema_from_config_before_write
  - postgres_backend_honors_skip_versioning_from_config
  - postgres_backend_reports_write_success_when_indexer_fails_after_persist
  - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch

Coverage delta vs. PR 4:

  | Behavior in #3118 phase 6                              | PR 5 |
  |--------------------------------------------------------|------|
  | `.config` metadata inheritance precedence              |  ✓  |
  | Closer `.config` overrides parent `.config`            |  ✓  |
  | `skip_indexing` from `.config`                         |  ✓  |
  | `skip_versioning` from `.config`                       |  ✓  |
  | Schema validation before persistence                   |  ✓  |
  | Indexer failure after persist => write success         |  ✓  |
  | Capability fail-closed for vector search               |  ✓  |
  | Query embedding dimension mismatch fails closed        |  ✓  |
  | RRF fusion (hybrid)                                    |  ✓  |
  | WeightedScore fusion                                   |  ✓  |
  | `limit` honored                                        |  ✓  |
  | FTS query escaping (already covered in PR 3 / PR 4)    | n/a |
  | `version` hash semantics (already covered in PR 3/4)   | n/a |

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185)

* test(reborn-memory): vertical integration through public seams

Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add
caller-facing vertical-integration coverage that drives the full
public-seam stack against the Reborn-native repositories:

  reborn-native repository
    -> ChunkingMemoryDocumentIndexer
    -> RepositoryMemoryBackend
    -> MemoryBackendFilesystemAdapter
    -> CompositeRootFilesystem mounted at /memory

These behaviors already exist in production code; PR 6 is purely
test coverage that proves the Phase 7 invariants hold against the
native substrate. No production-code changes.

`tests/reborn_native_filesystem_vertical_integration.rs` —
public-seam vertical:

libSQL (6 tests, in-process temp DB):
  - libsql_round_trips_authorized_read_write_through_composite_mount
  - libsql_lists_direct_children_through_composite_mount
  - libsql_search_through_composite_mount_returns_only_same_scope_results
  - libsql_capability_denied_search_fails_closed_before_repository_is_called
  - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error
  - libsql_composite_mount_rejects_duplicate_root_at_registration

Postgres (2 tests, skip-on-unreachable; per-tenant cleanup):
  - postgres_round_trips_authorized_read_write_through_composite_mount
  - postgres_search_through_composite_mount_returns_only_same_scope_results

This amend additionally closes thoroughness gaps identified in a
Required-Tests audit of #3118. None of these are production
behavior changes — every gap is a missing assertion on behavior
the production code already implements:

`tests/reborn_native_repository_backend_contract.rs`:
  - Strengthen skip_indexing test to assert chunk-count = 0 in DB
    instead of indexer-call-count = 1 (uses real
    ChunkingMemoryDocumentIndexer that respects skip_indexing).
    Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing`
    -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`.
  - Add `libsql_backend_document_metadata_overrides_inherited_config`:
    parent .config says skip_versioning=true, doc-level metadata
    overrides to false; overwrite must produce a version row.
  - Strengthen weighted-score test to assert ordering changes when
    weights flip (FT-heavy vs vector-heavy produces different top
    result, proving weights actually steer the score).
  - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch`
    asserting result count is bounded by an effective
    pre_fusion_limit.
  - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test
    locking the clamp invariant of `with_pre_fusion_limit`).

`tests/reborn_native_libsql_repository_contract.rs`:
  - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`:
    two `tokio::join!`-launched writes serialize via `BEGIN
    IMMEDIATE`; assert exactly one row in `list_documents`.
  - Add `fts_query_with_only_stopwords_does_not_error` covering the
    issue's "empty/stopword-ish" FTS case.

`tests/reborn_native_postgres_repository_contract.rs`:
  - Add `same_path_in_different_tenants_stores_separate_rows`
    (Postgres parity for the tenant axis the libSQL contract
    already covered).
  - Add Postgres mirrors of the concurrent-writes and stopword-FTS
    tests above.

`src/search.rs`:
  - Add unit tests for `fuse_memory_search_results` deterministic
    tiebreak: tied scores must sort by relative path ascending,
    independent of insertion order.

Coverage delta for issue #3118 "Required tests":

  | Required test (issue)                                        | Status |
  |--------------------------------------------------------------|--------|
  | Same path different tenants/users/agents/projects            |  ✓    |
  | Search/list scope isolation                                  |  ✓    |
  | projects/ prefix as user path                                |  ✓    |
  | Same scope+path upserts                                      |  ✓    |
  | Different scope+same path -> different docs                  |  ✓    |
  | File/directory prefix conflict                               |  ✓    |
  | Concurrent writes -> exactly one row                         |  ✓    |
  | Indexer failure after persist -> write succeeds              |  ✓    |
  | .config applies to children                                  |  ✓    |
  | Closer .config overrides parent                              |  ✓    |
  | Document metadata overrides inherited .config                |  ✓    |
  | skip_indexing -> chunk count = 0                             |  ✓    |
  | skip_versioning -> no version row                            |  ✓    |
  | Schema validation rejects before persistence                 |  ✓    |
  | Write -> chunk -> FTS end-to-end                             |  ✓    |
  | FTS escaping (punct + stopwords)                             |  ✓    |
  | Vector dim mismatch                                          |  ✓    |
  | Hybrid ranking deterministic (path tiebreak)                 |  ✓    |
  | RRF + WeightedScore fusion                                   |  ✓    |
  | limit + pre_fusion_limit                                     |  ✓    |
  | Version monotonic + content hash                             |  ✓    |
  | Adapter / RootFilesystem vertical integration                |  ✓    |
  | Unsupported capability fails closed before side effects      |  ✓    |

Acceptance criteria from #3118 closed by this PR:

  - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` /
        `RootFilesystem`) have vertical integration coverage against
        native repositories.

Verification (all green; second run confirms reliability — first
run hit a pre-existing parallel-execution flake in the legacy
`db_memory_repository_contract` target, unrelated to this PR; passes
in isolation and on re-run):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3185 review findings

Three medium-severity issues raised on the vertical integration PR:

1. Postgres vertical tests in reborn_native_filesystem_vertical_integration
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported them as passed even when Postgres was
   unreachable. Replace the silent-skip helper with `pg_require_connection`
   that panics with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the
   `ironclaw_memory` guardrail that Postgres behavioral coverage must
   be real, not compile/skip coverage.

2. The Postgres vertical search test built `RepositoryMemoryBackend`
   without a `ChunkingMemoryDocumentIndexer`, so writes through
   `CompositeRootFilesystem` never populated `reborn_memory_chunks`.
   Search returned an empty Vec, and the assertion
   `paths.iter().all(|p| p == "visible.md")` was vacuously true even
   without proving anything about the write -> index -> search flow.
   Wire the same chunking indexer the libSQL vertical stack uses
   (FTS-only — no embedding provider), and add an explicit non-empty
   assertion before the scope check. Verified end-to-end against a
   `pgvector/pgvector:pg16` container.

3. The `pre_fusion_limit_is_clamped_up_to_limit` test in
   reborn_native_repository_backend_contract claimed the invariant
   `pre_fusion_limit >= limit` held regardless of caller order, but
   only exercised `with_limit().with_pre_fusion_limit()`. The reverse
   case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left
   `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch
   SQL `LIMIT` shrink below the requested final limit. Make
   `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order
   regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3184 review findings

Three medium-severity issues raised on the behavior-contract PR:

1. Postgres backend tests in reborn_native_repository_backend_contract
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported all Postgres tests as passed without
   running migrations or exercising any of the schema validation,
   versioning, indexer, or search behavior against Postgres. Replace
   the silent-skip helper with `pg_require_connection` that panics
   with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring
   the `ironclaw_memory` guardrail that Postgres repository coverage
   must be real, not compile/skip coverage.

2. The `libsql_backend_search_honors_limit` and
   `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests
   only asserted `results.len() <= 2`, which is vacuously satisfied
   by 0 or 1 results — a regression that broke FTS or indexing
   entirely would still pass. Establish the precondition with a
   larger-limit baseline search asserting the full match count is
   visible, then assert the bounded search returns exactly 2.

3. The new search/fusion/limit contract was libSQL-only, while
   `RebornPostgresMemoryDocumentRepository` has its own FTS
   (`tsvector`) and pgvector query implementations. Add Postgres
   counterparts for hybrid search, weighted-score fusion,
   pre-fusion-limit per-branch, and the `limit` truncation cases.
   The Postgres weighted-score test uses asymmetric FTS content
   (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks
   the FTS leader unambiguously above the hybrid match — without
   this, `ts_rank_cd` ties non-deterministically. Generalize
   `RecordingEmbeddingProvider` to be parametric on `DIM` so the
   same provider semantics work against libSQL (DIM=3, blob
   embeddings) and Postgres (DIM=1536, fixed-width pgvector column).

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 105 memory tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3183 review findings

Two review issues raised on the native Postgres repository PR:

1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional
   `delete_document_chunks` for both `skip_indexing == Some(true)` and
   the empty-chunks case. A stale reindex (older bytes, older
   skip_indexing flag) racing with a concurrent writer that has
   already produced fresh chunks for newer content could clobber the
   newer write's chunk rows, leaving the latest content unsearchable.

   Compute `content_hash_at_read` up front and route both paths
   through the same hash-guarded `replace_document_chunks_if_current`
   call. The hash guard turns a stale clear into a no-op once the
   document has moved on. `delete_document_chunks` is no longer
   reached by the indexer; the trait method stays in place to avoid
   churning the API across the stack.

   Add 3 regression tests in `mod tests` covering the skip_indexing
   path, the empty-chunks path, and the non-empty path, each
   asserting routing through `replace_document_chunks_if_current`
   with the read-time content hash.

2. The Postgres contract harness silently skipped every test when the
   pool could not hand out a connection — the binary reported all
   tests as passed without running migrations or exercising any
   read/write/search/chunk/version behavior, violating the
   `ironclaw_memory` guardrail that Postgres coverage must be real.
   Make `try_connect` panic with an actionable message unless the
   new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set.

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 108 memory tests pass, zero clippy warnings, fmt clean. Verified
fail-loud and opt-in-skip behavior with and without DATABASE_URL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): remove unguarded delete_document_chunks API

Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous
fix routed the indexer through `replace_document_chunks_if_current` so
the live race the reviewer described (stale reindex clobbering chunks
from a newer write) was closed, but the unguarded
`delete_document_chunks` method was still on the trait and in 4
implementations as a footgun for any future caller.

Remove it entirely:

- Drop the trait method from `MemoryDocumentIndexRepository` and
  document at the trait level that chunk clearing is folded into
  `replace_document_chunks_if_current` (called with an empty `chunks`
  slice). A hash-guarded clear is the only safe shape here.
- Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`,
  `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the
  `libsql_document_id_and_content` helper that became dead with the
  legacy libsql impl.
- Remove the test mock's stub and the `IndexerCall::Delete` variant
  in the indexer regression tests; the recording repo now only
  implements the hash-checked path, so any future regression that
  starts calling something else fails to compile.

Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory
tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3181 review findings

Issue #2 of the two raised on the native-schema PR — silent skip in
the schema contract tests. (Issue #1, fail-closed metadata methods,
was mooted when PR3 squash-merged into this branch's history and
replaced the trait-default Ok stubs with real implementations on
both `RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`.)

The reviewer flagged that `reborn_native_schema_contract.rs` only
defined libSQL tests — running with `--features postgres` compiled
and ran zero tests, leaving the `pgcrypto`/`vector` extensions, the
generated `tsvector` column, the HNSW vector index, and the
`reborn_memory_*` tables as compile-only DDL coverage.

Add `reborn_postgres_run_migrations_creates_native_substrate_idempotently`:

- Uses the now-standard `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env
  var. Without it, an unreachable Postgres panics with an actionable
  message instead of skipping silently.
- Calls `run_migrations()` twice to lock in idempotency.
- Verifies both required extensions are installed.
- Verifies the three `reborn_memory_*` tables exist.
- Specifically verifies the generated `content_tsv` column has type
  `tsvector` and `is_generated = ALWAYS`.
- Specifically verifies the `idx_reborn_memory_chunks_embedding`
  index uses HNSW.
- Re-asserts the legacy `memory_documents` table is not created.

Verified end-to-end against `pgvector/pgvector:pg16`: both schema
tests pass, all 109 memory tests pass, zero clippy warnings, fmt
clean. Verified the fail-loud-without-DB and opt-in-skip paths
explicitly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
serrrfirat added a commit that referenced this pull request May 6, 2026
…plit (PR 1 of #3118) (#3180)

* docs(reborn-memory): reorient guardrails to native-isolated direction

Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112
and asks for a native, isolated Reborn memory subsystem with explicit
tenant/user/agent/project scope columns rather than synthetic
workspace user_id strings.

Update crates/ironclaw_memory/CLAUDE.md to reflect that decision:

- Drop reuse-first / preserve-existing-tables wording.
- State that src/workspace is reference material only and that
  ironclaw_memory must not depend on the main app crate.
- Spell out the full (tenant, user, agent, project, path) scope
  invariant on every read/list/search/write/version/chunk operation.
- Document the empty-string DB sentinel for absent agent/project
  columns and call out that _none is the virtual-path-only sentinel.
- Note that legacy migration/coexistence is explicitly deferred and
  that Postgres needs real (not compile-only) behavioral coverage.

No code changes; this PR is purely doc reorientation that unblocks
the native-substrate work in PR 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): split lib.rs into focused modules

Phase 2 of #3118. Make the crate agent-friendly before the
deeper native-substrate changes by splitting the 3801-line
crates/ironclaw_memory/src/lib.rs into 13 focused modules. No
behavior change: every public name keeps its identity, the four
test files under crates/ironclaw_memory/tests/ are untouched, and
all 42 existing tests still pass.

New layout:

  src/lib.rs         (34 LoC)  re-export hub + crate docs
  src/path.rs        scope, path, parsing, validation, error helpers
  src/metadata.rs    DocumentMetadata, .config inheritance
  src/embedding.rs   EmbeddingProvider + vector helpers
  src/schema.rs      JSON schema validation
  src/search.rs      search request/result + RRF/weighted fusion
  src/chunking.rs    ChunkConfig + chunk_document + content_sha256
  src/backend.rs     MemoryBackend + RepositoryMemoryBackend
  src/filesystem.rs  RootFilesystem adapters
  src/indexer.rs     MemoryDocumentIndexer + ChunkingIndexer
  src/repo/mod.rs    MemoryDocumentRepository + shared repo helpers
  src/repo/in_memory.rs
  src/repo/libsql.rs        (cfg(libsql))
  src/repo/postgres.rs      (cfg(postgres))

Visibility rules:

- Public items keep their public names (verified against the
  imports in all four crates/ironclaw_memory/tests/*.rs files).
- Helpers shared across modules become pub(crate).
- libSQL- and Postgres-only schema constants and helpers stay
  inside their respective repo files.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

This split is the foundation for the follow-up PRs that add the
native reborn_memory_* schema, libSQL/Postgres repositories on the
native schema, ported semantic/search/versioning tests, and host
wiring (#3118 phases 3-7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3180 review findings

Three medium-severity issues raised on the split refactor:

1. search.rs: `min_score` was filtering raw RRF/weighted scores before
   the later normalization pass. With default RRF k=60 the top single
   hit's raw score is ~0.016, so a caller-supplied `min_score=0.5`
   would drop every result before normalization could lift the best
   hit to 1.0. Move filtering to after normalization, matching the
   workspace fusion contract; add 3 regression tests covering RRF,
   weighted, and `min_score=0`.

2. indexer.rs: empty/whitespace documents routed through the
   unconditional `delete_document_chunks`, which races with a
   concurrent writer that has already produced fresh chunks for newer
   content. Route empty chunk sets through the same hash-checked
   `replace_document_chunks_if_current` path used for non-empty sets;
   the hash guard makes a stale delete a no-op once the document has
   moved on. Add 2 regression tests using a recording mock.

3. backend.rs: `RepositoryMemoryBackend` ignored its host-resolved
   `MemoryContext` and forwarded the separately supplied path/scope
   to the repository, so a direct caller of this public seam that
   authorized one context but passed a path or scope for a different
   tenant/user/agent/project would bypass the boundary. Add
   `ensure_path_matches_context` / `ensure_scope_matches_context`
   guards before any repository side effect on read, write,
   compare-and-append, and list. Add 5 regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native schema + empty repo wiring (PR 2 of #3118) (#3181)

* docs(reborn-memory): reorient guardrails to native-isolated direction

Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112
and asks for a native, isolated Reborn memory subsystem with explicit
tenant/user/agent/project scope columns rather than synthetic
workspace user_id strings.

Update crates/ironclaw_memory/CLAUDE.md to reflect that decision:

- Drop reuse-first / preserve-existing-tables wording.
- State that src/workspace is reference material only and that
  ironclaw_memory must not depend on the main app crate.
- Spell out the full (tenant, user, agent, project, path) scope
  invariant on every read/list/search/write/version/chunk operation.
- Document the empty-string DB sentinel for absent agent/project
  columns and call out that _none is the virtual-path-only sentinel.
- Note that legacy migration/coexistence is explicitly deferred and
  that Postgres needs real (not compile-only) behavioral coverage.

No code changes; this PR is purely doc reorientation that unblocks
the native-substrate work in PR 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): split lib.rs into focused modules

Phase 2 of #3118. Make the crate agent-friendly before the
deeper native-substrate changes by splitting the 3801-line
crates/ironclaw_memory/src/lib.rs into 13 focused modules. No
behavior change: every public name keeps its identity, the four
test files under crates/ironclaw_memory/tests/ are untouched, and
all 42 existing tests still pass.

New layout:

  src/lib.rs         (34 LoC)  re-export hub + crate docs
  src/path.rs        scope, path, parsing, validation, error helpers
  src/metadata.rs    DocumentMetadata, .config inheritance
  src/embedding.rs   EmbeddingProvider + vector helpers
  src/schema.rs      JSON schema validation
  src/search.rs      search request/result + RRF/weighted fusion
  src/chunking.rs    ChunkConfig + chunk_document + content_sha256
  src/backend.rs     MemoryBackend + RepositoryMemoryBackend
  src/filesystem.rs  RootFilesystem adapters
  src/indexer.rs     MemoryDocumentIndexer + ChunkingIndexer
  src/repo/mod.rs    MemoryDocumentRepository + shared repo helpers
  src/repo/in_memory.rs
  src/repo/libsql.rs        (cfg(libsql))
  src/repo/postgres.rs      (cfg(postgres))

Visibility rules:

- Public items keep their public names (verified against the
  imports in all four crates/ironclaw_memory/tests/*.rs files).
- Helpers shared across modules become pub(crate).
- libSQL- and Postgres-only schema constants and helpers stay
  inside their respective repo files.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

This split is the foundation for the follow-up PRs that add the
native reborn_memory_* schema, libSQL/Postgres repositories on the
native schema, ported semantic/search/versioning tests, and host
wiring (#3118 phases 3-7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): add native reborn_memory_* schema + empty repo wiring

Phase 3 of #3118, PR 2 of the stack. Add the dedicated
reborn_memory_* substrate that the rest of the stack will build on,
without yet implementing any read/write/list/search behavior.

The native schema lives inside the crate (consistent with the
existing `LibSqlMemoryDocumentRepository::run_migrations` pattern
where the crate manages its own DDL in isolation from the main
app's migration system). The legacy `memory_documents` table is
deliberately untouched — Reborn memory must not depend on or
piggy-back on the legacy schema.

Added:

  src/repo/native_libsql.rs
    - RebornLibSqlMemoryDocumentRepository (compiles, errors on every op)
    - run_migrations() materializes the substrate
    - REBORN_LIBSQL_MEMORY_DOCUMENTS_SCHEMA (4 tables + indexes + triggers)

  src/repo/native_postgres.rs
    - RebornPostgresMemoryDocumentRepository (compiles, errors on every op)
    - run_migrations() materializes the substrate
    - REBORN_POSTGRES_MEMORY_DOCUMENTS_SCHEMA (4 tables + GIN/HNSW indexes)

  tests/reborn_native_schema_contract.rs
    - reborn_libsql_run_migrations_creates_native_substrate_idempotently
    - reborn_libsql_repository_fails_closed_until_behavior_lands

Schema design (matches issue #3118 exactly):

  reborn_memory_documents
    tenant_id TEXT NOT NULL
    user_id   TEXT NOT NULL
    agent_id  TEXT NOT NULL DEFAULT ''   -- empty string is the
    project_id TEXT NOT NULL DEFAULT ''  -- DB-only "absent" sentinel;
    path      TEXT NOT NULL              -- _none stays virtual-path-only
    UNIQUE (tenant_id, user_id, agent_id, project_id, path)

  reborn_memory_chunks         -- chunks with content_hash + optional embedding
  reborn_memory_chunks_fts     -- FTS5 virtual table on content (libSQL)
                               -- TSVECTOR + GIN + HNSW (Postgres)
  reborn_memory_document_versions  -- monotonic version per document, cascade

Indexes cover full-scope lookup, scope+path lookup, list/search by
scope, chunk lookup by document_id, FTS lookup, and version lookup
by (document_id, version DESC).

Repositories return a clear "reborn-native ... is not yet
implemented" `FilesystemError::Backend` for every read/write/list/
search/index operation. Callers fail closed instead of treating an
empty result as authoritative.

Verification (all green, 44 tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native libSQL repository behavior (PR 3 of #3118) (#3182)

* feat(reborn-memory): implement native libSQL repository behavior

Phase 4 of #3118, PR 3 of the stack. Stacks on PR 2 (#3181). Replace
the `not yet implemented` stubs in `RebornLibSqlMemoryDocumentRepository`
with a full read/write/list/search/version/chunk implementation
against the `reborn_memory_*` substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = ?`) is unambiguous and
NULL/IS NULL gymnastics are gone.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinel back to `None`).
- write_document, write_document_with_options:
  BEGIN IMMEDIATE -> reborn_libsql_list_paths_for_scope -> conflict
  check -> insert-or-update; computes content_hash on every write;
  writes prior content to reborn_memory_document_versions if the
  content changed and skip_versioning is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through reborn_memory_chunks_fts
  with full-scope WHERE; vector branch reads embeddings, computes
  cosine, sorts deterministically (path tiebreaker), then RRF /
  weighted-score fusion.
- replace_document_chunks_if_current: re-reads content_hash inside
  the transaction; no-op if it has drifted (prevents corrupting the
  index after a between-read-and-refresh rewrite).
- delete_document_chunks: scoped DELETE.

Reusable helpers added to `repo/mod.rs` and shared by both native
backends:

- REBORN_SCOPE_NONE_SENTINEL ("")
- reborn_agent_id_db_value, reborn_project_id_db_value
- reborn_memory_document_from_row(tenant, user, agent_db, project_db, db_path)

13 new behavioral tests in
`tests/reborn_native_libsql_repository_contract.rs`:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_tenant_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back_for_native_documents
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

The `reborn_libsql_repository_fails_closed_until_behavior_lands`
smoke test from PR 2 is removed (its purpose — proving stubs were
fail-closed before behavior landed — is now obsolete).

Postgres native repository remains stubbed; PR 4 implements it on
the same substrate with a behavioral testcontainer harness.

Verification (all green; 56 memory tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) (#3183)

* feat(reborn-memory): implement native Postgres repository behavior

Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e4821).
Replace the `not yet implemented` stubs in
`RebornPostgresMemoryDocumentRepository` with a full
read/write/list/search/version/chunk implementation against the
`reborn_memory_*` Postgres substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and
NULL/IS NULL gymnastics are unnecessary.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinels back to `None`).
- write_document, write_document_with_options:
  BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE ->
  reborn_postgres_list_paths_for_scope -> conflict check ->
  upsert -> optional version row -> COMMIT. Computes content_hash on
  every write; archives prior content into
  `reborn_memory_document_versions` if the content changed and
  `skip_versioning` is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through `reborn_memory_chunks`
  with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector
  branch uses pgvector `<=>` ordering. Both branches are pre-fusion
  ranked; results are fused through the shared
  `search::fuse_memory_search_results`, honoring `FusionStrategy`.
- replace_document_chunks_if_current: SELECT … FOR UPDATE row lock,
  hash drift check (no-op if the document was rewritten under it),
  delete-then-insert chunks with optional pgvector embedding.
- delete_document_chunks: scoped DELETE through
  `reborn_memory_documents` join.
- run_migrations: wrapped in a session-level
  `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent
  processes (parallel test runs) serialize on
  `CREATE EXTENSION pgcrypto/vector`.

Behavioral coverage in
`tests/reborn_native_postgres_repository_contract.rs` (13 tests at
parity with the libSQL contract). Tests use the standard
`DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern
with `try_connect` skipping cleanly when Postgres is unreachable
(matches `tests/workspace_integration.rs`); each test scopes a
unique tenant prefix and cleans up via `cleanup_tenant` so parallel
runs do not collide:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

`plainto_tsquery('english', $5)` already tolerates arbitrary
punctuation, so the Postgres FTS-escape test asserts the same
contract as the libSQL counterpart against the unescaped path.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --features postgres
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `try_connect` so CI without a database
remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): port pure-behavior contract over native repos (PR 5 of #3118) (#3184)

* test(reborn-memory): port pure-behavior contract over native repos

Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a
behavior contract for `RepositoryMemoryBackend` layered over both
`RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`. The semantics under test
live above the bare repository — they belong to the
`RepositoryMemoryBackend` composition (`.config` inheritance, schema
validation, indexer best-effort, capability fail-closed,
embedding-dimension guard).

These behaviors already exist in production code; PR 5 is purely
test coverage that ports the matching contract from the legacy
`db_memory_repository_contract.rs` over to the native
`reborn_memory_*` substrate. No production-code changes.

New file `tests/reborn_native_repository_backend_contract.rs`:

libSQL (10 tests, in-process temp DB):
  - libsql_backend_inherits_config_metadata_for_skip_indexing
  - libsql_backend_closer_config_overrides_parent_config
  - libsql_backend_honors_skip_versioning_from_config
  - libsql_backend_validates_schema_from_config_before_write
  - libsql_backend_reports_write_success_when_indexer_fails_after_persist
  - libsql_backend_search_fails_closed_for_unsupported_vector_search
  - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch
  - libsql_backend_hybrid_search_fuses_full_text_and_vector_results
  - libsql_backend_weighted_score_fusion_orders_results_by_weights
  - libsql_backend_search_honors_limit

Postgres (4 tests, skip-on-unreachable per
`tests/workspace_integration.rs:26-33`; each test scopes a unique
tenant prefix and cleans up via `pg_cleanup_tenant`):
  - postgres_backend_validates_schema_from_config_before_write
  - postgres_backend_honors_skip_versioning_from_config
  - postgres_backend_reports_write_success_when_indexer_fails_after_persist
  - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch

Coverage delta vs. PR 4:

  | Behavior in #3118 phase 6                              | PR 5 |
  |--------------------------------------------------------|------|
  | `.config` metadata inheritance precedence              |  ✓  |
  | Closer `.config` overrides parent `.config`            |  ✓  |
  | `skip_indexing` from `.config`                         |  ✓  |
  | `skip_versioning` from `.config`                       |  ✓  |
  | Schema validation before persistence                   |  ✓  |
  | Indexer failure after persist => write success         |  ✓  |
  | Capability fail-closed for vector search               |  ✓  |
  | Query embedding dimension mismatch fails closed        |  ✓  |
  | RRF fusion (hybrid)                                    |  ✓  |
  | WeightedScore fusion                                   |  ✓  |
  | `limit` honored                                        |  ✓  |
  | FTS query escaping (already covered in PR 3 / PR 4)    | n/a |
  | `version` hash semantics (already covered in PR 3/4)   | n/a |

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185)

* test(reborn-memory): vertical integration through public seams

Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add
caller-facing vertical-integration coverage that drives the full
public-seam stack against the Reborn-native repositories:

  reborn-native repository
    -> ChunkingMemoryDocumentIndexer
    -> RepositoryMemoryBackend
    -> MemoryBackendFilesystemAdapter
    -> CompositeRootFilesystem mounted at /memory

These behaviors already exist in production code; PR 6 is purely
test coverage that proves the Phase 7 invariants hold against the
native substrate. No production-code changes.

`tests/reborn_native_filesystem_vertical_integration.rs` —
public-seam vertical:

libSQL (6 tests, in-process temp DB):
  - libsql_round_trips_authorized_read_write_through_composite_mount
  - libsql_lists_direct_children_through_composite_mount
  - libsql_search_through_composite_mount_returns_only_same_scope_results
  - libsql_capability_denied_search_fails_closed_before_repository_is_called
  - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error
  - libsql_composite_mount_rejects_duplicate_root_at_registration

Postgres (2 tests, skip-on-unreachable; per-tenant cleanup):
  - postgres_round_trips_authorized_read_write_through_composite_mount
  - postgres_search_through_composite_mount_returns_only_same_scope_results

This amend additionally closes thoroughness gaps identified in a
Required-Tests audit of #3118. None of these are production
behavior changes — every gap is a missing assertion on behavior
the production code already implements:

`tests/reborn_native_repository_backend_contract.rs`:
  - Strengthen skip_indexing test to assert chunk-count = 0 in DB
    instead of indexer-call-count = 1 (uses real
    ChunkingMemoryDocumentIndexer that respects skip_indexing).
    Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing`
    -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`.
  - Add `libsql_backend_document_metadata_overrides_inherited_config`:
    parent .config says skip_versioning=true, doc-level metadata
    overrides to false; overwrite must produce a version row.
  - Strengthen weighted-score test to assert ordering changes when
    weights flip (FT-heavy vs vector-heavy produces different top
    result, proving weights actually steer the score).
  - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch`
    asserting result count is bounded by an effective
    pre_fusion_limit.
  - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test
    locking the clamp invariant of `with_pre_fusion_limit`).

`tests/reborn_native_libsql_repository_contract.rs`:
  - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`:
    two `tokio::join!`-launched writes serialize via `BEGIN
    IMMEDIATE`; assert exactly one row in `list_documents`.
  - Add `fts_query_with_only_stopwords_does_not_error` covering the
    issue's "empty/stopword-ish" FTS case.

`tests/reborn_native_postgres_repository_contract.rs`:
  - Add `same_path_in_different_tenants_stores_separate_rows`
    (Postgres parity for the tenant axis the libSQL contract
    already covered).
  - Add Postgres mirrors of the concurrent-writes and stopword-FTS
    tests above.

`src/search.rs`:
  - Add unit tests for `fuse_memory_search_results` deterministic
    tiebreak: tied scores must sort by relative path ascending,
    independent of insertion order.

Coverage delta for issue #3118 "Required tests":

  | Required test (issue)                                        | Status |
  |--------------------------------------------------------------|--------|
  | Same path different tenants/users/agents/projects            |  ✓    |
  | Search/list scope isolation                                  |  ✓    |
  | projects/ prefix as user path                                |  ✓    |
  | Same scope+path upserts                                      |  ✓    |
  | Different scope+same path -> different docs                  |  ✓    |
  | File/directory prefix conflict                               |  ✓    |
  | Concurrent writes -> exactly one row                         |  ✓    |
  | Indexer failure after persist -> write succeeds              |  ✓    |
  | .config applies to children                                  |  ✓    |
  | Closer .config overrides parent                              |  ✓    |
  | Document metadata overrides inherited .config                |  ✓    |
  | skip_indexing -> chunk count = 0                             |  ✓    |
  | skip_versioning -> no version row                            |  ✓    |
  | Schema validation rejects before persistence                 |  ✓    |
  | Write -> chunk -> FTS end-to-end                             |  ✓    |
  | FTS escaping (punct + stopwords)                             |  ✓    |
  | Vector dim mismatch                                          |  ✓    |
  | Hybrid ranking deterministic (path tiebreak)                 |  ✓    |
  | RRF + WeightedScore fusion                                   |  ✓    |
  | limit + pre_fusion_limit                                     |  ✓    |
  | Version monotonic + content hash                             |  ✓    |
  | Adapter / RootFilesystem vertical integration                |  ✓    |
  | Unsupported capability fails closed before side effects      |  ✓    |

Acceptance criteria from #3118 closed by this PR:

  - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` /
        `RootFilesystem`) have vertical integration coverage against
        native repositories.

Verification (all green; second run confirms reliability — first
run hit a pre-existing parallel-execution flake in the legacy
`db_memory_repository_contract` target, unrelated to this PR; passes
in isolation and on re-run):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3185 review findings

Three medium-severity issues raised on the vertical integration PR:

1. Postgres vertical tests in reborn_native_filesystem_vertical_integration
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported them as passed even when Postgres was
   unreachable. Replace the silent-skip helper with `pg_require_connection`
   that panics with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the
   `ironclaw_memory` guardrail that Postgres behavioral coverage must
   be real, not compile/skip coverage.

2. The Postgres vertical search test built `RepositoryMemoryBackend`
   without a `ChunkingMemoryDocumentIndexer`, so writes through
   `CompositeRootFilesystem` never populated `reborn_memory_chunks`.
   Search returned an empty Vec, and the assertion
   `paths.iter().all(|p| p == "visible.md")` was vacuously true even
   without proving anything about the write -> index -> search flow.
   Wire the same chunking indexer the libSQL vertical stack uses
   (FTS-only — no embedding provider), and add an explicit non-empty
   assertion before the scope check. Verified end-to-end against a
   `pgvector/pgvector:pg16` container.

3. The `pre_fusion_limit_is_clamped_up_to_limit` test in
   reborn_native_repository_backend_contract claimed the invariant
   `pre_fusion_limit >= limit` held regardless of caller order, but
   only exercised `with_limit().with_pre_fusion_limit()`. The reverse
   case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left
   `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch
   SQL `LIMIT` shrink below the requested final limit. Make
   `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order
   regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3184 review findings

Three medium-severity issues raised on the behavior-contract PR:

1. Postgres backend tests in reborn_native_repository_backend_contract
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported all Postgres tests as passed without
   running migrations or exercising any of the schema validation,
   versioning, indexer, or search behavior against Postgres. Replace
   the silent-skip helper with `pg_require_connection` that panics
   with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring
   the `ironclaw_memory` guardrail that Postgres repository coverage
   must be real, not compile/skip coverage.

2. The `libsql_backend_search_honors_limit` and
   `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests
   only asserted `results.len() <= 2`, which is vacuously satisfied
   by 0 or 1 results — a regression that broke FTS or indexing
   entirely would still pass. Establish the precondition with a
   larger-limit baseline search asserting the full match count is
   visible, then assert the bounded search returns exactly 2.

3. The new search/fusion/limit contract was libSQL-only, while
   `RebornPostgresMemoryDocumentRepository` has its own FTS
   (`tsvector`) and pgvector query implementations. Add Postgres
   counterparts for hybrid search, weighted-score fusion,
   pre-fusion-limit per-branch, and the `limit` truncation cases.
   The Postgres weighted-score test uses asymmetric FTS content
   (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks
   the FTS leader unambiguously above the hybrid match — without
   this, `ts_rank_cd` ties non-deterministically. Generalize
   `RecordingEmbeddingProvider` to be parametric on `DIM` so the
   same provider semantics work against libSQL (DIM=3, blob
   embeddings) and Postgres (DIM=1536, fixed-width pgvector column).

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 105 memory tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3183 review findings

Two review issues raised on the native Postgres repository PR:

1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional
   `delete_document_chunks` for both `skip_indexing == Some(true)` and
   the empty-chunks case. A stale reindex (older bytes, older
   skip_indexing flag) racing with a concurrent writer that has
   already produced fresh chunks for newer content could clobber the
   newer write's chunk rows, leaving the latest content unsearchable.

   Compute `content_hash_at_read` up front and route both paths
   through the same hash-guarded `replace_document_chunks_if_current`
   call. The hash guard turns a stale clear into a no-op once the
   document has moved on. `delete_document_chunks` is no longer
   reached by the indexer; the trait method stays in place to avoid
   churning the API across the stack.

   Add 3 regression tests in `mod tests` covering the skip_indexing
   path, the empty-chunks path, and the non-empty path, each
   asserting routing through `replace_document_chunks_if_current`
   with the read-time content hash.

2. The Postgres contract harness silently skipped every test when the
   pool could not hand out a connection — the binary reported all
   tests as passed without running migrations or exercising any
   read/write/search/chunk/version behavior, violating the
   `ironclaw_memory` guardrail that Postgres coverage must be real.
   Make `try_connect` panic with an actionable message unless the
   new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set.

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 108 memory tests pass, zero clippy warnings, fmt clean. Verified
fail-loud and opt-in-skip behavior with and without DATABASE_URL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): remove unguarded delete_document_chunks API

Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous
fix routed the indexer through `replace_document_chunks_if_current` so
the live race the reviewer described (stale reindex clobbering chunks
from a newer write) was closed, but the unguarded
`delete_document_chunks` method was still on the trait and in 4
implementations as a footgun for any future caller.

Remove it entirely:

- Drop the trait method from `MemoryDocumentIndexRepository` and
  document at the trait level that chunk clearing is folded into
  `replace_document_chunks_if_current` (called with an empty `chunks`
  slice). A hash-guarded clear is the only safe shape here.
- Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`,
  `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the
  `libsql_document_id_and_content` helper that became dead with the
  legacy libsql impl.
- Remove the test mock's stub and the `IndexerCall::Delete` variant
  in the indexer regression tests; the recording repo now only
  implements the hash-checked path, so any future regression that
  starts calling something else fails to compile.

Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory
tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3181 review findings

Issue #2 of the two raised on the native-schema PR — silent skip in
the schema contract tests. (Issue #1, fail-closed metadata methods,
was mooted when PR3 squash-merged into this branch's history and
replaced the trait-default Ok stubs with real implementations on
both `RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`.)

The reviewer flagged that `reborn_native_schema_contract.rs` only
defined libSQL tests — running with `--features postgres` compiled
and ran zero tests, leaving the `pgcrypto`/`vector` extensions, the
generated `tsvector` column, the HNSW vector index, and the
`reborn_memory_*` tables as compile-only DDL coverage.

Add `reborn_postgres_run_migrations_creates_native_substrate_idempotently`:

- Uses the now-standard `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env
  var. Without it, an unreachable Postgres panics with an actionable
  message instead of skipping silently.
- Calls `run_migrations()` twice to lock in idempotency.
- Verifies both required extensions are installed.
- Verifies the three `reborn_memory_*` tables exist.
- Specifically verifies the generated `content_tsv` column has type
  `tsvector` and `is_generated = ALWAYS`.
- Specifically verifies the `idx_reborn_memory_chunks_embedding`
  index uses HNSW.
- Re-asserts the legacy `memory_documents` table is not created.

Verified end-to-end against `pgvector/pgvector:pg16`: both schema
tests pass, all 109 memory tests pass, zero clippy warnings, fmt
clean. Verified the fail-loud-without-DB and opt-in-skip paths
explicitly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): native append, version attribution, unbounded vector

Address three high/medium severity unresolved review comments on the
native Reborn memory repositories:

- libSQL/Postgres native repos now implement
  `compare_and_append_document_with_options` directly with hash conflict
  checks, version archival, path-conflict checks on insert, and the same
  transaction/locking the legacy backends use (BEGIN IMMEDIATE on
  libSQL; LOCK TABLE IN SHARE ROW EXCLUSIVE MODE + FOR UPDATE on
  Postgres). Without the override, atomic appends silently fell back to
  the trait default.
- Direct-repo `write_document` now populates `changed_by` from a scoped
  owner key so version rows are never NULL-attributed when callers
  bypass the backend/filesystem seam.
- `reborn_memory_chunks.embedding` is now `vector` (unbounded) instead
  of `vector(1536)` so non-1536 providers (Ollama 768/1024-dim, OpenAI
  3072-dim, Claude 1024-dim, …) can write embeddings. The prior fixed
  dimension was a footgun. Schema contract test asserts the unbounded
  shape and that no HNSW index is reintroduced (HNSW requires fixed
  dimension).

Adds tests at every tier the reviewer asked for: native repository
contract tests (append + changed_by), filesystem vertical-integration
tests that drive `append_file` through the composite mount, and a
Postgres test that writes mixed-dimension embeddings to lock in the
unbounded vector contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): per-scope advisory lock + per-file path class

Address zmanian's two must-fix items on PR #3180 and the seven test gaps
he flagged for follow-up.

Must-fix:

- H2: Replace the table-level `LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE`
  on the Postgres write/append paths with a per-scope advisory lock keyed
  on `hashtext('tenant:user:agent:project')`. Concurrent writes for
  different scopes no longer serialize globally; the unique constraint
  still prevents same-path duplicates and `FOR UPDATE` still pins the
  row. Exposes a latent H1 deadlock between concurrent `run_migrations`
  (DROP TRIGGER → AccessExclusiveLock) and writers (RowExclusiveLock);
  fixed by short-circuiting `run_migrations` when the schema is already
  present and gating the trigger setup behind a `pg_trigger` existence
  check so the table is never re-locked at AccessExclusive level on
  re-runs.
- M2: `PromptProtectedPathClass::as_str()` now maps each default
  protected path to a distinct stable class string (`agents_md`,
  `soul_md`, `heartbeat_md`, …) instead of the single
  `system_prompt_file` bucket. Custom paths fall through to
  `custom_protected_path`; consumers that need the exact path use
  `relative_path()`.

Test gaps:

1. Concurrent `replace_document_chunks_if_current` with the same
   `expected_content_hash` and different chunk sets — exactly one
   writer's set lands, no partial union (libSQL + Postgres).
2. Write/index-time embedding dimension mismatch — provider declares
   one dim, returns another; indexer fails closed before any chunks
   land while the durable document row still persists (libSQL +
   Postgres).
3. `BypassAllowed` path on `empty_prompt_file_clear` actually
   overwrites the prior content with the cleared (empty) content and
   records a `BypassAllowed` audit event; the `require_sink: true` ⇒
   no-sink fail-loud path was already covered by existing tests.
4. Audit event records the *classifying* registry's policy version
   per-path, not always the default — locks in v1/v2 reconciliation
   during a policy bump.
5. Postgres tsvector with CJK content writes cleanly under the
   `'english'` config; English tokens remain searchable alongside CJK
   chunks.
6. Two `RebornPostgresMemoryDocumentRepository` instances on
   independent pools call `run_migrations` concurrently against a
   *cold* schema (per-test isolated Postgres schema) — both succeed
   under `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)`; the schema
   fingerprint count is exactly one.
7. End-to-end search with 60 candidates exercises the per-branch
   `pre_fusion_limit` SQL `LIMIT` cap and asserts a unique
   limit-respecting result set.

Also surfaces tokio_postgres error source chains via a `pg_error_chain`
helper so `FilesystemError::Backend.reason` fields contain SQLSTATE
instead of the opaque `"db error"` string. This was load-bearing for
diagnosing the H1 deadlock during this work and remains useful for
future operators.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(memory): address serrrfirat review — harden reborn memory (#3180)

* fix(reborn-memory): close 5 serrrfirat review items (3 HIGH safety, 2 MED data-destruction)

Each fix has a regression test driving the original bug shape.

HIGH `backend.rs:79` — public forgeable prompt-safety bypass

`MemoryContext::with_prompt_write_safety_enforced()` was `pub`, so any
direct backend caller could construct an "already-enforced" context and
persist high-risk content to SOUL.md/BOOTSTRAP.md through
`RepositoryMemoryBackend::write_document` without ever passing through
policy. Made the builder `pub(crate)` so only the in-crate filesystem
adapter can produce an enforced context, and added an anchor test for
the default-unenforced invariant.

HIGH `filesystem.rs:47` — sink config no longer flips the policy override

`with_prompt_write_safety_event_sink()` was setting
`prompt_safety_config_overridden = true`, which caused the adapter to
take over enforcement and skip a stricter wrapped backend's policy. A
host that wanted only a durable audit sink ended up bypassing the
policy. Sink config is observability and is now scoped to itself; the
override flag is only flipped by the explicit policy/registry builders.
Regression: stricter `AlwaysRejectProtectedPolicy` on the backend
still fires when the adapter only adds a sink.

HIGH `filesystem.rs:196` — adapter must check the path-level enforce result

`adapter_should_enforce_prompt_safety` was using
`!backend_capabilities.prompt_write_safety` instead of
`!backend_will_enforce_protected_path`. A backend that advertised the
capability while reporting `prompt_write_safety_protects_path() ==
false` for adapter-classified protected files (SOUL.md, AGENTS.md, …)
caused the adapter to skip its own check and the backend to do
nothing. Both `write_file` and `append_file` now consult the
path-level result. Regression: a custom backend with capability=true
and protects_path=false is rejected by the adapter for SOUL.md
high-risk content.

MED `native_libsql.rs:801` — LIKE wildcards in path segments

`MemoryDocumentPath` allows `%` and `_` in valid segments, but the
metadata-clear path interpolated the parent prefix raw into a `LIKE
pattern`. A `team_%/.config` write would also clear chunks for
unrelated `team-a/note.md`. Added shared `escape_like_pattern` helper;
both libSQL and Postgres metadata-clear queries now use `LIKE … ESCAPE
'\\'`. Regression on both backends drives `team_%/` vs `team-a/`.

MED `native_libsql.rs:531` — chunk-clear must be gated on row update +
honor descendant overrides

Two related issues:
1. `write_document_metadata` used to run the descendant chunk-clear
   even when the UPDATE matched zero rows. For a missing root `.config`
   the LIKE pattern was bare `%` → every chunk in the scope wiped.
   Now both backends capture `rows_affected` and gate the clear on it.
2. The descendant clear deleted chunks for documents whose own
   document-level metadata explicitly set `skip_indexing=false` (an
   override of the inherited parent .config). Both backends now
   exclude descendants with explicit `skip_indexing=false` via
   `json_extract` (libSQL) and `metadata->>'skip_indexing'` (Postgres).
   Two libSQL regressions cover both fixes.

Already-closed items verified during review

Two MED items zmanian flagged were already addressed by earlier work
in this branch — verified with assertions and an existing test pass:
- MED `backend.rs:342` — direct backend file ops fail closed when
  `file_documents=false` (`ensure_file_documents_supported` covers
  read/write/compare-and-append/list; existing
  `file_document_capability_rejects_direct_backend_file_operations`).
- MED `backend.rs:561` — search path checks `capabilities.embeddings`
  before any `embed_text` call (existing
  `vector_request_fails_closed_when_embedding_generation_is_disabled`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): close remaining 9 serrrfirat review items

Each fix has a regression test driving the bug shape.

MED `native_postgres.rs:985` — vector search bound type cast

The dimension-filtered vector search query bound `$5` without an
explicit `::vector` cast, causing pgvector to fail with "function
vector_dims(unknown) is not unique" because the function is overloaded
on `vector` and `halfvec`. Added `$5::vector` casts on both the
filter and the ORDER BY so the overload resolves cleanly. The
filter itself (already in place) excludes mismatched-dim chunks.
Postgres regression `vector_search_with_mixed_dimension_chunks_does_not_error`
proves a mixed-dim scope returns the matching chunk and excludes the
mismatched one without erroring the whole search.

MED `native_libsql.rs:1232` — FTS backfill regression test

The current code already issues `INSERT INTO reborn_memory_chunks_fts
(reborn_memory_chunks_fts) VALUES ('rebuild')` after migration. Added
a regression that drops the FTS table+triggers, leaves chunk rows
intact, re-runs migrations, and asserts FTS search surfaces the
pre-existing chunks.

MED `indexer.rs:117` — embedding outage degrades to text-only chunks

When `build_chunk_writes` fails (provider outage, dimension mismatch,
…), the indexer used to clear chunk rows then return the error;
backend writes intentionally swallow indexer errors after persistence,
so a successfully-overwritten document disappeared from FTS too.
Indexer now persists *text-only* chunks (embedding = NULL) via the
hash-guarded replace, then surfaces the embedding error for
observability. FTS stays current; vector search degrades. Updated
the existing `embedding_failure_*_with_hash_guard` test to lock in
the new contract; updated the dim-mismatch backend test to assert
chunks have NO embedding rather than no chunk rows.

MED `indexer.rs:122` — race-narrowing for concurrent skip_indexing

Reindex used to read content + metadata, build chunks, and finally
replace chunks guarded only by the content hash. A concurrent
metadata write that flipped `skip_indexing=true` between the initial
resolve and the replace had its chunk-clear undone (content hash
unchanged → reindex re-inserted chunks). Indexer now re-resolves
metadata immediately before the final replace and short-circuits to
an empty chunk set when skip_indexing is now true. New
`FlipsSkipIndexingMidwayRepo` mock + regression
`concurrent_skip_indexing_metadata_write_wins_against_in_flight_reindex`.
A truly race-free fix (atomic metadata-version-checked replace at the
repo layer) is documented as a follow-up.

MED `native_libsql.rs:491` — metadata-only writes invalidate chunks

Trait method `write_document_metadata` already invalidates chunk rows
when the new metadata sets skip_indexing=true (covered by the MED #33
fix). Documented the contract on the trait and added a libSQL
regression `libsql_metadata_write_setting_skip_indexing_true_clears_existing_chunks`
that pins it. Re-enabling indexing (true → false) requires the caller
to drive a reindex; the repo can't run the chunker/embedder itself.

MED `filesystem.rs:245` — backend skips re-rejection of adapter approval

Already addressed: `RepositoryMemoryBackend::write_document/append`
guards on `if !context.prompt_write_safety_enforced()`, and the
adapter sets that flag on the backend context after running its own
enforcement. Added regression
`backend_does_not_re_reject_adapter_approved_warn_or_bypass`
composing an `AlwaysRejectIfRecheckedPolicy` on the backend with a
permissive default policy on the adapter — benign content reaches
the wrapped repository because the backend's policy is short-circuited.

MED `safety.rs:809` — sink errors only fatal when require_sink=true

Already addressed: `emit_prompt_write_safety_event` checks
`parts.require_sink` before converting a sink error into
`PromptWriteSafetyEventUnavailable`. Added regression
`configured_sink_failure_does_not_block_clean_allow_writes` proving an
Allow outcome on a protected path with a failing sink still persists
(only warn/bypass require a durable audit event).

MED `postgres.rs:121` — legacy postgres agent_id binds as Uuid

Already addressed: the legacy schema declares `agent_id UUID` in this
file's `CREATE TABLE IF NOT EXISTS` (matching the V1 migration), and
`scoped_memory_agent_uuid` parses the string into `Option<uuid::Uuid>`
before binding. Added behavioural regression
`postgres_memory_repository_round_trips_agent_scoped_writes_against_uuid_schema`
that creates an agent-scoped path with a real UUID, writes through the
repo, reads it back, and asserts the listing surfaces it — proving the
UUID bindings work against a real Postgres instance.

Bonus close-out

The earlier fix in MED `indexer.rs:117` exposed that one existing
backend test asserted "0 chunk rows after dim mismatch" — the new
contract is "chunk rows exist but with NULL embeddings". Updated the
assertion to count `embedding IS NOT NULL`, which is the actually
load-bearing invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(memory): address reborn review guardrails

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: serrrfirat <f@nuff.tech>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…plit (PR 1 of nearai#3118) (nearai#3180)

* docs(reborn-memory): reorient guardrails to native-isolated direction

Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112
and asks for a native, isolated Reborn memory subsystem with explicit
tenant/user/agent/project scope columns rather than synthetic
workspace user_id strings.

Update crates/ironclaw_memory/CLAUDE.md to reflect that decision:

- Drop reuse-first / preserve-existing-tables wording.
- State that src/workspace is reference material only and that
  ironclaw_memory must not depend on the main app crate.
- Spell out the full (tenant, user, agent, project, path) scope
  invariant on every read/list/search/write/version/chunk operation.
- Document the empty-string DB sentinel for absent agent/project
  columns and call out that _none is the virtual-path-only sentinel.
- Note that legacy migration/coexistence is explicitly deferred and
  that Postgres needs real (not compile-only) behavioral coverage.

No code changes; this PR is purely doc reorientation that unblocks
the native-substrate work in PR 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): split lib.rs into focused modules

Phase 2 of #3118. Make the crate agent-friendly before the
deeper native-substrate changes by splitting the 3801-line
crates/ironclaw_memory/src/lib.rs into 13 focused modules. No
behavior change: every public name keeps its identity, the four
test files under crates/ironclaw_memory/tests/ are untouched, and
all 42 existing tests still pass.

New layout:

  src/lib.rs         (34 LoC)  re-export hub + crate docs
  src/path.rs        scope, path, parsing, validation, error helpers
  src/metadata.rs    DocumentMetadata, .config inheritance
  src/embedding.rs   EmbeddingProvider + vector helpers
  src/schema.rs      JSON schema validation
  src/search.rs      search request/result + RRF/weighted fusion
  src/chunking.rs    ChunkConfig + chunk_document + content_sha256
  src/backend.rs     MemoryBackend + RepositoryMemoryBackend
  src/filesystem.rs  RootFilesystem adapters
  src/indexer.rs     MemoryDocumentIndexer + ChunkingIndexer
  src/repo/mod.rs    MemoryDocumentRepository + shared repo helpers
  src/repo/in_memory.rs
  src/repo/libsql.rs        (cfg(libsql))
  src/repo/postgres.rs      (cfg(postgres))

Visibility rules:

- Public items keep their public names (verified against the
  imports in all four crates/ironclaw_memory/tests/*.rs files).
- Helpers shared across modules become pub(crate).
- libSQL- and Postgres-only schema constants and helpers stay
  inside their respective repo files.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

This split is the foundation for the follow-up PRs that add the
native reborn_memory_* schema, libSQL/Postgres repositories on the
native schema, ported semantic/search/versioning tests, and host
wiring (#3118 phases 3-7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3180 review findings

Three medium-severity issues raised on the split refactor:

1. search.rs: `min_score` was filtering raw RRF/weighted scores before
   the later normalization pass. With default RRF k=60 the top single
   hit's raw score is ~0.016, so a caller-supplied `min_score=0.5`
   would drop every result before normalization could lift the best
   hit to 1.0. Move filtering to after normalization, matching the
   workspace fusion contract; add 3 regression tests covering RRF,
   weighted, and `min_score=0`.

2. indexer.rs: empty/whitespace documents routed through the
   unconditional `delete_document_chunks`, which races with a
   concurrent writer that has already produced fresh chunks for newer
   content. Route empty chunk sets through the same hash-checked
   `replace_document_chunks_if_current` path used for non-empty sets;
   the hash guard makes a stale delete a no-op once the document has
   moved on. Add 2 regression tests using a recording mock.

3. backend.rs: `RepositoryMemoryBackend` ignored its host-resolved
   `MemoryContext` and forwarded the separately supplied path/scope
   to the repository, so a direct caller of this public seam that
   authorized one context but passed a path or scope for a different
   tenant/user/agent/project would bypass the boundary. Add
   `ensure_path_matches_context` / `ensure_scope_matches_context`
   guards before any repository side effect on read, write,
   compare-and-append, and list. Add 5 regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native schema + empty repo wiring (PR 2 of #3118) (#3181)

* docs(reborn-memory): reorient guardrails to native-isolated direction

Issue #3118 supersedes the WorkspaceMemoryAdapter direction in #3112
and asks for a native, isolated Reborn memory subsystem with explicit
tenant/user/agent/project scope columns rather than synthetic
workspace user_id strings.

Update crates/ironclaw_memory/CLAUDE.md to reflect that decision:

- Drop reuse-first / preserve-existing-tables wording.
- State that src/workspace is reference material only and that
  ironclaw_memory must not depend on the main app crate.
- Spell out the full (tenant, user, agent, project, path) scope
  invariant on every read/list/search/write/version/chunk operation.
- Document the empty-string DB sentinel for absent agent/project
  columns and call out that _none is the virtual-path-only sentinel.
- Note that legacy migration/coexistence is explicitly deferred and
  that Postgres needs real (not compile-only) behavioral coverage.

No code changes; this PR is purely doc reorientation that unblocks
the native-substrate work in PR 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): split lib.rs into focused modules

Phase 2 of #3118. Make the crate agent-friendly before the
deeper native-substrate changes by splitting the 3801-line
crates/ironclaw_memory/src/lib.rs into 13 focused modules. No
behavior change: every public name keeps its identity, the four
test files under crates/ironclaw_memory/tests/ are untouched, and
all 42 existing tests still pass.

New layout:

  src/lib.rs         (34 LoC)  re-export hub + crate docs
  src/path.rs        scope, path, parsing, validation, error helpers
  src/metadata.rs    DocumentMetadata, .config inheritance
  src/embedding.rs   EmbeddingProvider + vector helpers
  src/schema.rs      JSON schema validation
  src/search.rs      search request/result + RRF/weighted fusion
  src/chunking.rs    ChunkConfig + chunk_document + content_sha256
  src/backend.rs     MemoryBackend + RepositoryMemoryBackend
  src/filesystem.rs  RootFilesystem adapters
  src/indexer.rs     MemoryDocumentIndexer + ChunkingIndexer
  src/repo/mod.rs    MemoryDocumentRepository + shared repo helpers
  src/repo/in_memory.rs
  src/repo/libsql.rs        (cfg(libsql))
  src/repo/postgres.rs      (cfg(postgres))

Visibility rules:

- Public items keep their public names (verified against the
  imports in all four crates/ironclaw_memory/tests/*.rs files).
- Helpers shared across modules become pub(crate).
- libSQL- and Postgres-only schema constants and helpers stay
  inside their respective repo files.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

This split is the foundation for the follow-up PRs that add the
native reborn_memory_* schema, libSQL/Postgres repositories on the
native schema, ported semantic/search/versioning tests, and host
wiring (#3118 phases 3-7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): add native reborn_memory_* schema + empty repo wiring

Phase 3 of #3118, PR 2 of the stack. Add the dedicated
reborn_memory_* substrate that the rest of the stack will build on,
without yet implementing any read/write/list/search behavior.

The native schema lives inside the crate (consistent with the
existing `LibSqlMemoryDocumentRepository::run_migrations` pattern
where the crate manages its own DDL in isolation from the main
app's migration system). The legacy `memory_documents` table is
deliberately untouched — Reborn memory must not depend on or
piggy-back on the legacy schema.

Added:

  src/repo/native_libsql.rs
    - RebornLibSqlMemoryDocumentRepository (compiles, errors on every op)
    - run_migrations() materializes the substrate
    - REBORN_LIBSQL_MEMORY_DOCUMENTS_SCHEMA (4 tables + indexes + triggers)

  src/repo/native_postgres.rs
    - RebornPostgresMemoryDocumentRepository (compiles, errors on every op)
    - run_migrations() materializes the substrate
    - REBORN_POSTGRES_MEMORY_DOCUMENTS_SCHEMA (4 tables + GIN/HNSW indexes)

  tests/reborn_native_schema_contract.rs
    - reborn_libsql_run_migrations_creates_native_substrate_idempotently
    - reborn_libsql_repository_fails_closed_until_behavior_lands

Schema design (matches issue #3118 exactly):

  reborn_memory_documents
    tenant_id TEXT NOT NULL
    user_id   TEXT NOT NULL
    agent_id  TEXT NOT NULL DEFAULT ''   -- empty string is the
    project_id TEXT NOT NULL DEFAULT ''  -- DB-only "absent" sentinel;
    path      TEXT NOT NULL              -- _none stays virtual-path-only
    UNIQUE (tenant_id, user_id, agent_id, project_id, path)

  reborn_memory_chunks         -- chunks with content_hash + optional embedding
  reborn_memory_chunks_fts     -- FTS5 virtual table on content (libSQL)
                               -- TSVECTOR + GIN + HNSW (Postgres)
  reborn_memory_document_versions  -- monotonic version per document, cascade

Indexes cover full-scope lookup, scope+path lookup, list/search by
scope, chunk lookup by document_id, FTS lookup, and version lookup
by (document_id, version DESC).

Repositories return a clear "reborn-native ... is not yet
implemented" `FilesystemError::Backend` for every read/write/list/
search/index operation. Callers fail closed instead of treating an
empty result as authoritative.

Verification (all green, 44 tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native libSQL repository behavior (PR 3 of #3118) (#3182)

* feat(reborn-memory): implement native libSQL repository behavior

Phase 4 of #3118, PR 3 of the stack. Stacks on PR 2 (#3181). Replace
the `not yet implemented` stubs in `RebornLibSqlMemoryDocumentRepository`
with a full read/write/list/search/version/chunk implementation
against the `reborn_memory_*` substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = ?`) is unambiguous and
NULL/IS NULL gymnastics are gone.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinel back to `None`).
- write_document, write_document_with_options:
  BEGIN IMMEDIATE -> reborn_libsql_list_paths_for_scope -> conflict
  check -> insert-or-update; computes content_hash on every write;
  writes prior content to reborn_memory_document_versions if the
  content changed and skip_versioning is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through reborn_memory_chunks_fts
  with full-scope WHERE; vector branch reads embeddings, computes
  cosine, sorts deterministically (path tiebreaker), then RRF /
  weighted-score fusion.
- replace_document_chunks_if_current: re-reads content_hash inside
  the transaction; no-op if it has drifted (prevents corrupting the
  index after a between-read-and-refresh rewrite).
- delete_document_chunks: scoped DELETE.

Reusable helpers added to `repo/mod.rs` and shared by both native
backends:

- REBORN_SCOPE_NONE_SENTINEL ("")
- reborn_agent_id_db_value, reborn_project_id_db_value
- reborn_memory_document_from_row(tenant, user, agent_db, project_db, db_path)

13 new behavioral tests in
`tests/reborn_native_libsql_repository_contract.rs`:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_tenant_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back_for_native_documents
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

The `reborn_libsql_repository_fails_closed_until_behavior_lands`
smoke test from PR 2 is removed (its purpose — proving stubs were
fail-closed before behavior landed — is now obsolete).

Postgres native repository remains stubbed; PR 4 implements it on
the same substrate with a behavioral testcontainer harness.

Verification (all green; 56 memory tests pass):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reborn-memory): native Postgres repository behavior (PR 4 of #3118) (#3183)

* feat(reborn-memory): implement native Postgres repository behavior

Phase 5 of #3118, PR 4 of the stack. Stacks on PR 3 (5b6e4821).
Replace the `not yet implemented` stubs in
`RebornPostgresMemoryDocumentRepository` with a full
read/write/list/search/version/chunk implementation against the
`reborn_memory_*` Postgres substrate.

Every query carries the full `(tenant_id, user_id, agent_id,
project_id)` scope tuple. Absent agent/project IDs use the
empty-string DB sentinel; the constructor rejects empty / `_none`
user-supplied IDs, so equality (`agent_id = $3`) is unambiguous and
NULL/IS NULL gymnastics are unnecessary.

Implementation:

- read_document, read_document_metadata, list_documents:
  filter by full scope tuple + path; native rows reconstruct via
  `reborn_memory_document_from_row` (which round-trips empty-string
  sentinels back to `None`).
- write_document, write_document_with_options:
  BEGIN -> LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE ->
  reborn_postgres_list_paths_for_scope -> conflict check ->
  upsert -> optional version row -> COMMIT. Computes content_hash on
  every write; archives prior content into
  `reborn_memory_document_versions` if the content changed and
  `skip_versioning` is not set.
- write_document_metadata: scoped UPDATE of metadata column.
- search_documents: FTS branch joins through `reborn_memory_chunks`
  with `ts_rank_cd` over the GIN-indexed `content_tsv`; vector
  branch uses pgvector `<=>` ordering. Both branches are pre-fusion
  ranked; results are fused through the shared
  `search::fuse_memory_search_results`, honoring `FusionStrategy`.
- replace_document_chunks_if_current: SELECT … FOR UPDATE row lock,
  hash drift check (no-op if the document was rewritten under it),
  delete-then-insert chunks with optional pgvector embedding.
- delete_document_chunks: scoped DELETE through
  `reborn_memory_documents` join.
- run_migrations: wrapped in a session-level
  `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)` so concurrent
  processes (parallel test runs) serialize on
  `CREATE EXTENSION pgcrypto/vector`.

Behavioral coverage in
`tests/reborn_native_postgres_repository_contract.rs` (13 tests at
parity with the libSQL contract). Tests use the standard
`DATABASE_URL=postgres://localhost/ironclaw_test` env-var pattern
with `try_connect` skipping cleanly when Postgres is unreachable
(matches `tests/workspace_integration.rs`); each test scopes a
unique tenant prefix and cleans up via `cleanup_tenant` so parallel
runs do not collide:

  - round_trips_a_document_within_full_scope
  - returns_none_when_document_is_missing
  - upsert_replaces_content_for_same_full_scope_and_path
  - full_scope_isolates_user_agent_project_independently
  - top_level_projects_path_is_a_normal_user_path_not_project_scope
  - rejects_file_directory_prefix_conflicts_within_scope
  - writes_metadata_and_reads_it_back
  - write_with_options_creates_version_row_only_when_not_skipped
  - version_numbers_are_monotonic_and_content_hash_matches_archived_content
  - replace_chunks_if_current_is_a_noop_when_document_was_rewritten
  - full_text_search_returns_only_chunks_within_full_scope
  - fts_query_escapes_punctuation_and_handles_empty_input_gracefully
  - full_text_search_uses_rrf_when_only_full_text_branch_returns_results

`plainto_tsquery('english', $5)` already tolerates arbitrary
punctuation, so the Postgres FTS-escape test asserts the same
contract as the libSQL counterpart against the unescaped path.

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --features postgres
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `try_connect` so CI without a database
remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): port pure-behavior contract over native repos (PR 5 of #3118) (#3184)

* test(reborn-memory): port pure-behavior contract over native repos

Phase 6 of #3118, PR 5 of the stack. Stacks on PR 4 (#3183). Add a
behavior contract for `RepositoryMemoryBackend` layered over both
`RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`. The semantics under test
live above the bare repository — they belong to the
`RepositoryMemoryBackend` composition (`.config` inheritance, schema
validation, indexer best-effort, capability fail-closed,
embedding-dimension guard).

These behaviors already exist in production code; PR 5 is purely
test coverage that ports the matching contract from the legacy
`db_memory_repository_contract.rs` over to the native
`reborn_memory_*` substrate. No production-code changes.

New file `tests/reborn_native_repository_backend_contract.rs`:

libSQL (10 tests, in-process temp DB):
  - libsql_backend_inherits_config_metadata_for_skip_indexing
  - libsql_backend_closer_config_overrides_parent_config
  - libsql_backend_honors_skip_versioning_from_config
  - libsql_backend_validates_schema_from_config_before_write
  - libsql_backend_reports_write_success_when_indexer_fails_after_persist
  - libsql_backend_search_fails_closed_for_unsupported_vector_search
  - libsql_backend_search_fails_closed_on_query_embedding_dimension_mismatch
  - libsql_backend_hybrid_search_fuses_full_text_and_vector_results
  - libsql_backend_weighted_score_fusion_orders_results_by_weights
  - libsql_backend_search_honors_limit

Postgres (4 tests, skip-on-unreachable per
`tests/workspace_integration.rs:26-33`; each test scopes a unique
tenant prefix and cleans up via `pg_cleanup_tenant`):
  - postgres_backend_validates_schema_from_config_before_write
  - postgres_backend_honors_skip_versioning_from_config
  - postgres_backend_reports_write_success_when_indexer_fails_after_persist
  - postgres_backend_search_fails_closed_on_query_embedding_dimension_mismatch

Coverage delta vs. PR 4:

  | Behavior in #3118 phase 6                              | PR 5 |
  |--------------------------------------------------------|------|
  | `.config` metadata inheritance precedence              |  ✓  |
  | Closer `.config` overrides parent `.config`            |  ✓  |
  | `skip_indexing` from `.config`                         |  ✓  |
  | `skip_versioning` from `.config`                       |  ✓  |
  | Schema validation before persistence                   |  ✓  |
  | Indexer failure after persist => write success         |  ✓  |
  | Capability fail-closed for vector search               |  ✓  |
  | Query embedding dimension mismatch fails closed        |  ✓  |
  | RRF fusion (hybrid)                                    |  ✓  |
  | WeightedScore fusion                                   |  ✓  |
  | `limit` honored                                        |  ✓  |
  | FTS query escaping (already covered in PR 3 / PR 4)    | n/a |
  | `version` hash semantics (already covered in PR 3/4)   | n/a |

Verification (all green):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(reborn-memory): vertical integration through public seams (PR 6 of #3118) (#3185)

* test(reborn-memory): vertical integration through public seams

Phase 7 of #3118, PR 6 of the stack. Stacks on PR 5 (#3184). Add
caller-facing vertical-integration coverage that drives the full
public-seam stack against the Reborn-native repositories:

  reborn-native repository
    -> ChunkingMemoryDocumentIndexer
    -> RepositoryMemoryBackend
    -> MemoryBackendFilesystemAdapter
    -> CompositeRootFilesystem mounted at /memory

These behaviors already exist in production code; PR 6 is purely
test coverage that proves the Phase 7 invariants hold against the
native substrate. No production-code changes.

`tests/reborn_native_filesystem_vertical_integration.rs` —
public-seam vertical:

libSQL (6 tests, in-process temp DB):
  - libsql_round_trips_authorized_read_write_through_composite_mount
  - libsql_lists_direct_children_through_composite_mount
  - libsql_search_through_composite_mount_returns_only_same_scope_results
  - libsql_capability_denied_search_fails_closed_before_repository_is_called
  - libsql_composite_mount_rejects_invalid_memory_path_with_clean_error
  - libsql_composite_mount_rejects_duplicate_root_at_registration

Postgres (2 tests, skip-on-unreachable; per-tenant cleanup):
  - postgres_round_trips_authorized_read_write_through_composite_mount
  - postgres_search_through_composite_mount_returns_only_same_scope_results

This amend additionally closes thoroughness gaps identified in a
Required-Tests audit of #3118. None of these are production
behavior changes — every gap is a missing assertion on behavior
the production code already implements:

`tests/reborn_native_repository_backend_contract.rs`:
  - Strengthen skip_indexing test to assert chunk-count = 0 in DB
    instead of indexer-call-count = 1 (uses real
    ChunkingMemoryDocumentIndexer that respects skip_indexing).
    Renamed `libsql_backend_inherits_config_metadata_for_skip_indexing`
    -> `libsql_backend_skip_indexing_from_config_writes_zero_chunks_to_db`.
  - Add `libsql_backend_document_metadata_overrides_inherited_config`:
    parent .config says skip_versioning=true, doc-level metadata
    overrides to false; overwrite must produce a version row.
  - Strengthen weighted-score test to assert ordering changes when
    weights flip (FT-heavy vs vector-heavy produces different top
    result, proving weights actually steer the score).
  - Add `libsql_backend_search_honors_pre_fusion_limit_per_branch`
    asserting result count is bounded by an effective
    pre_fusion_limit.
  - Add `pre_fusion_limit_is_clamped_up_to_limit` (a unit test
    locking the clamp invariant of `with_pre_fusion_limit`).

`tests/reborn_native_libsql_repository_contract.rs`:
  - Add `concurrent_writes_under_same_scope_and_path_produce_exactly_one_row`:
    two `tokio::join!`-launched writes serialize via `BEGIN
    IMMEDIATE`; assert exactly one row in `list_documents`.
  - Add `fts_query_with_only_stopwords_does_not_error` covering the
    issue's "empty/stopword-ish" FTS case.

`tests/reborn_native_postgres_repository_contract.rs`:
  - Add `same_path_in_different_tenants_stores_separate_rows`
    (Postgres parity for the tenant axis the libSQL contract
    already covered).
  - Add Postgres mirrors of the concurrent-writes and stopword-FTS
    tests above.

`src/search.rs`:
  - Add unit tests for `fuse_memory_search_results` deterministic
    tiebreak: tied scores must sort by relative path ascending,
    independent of insertion order.

Coverage delta for issue #3118 "Required tests":

  | Required test (issue)                                        | Status |
  |--------------------------------------------------------------|--------|
  | Same path different tenants/users/agents/projects            |  ✓    |
  | Search/list scope isolation                                  |  ✓    |
  | projects/ prefix as user path                                |  ✓    |
  | Same scope+path upserts                                      |  ✓    |
  | Different scope+same path -> different docs                  |  ✓    |
  | File/directory prefix conflict                               |  ✓    |
  | Concurrent writes -> exactly one row                         |  ✓    |
  | Indexer failure after persist -> write succeeds              |  ✓    |
  | .config applies to children                                  |  ✓    |
  | Closer .config overrides parent                              |  ✓    |
  | Document metadata overrides inherited .config                |  ✓    |
  | skip_indexing -> chunk count = 0                             |  ✓    |
  | skip_versioning -> no version row                            |  ✓    |
  | Schema validation rejects before persistence                 |  ✓    |
  | Write -> chunk -> FTS end-to-end                             |  ✓    |
  | FTS escaping (punct + stopwords)                             |  ✓    |
  | Vector dim mismatch                                          |  ✓    |
  | Hybrid ranking deterministic (path tiebreak)                 |  ✓    |
  | RRF + WeightedScore fusion                                   |  ✓    |
  | limit + pre_fusion_limit                                     |  ✓    |
  | Version monotonic + content hash                             |  ✓    |
  | Adapter / RootFilesystem vertical integration                |  ✓    |
  | Unsupported capability fails closed before side effects      |  ✓    |

Acceptance criteria from #3118 closed by this PR:

  - [x] Public Reborn seams (`MemoryBackendFilesystemAdapter` /
        `RootFilesystem`) have vertical integration coverage against
        native repositories.

Verification (all green; second run confirms reliability — first
run hit a pre-existing parallel-execution flake in the legacy
`db_memory_repository_contract` target, unrelated to this PR; passes
in isolation and on re-run):

  cargo fmt --all -- --check
  cargo clippy -p ironclaw_memory --all-targets --all-features -- -D warnings
  cargo clippy --all --tests --examples --all-features -- -D warnings
  cargo test -p ironclaw_memory --features libsql
  cargo test -p ironclaw_memory --all-features
  cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
  python3 scripts/check_no_panics.py --base origin/reborn-integration --head HEAD
  git diff --check origin/reborn-integration

Postgres tests run for real against a local DB when DATABASE_URL is
reachable; otherwise skip via `pg_try_connect` so CI without a
database remains green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3185 review findings

Three medium-severity issues raised on the vertical integration PR:

1. Postgres vertical tests in reborn_native_filesystem_vertical_integration
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported them as passed even when Postgres was
   unreachable. Replace the silent-skip helper with `pg_require_connection`
   that panics with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring the
   `ironclaw_memory` guardrail that Postgres behavioral coverage must
   be real, not compile/skip coverage.

2. The Postgres vertical search test built `RepositoryMemoryBackend`
   without a `ChunkingMemoryDocumentIndexer`, so writes through
   `CompositeRootFilesystem` never populated `reborn_memory_chunks`.
   Search returned an empty Vec, and the assertion
   `paths.iter().all(|p| p == "visible.md")` was vacuously true even
   without proving anything about the write -> index -> search flow.
   Wire the same chunking indexer the libSQL vertical stack uses
   (FTS-only — no embedding provider), and add an explicit non-empty
   assertion before the scope check. Verified end-to-end against a
   `pgvector/pgvector:pg16` container.

3. The `pre_fusion_limit_is_clamped_up_to_limit` test in
   reborn_native_repository_backend_contract claimed the invariant
   `pre_fusion_limit >= limit` held regardless of caller order, but
   only exercised `with_limit().with_pre_fusion_limit()`. The reverse
   case (`with_limit(2).with_pre_fusion_limit(2).with_limit(5)`) left
   `pre_fusion_limit == 2` with `limit == 5`, letting the per-branch
   SQL `LIMIT` shrink below the requested final limit. Make
   `with_limit()` re-clamp `pre_fusion_limit` and add reverse-order
   regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3184 review findings

Three medium-severity issues raised on the behavior-contract PR:

1. Postgres backend tests in reborn_native_repository_backend_contract
   were silently skipped when the pool could not hand out a connection,
   so the test binary reported all Postgres tests as passed without
   running migrations or exercising any of the schema validation,
   versioning, indexer, or search behavior against Postgres. Replace
   the silent-skip helper with `pg_require_connection` that panics
   with an actionable message unless the new
   `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set, restoring
   the `ironclaw_memory` guardrail that Postgres repository coverage
   must be real, not compile/skip coverage.

2. The `libsql_backend_search_honors_limit` and
   `libsql_backend_search_honors_pre_fusion_limit_per_branch` tests
   only asserted `results.len() <= 2`, which is vacuously satisfied
   by 0 or 1 results — a regression that broke FTS or indexing
   entirely would still pass. Establish the precondition with a
   larger-limit baseline search asserting the full match count is
   visible, then assert the bounded search returns exactly 2.

3. The new search/fusion/limit contract was libSQL-only, while
   `RebornPostgresMemoryDocumentRepository` has its own FTS
   (`tsvector`) and pgvector query implementations. Add Postgres
   counterparts for hybrid search, weighted-score fusion,
   pre-fusion-limit per-branch, and the `limit` truncation cases.
   The Postgres weighted-score test uses asymmetric FTS content
   (`fts.md` repeats "literal" four times) so `ts_rank_cd` ranks
   the FTS leader unambiguously above the hybrid match — without
   this, `ts_rank_cd` ties non-deterministically. Generalize
   `RecordingEmbeddingProvider` to be parametric on `DIM` so the
   same provider semantics work against libSQL (DIM=3, blob
   embeddings) and Postgres (DIM=1536, fixed-width pgvector column).

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 105 memory tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3183 review findings

Two review issues raised on the native Postgres repository PR:

1. The shared `ChunkingMemoryDocumentIndexer` called the unconditional
   `delete_document_chunks` for both `skip_indexing == Some(true)` and
   the empty-chunks case. A stale reindex (older bytes, older
   skip_indexing flag) racing with a concurrent writer that has
   already produced fresh chunks for newer content could clobber the
   newer write's chunk rows, leaving the latest content unsearchable.

   Compute `content_hash_at_read` up front and route both paths
   through the same hash-guarded `replace_document_chunks_if_current`
   call. The hash guard turns a stale clear into a no-op once the
   document has moved on. `delete_document_chunks` is no longer
   reached by the indexer; the trait method stays in place to avoid
   churning the API across the stack.

   Add 3 regression tests in `mod tests` covering the skip_indexing
   path, the empty-chunks path, and the non-empty path, each
   asserting routing through `replace_document_chunks_if_current`
   with the read-time content hash.

2. The Postgres contract harness silently skipped every test when the
   pool could not hand out a connection — the binary reported all
   tests as passed without running migrations or exercising any
   read/write/search/chunk/version behavior, violating the
   `ironclaw_memory` guardrail that Postgres coverage must be real.
   Make `try_connect` panic with an actionable message unless the
   new `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env var is set.

Verified end-to-end against a `pgvector/pgvector:pg16` container:
all 108 memory tests pass, zero clippy warnings, fmt clean. Verified
fail-loud and opt-in-skip behavior with and without DATABASE_URL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(reborn-memory): remove unguarded delete_document_chunks API

Follow-up to PR #3183 review on `native_postgres.rs:511`. The previous
fix routed the indexer through `replace_document_chunks_if_current` so
the live race the reviewer described (stale reindex clobbering chunks
from a newer write) was closed, but the unguarded
`delete_document_chunks` method was still on the trait and in 4
implementations as a footgun for any future caller.

Remove it entirely:

- Drop the trait method from `MemoryDocumentIndexRepository` and
  document at the trait level that chunk clearing is folded into
  `replace_document_chunks_if_current` (called with an empty `chunks`
  slice). A hash-guarded clear is the only safe shape here.
- Remove the impls in `repo/libsql.rs`, `repo/postgres.rs`,
  `repo/native_libsql.rs`, `repo/native_postgres.rs`, plus the
  `libsql_document_id_and_content` helper that became dead with the
  legacy libsql impl.
- Remove the test mock's stub and the `IndexerCall::Delete` variant
  in the indexer regression tests; the recording repo now only
  implements the hash-checked path, so any future regression that
  starts calling something else fails to compile.

Verified end-to-end against `pgvector/pgvector:pg16`: all 108 memory
tests pass, zero clippy warnings, fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): address PR #3181 review findings

Issue #2 of the two raised on the native-schema PR — silent skip in
the schema contract tests. (Issue #1, fail-closed metadata methods,
was mooted when PR3 squash-merged into this branch's history and
replaced the trait-default Ok stubs with real implementations on
both `RebornLibSqlMemoryDocumentRepository` and
`RebornPostgresMemoryDocumentRepository`.)

The reviewer flagged that `reborn_native_schema_contract.rs` only
defined libSQL tests — running with `--features postgres` compiled
and ran zero tests, leaving the `pgcrypto`/`vector` extensions, the
generated `tsvector` column, the HNSW vector index, and the
`reborn_memory_*` tables as compile-only DDL coverage.

Add `reborn_postgres_run_migrations_creates_native_substrate_idempotently`:

- Uses the now-standard `IRONCLAW_SKIP_POSTGRES_TESTS=1` opt-in env
  var. Without it, an unreachable Postgres panics with an actionable
  message instead of skipping silently.
- Calls `run_migrations()` twice to lock in idempotency.
- Verifies both required extensions are installed.
- Verifies the three `reborn_memory_*` tables exist.
- Specifically verifies the generated `content_tsv` column has type
  `tsvector` and `is_generated = ALWAYS`.
- Specifically verifies the `idx_reborn_memory_chunks_embedding`
  index uses HNSW.
- Re-asserts the legacy `memory_documents` table is not created.

Verified end-to-end against `pgvector/pgvector:pg16`: both schema
tests pass, all 109 memory tests pass, zero clippy warnings, fmt
clean. Verified the fail-loud-without-DB and opt-in-skip paths
explicitly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): native append, version attribution, unbounded vector

Address three high/medium severity unresolved review comments on the
native Reborn memory repositories:

- libSQL/Postgres native repos now implement
  `compare_and_append_document_with_options` directly with hash conflict
  checks, version archival, path-conflict checks on insert, and the same
  transaction/locking the legacy backends use (BEGIN IMMEDIATE on
  libSQL; LOCK TABLE IN SHARE ROW EXCLUSIVE MODE + FOR UPDATE on
  Postgres). Without the override, atomic appends silently fell back to
  the trait default.
- Direct-repo `write_document` now populates `changed_by` from a scoped
  owner key so version rows are never NULL-attributed when callers
  bypass the backend/filesystem seam.
- `reborn_memory_chunks.embedding` is now `vector` (unbounded) instead
  of `vector(1536)` so non-1536 providers (Ollama 768/1024-dim, OpenAI
  3072-dim, Claude 1024-dim, …) can write embeddings. The prior fixed
  dimension was a footgun. Schema contract test asserts the unbounded
  shape and that no HNSW index is reintroduced (HNSW requires fixed
  dimension).

Adds tests at every tier the reviewer asked for: native repository
contract tests (append + changed_by), filesystem vertical-integration
tests that drive `append_file` through the composite mount, and a
Postgres test that writes mixed-dimension embeddings to lock in the
unbounded vector contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): per-scope advisory lock + per-file path class

Address zmanian's two must-fix items on PR #3180 and the seven test gaps
he flagged for follow-up.

Must-fix:

- H2: Replace the table-level `LOCK TABLE … IN SHARE ROW EXCLUSIVE MODE`
  on the Postgres write/append paths with a per-scope advisory lock keyed
  on `hashtext('tenant:user:agent:project')`. Concurrent writes for
  different scopes no longer serialize globally; the unique constraint
  still prevents same-path duplicates and `FOR UPDATE` still pins the
  row. Exposes a latent H1 deadlock between concurrent `run_migrations`
  (DROP TRIGGER → AccessExclusiveLock) and writers (RowExclusiveLock);
  fixed by short-circuiting `run_migrations` when the schema is already
  present and gating the trigger setup behind a `pg_trigger` existence
  check so the table is never re-locked at AccessExclusive level on
  re-runs.
- M2: `PromptProtectedPathClass::as_str()` now maps each default
  protected path to a distinct stable class string (`agents_md`,
  `soul_md`, `heartbeat_md`, …) instead of the single
  `system_prompt_file` bucket. Custom paths fall through to
  `custom_protected_path`; consumers that need the exact path use
  `relative_path()`.

Test gaps:

1. Concurrent `replace_document_chunks_if_current` with the same
   `expected_content_hash` and different chunk sets — exactly one
   writer's set lands, no partial union (libSQL + Postgres).
2. Write/index-time embedding dimension mismatch — provider declares
   one dim, returns another; indexer fails closed before any chunks
   land while the durable document row still persists (libSQL +
   Postgres).
3. `BypassAllowed` path on `empty_prompt_file_clear` actually
   overwrites the prior content with the cleared (empty) content and
   records a `BypassAllowed` audit event; the `require_sink: true` ⇒
   no-sink fail-loud path was already covered by existing tests.
4. Audit event records the *classifying* registry's policy version
   per-path, not always the default — locks in v1/v2 reconciliation
   during a policy bump.
5. Postgres tsvector with CJK content writes cleanly under the
   `'english'` config; English tokens remain searchable alongside CJK
   chunks.
6. Two `RebornPostgresMemoryDocumentRepository` instances on
   independent pools call `run_migrations` concurrently against a
   *cold* schema (per-test isolated Postgres schema) — both succeed
   under `pg_advisory_lock(REBORN_MIGRATION_LOCK_ID)`; the schema
   fingerprint count is exactly one.
7. End-to-end search with 60 candidates exercises the per-branch
   `pre_fusion_limit` SQL `LIMIT` cap and asserts a unique
   limit-respecting result set.

Also surfaces tokio_postgres error source chains via a `pg_error_chain`
helper so `FilesystemError::Backend.reason` fields contain SQLSTATE
instead of the opaque `"db error"` string. This was load-bearing for
diagnosing the H1 deadlock during this work and remains useful for
future operators.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(memory): address serrrfirat review — harden reborn memory (#3180)

* fix(reborn-memory): close 5 serrrfirat review items (3 HIGH safety, 2 MED data-destruction)

Each fix has a regression test driving the original bug shape.

HIGH `backend.rs:79` — public forgeable prompt-safety bypass

`MemoryContext::with_prompt_write_safety_enforced()` was `pub`, so any
direct backend caller could construct an "already-enforced" context and
persist high-risk content to SOUL.md/BOOTSTRAP.md through
`RepositoryMemoryBackend::write_document` without ever passing through
policy. Made the builder `pub(crate)` so only the in-crate filesystem
adapter can produce an enforced context, and added an anchor test for
the default-unenforced invariant.

HIGH `filesystem.rs:47` — sink config no longer flips the policy override

`with_prompt_write_safety_event_sink()` was setting
`prompt_safety_config_overridden = true`, which caused the adapter to
take over enforcement and skip a stricter wrapped backend's policy. A
host that wanted only a durable audit sink ended up bypassing the
policy. Sink config is observability and is now scoped to itself; the
override flag is only flipped by the explicit policy/registry builders.
Regression: stricter `AlwaysRejectProtectedPolicy` on the backend
still fires when the adapter only adds a sink.

HIGH `filesystem.rs:196` — adapter must check the path-level enforce result

`adapter_should_enforce_prompt_safety` was using
`!backend_capabilities.prompt_write_safety` instead of
`!backend_will_enforce_protected_path`. A backend that advertised the
capability while reporting `prompt_write_safety_protects_path() ==
false` for adapter-classified protected files (SOUL.md, AGENTS.md, …)
caused the adapter to skip its own check and the backend to do
nothing. Both `write_file` and `append_file` now consult the
path-level result. Regression: a custom backend with capability=true
and protects_path=false is rejected by the adapter for SOUL.md
high-risk content.

MED `native_libsql.rs:801` — LIKE wildcards in path segments

`MemoryDocumentPath` allows `%` and `_` in valid segments, but the
metadata-clear path interpolated the parent prefix raw into a `LIKE
pattern`. A `team_%/.config` write would also clear chunks for
unrelated `team-a/note.md`. Added shared `escape_like_pattern` helper;
both libSQL and Postgres metadata-clear queries now use `LIKE … ESCAPE
'\\'`. Regression on both backends drives `team_%/` vs `team-a/`.

MED `native_libsql.rs:531` — chunk-clear must be gated on row update +
honor descendant overrides

Two related issues:
1. `write_document_metadata` used to run the descendant chunk-clear
   even when the UPDATE matched zero rows. For a missing root `.config`
   the LIKE pattern was bare `%` → every chunk in the scope wiped.
   Now both backends capture `rows_affected` and gate the clear on it.
2. The descendant clear deleted chunks for documents whose own
   document-level metadata explicitly set `skip_indexing=false` (an
   override of the inherited parent .config). Both backends now
   exclude descendants with explicit `skip_indexing=false` via
   `json_extract` (libSQL) and `metadata->>'skip_indexing'` (Postgres).
   Two libSQL regressions cover both fixes.

Already-closed items verified during review

Two MED items zmanian flagged were already addressed by earlier work
in this branch — verified with assertions and an existing test pass:
- MED `backend.rs:342` — direct backend file ops fail closed when
  `file_documents=false` (`ensure_file_documents_supported` covers
  read/write/compare-and-append/list; existing
  `file_document_capability_rejects_direct_backend_file_operations`).
- MED `backend.rs:561` — search path checks `capabilities.embeddings`
  before any `embed_text` call (existing
  `vector_request_fails_closed_when_embedding_generation_is_disabled`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn-memory): close remaining 9 serrrfirat review items

Each fix has a regression test driving the bug shape.

MED `native_postgres.rs:985` — vector search bound type cast

The dimension-filtered vector search query bound `$5` without an
explicit `::vector` cast, causing pgvector to fail with "function
vector_dims(unknown) is not unique" because the function is overloaded
on `vector` and `halfvec`. Added `$5::vector` casts on both the
filter and the ORDER BY so the overload resolves cleanly. The
filter itself (already in place) excludes mismatched-dim chunks.
Postgres regression `vector_search_with_mixed_dimension_chunks_does_not_error`
proves a mixed-dim scope returns the matching chunk and excludes the
mismatched one without erroring the whole search.

MED `native_libsql.rs:1232` — FTS backfill regression test

The current code already issues `INSERT INTO reborn_memory_chunks_fts
(reborn_memory_chunks_fts) VALUES ('rebuild')` after migration. Added
a regression that drops the FTS table+triggers, leaves chunk rows
intact, re-runs migrations, and asserts FTS search surfaces the
pre-existing chunks.

MED `indexer.rs:117` — embedding outage degrades to text-only chunks

When `build_chunk_writes` fails (provider outage, dimension mismatch,
…), the indexer used to clear chunk rows then return the error;
backend writes intentionally swallow indexer errors after persistence,
so a successfully-overwritten document disappeared from FTS too.
Indexer now persists *text-only* chunks (embedding = NULL) via the
hash-guarded replace, then surfaces the embedding error for
observability. FTS stays current; vector search degrades. Updated
the existing `embedding_failure_*_with_hash_guard` test to lock in
the new contract; updated the dim-mismatch backend test to assert
chunks have NO embedding rather than no chunk rows.

MED `indexer.rs:122` — race-narrowing for concurrent skip_indexing

Reindex used to read content + metadata, build chunks, and finally
replace chunks guarded only by the content hash. A concurrent
metadata write that flipped `skip_indexing=true` between the initial
resolve and the replace had its chunk-clear undone (content hash
unchanged → reindex re-inserted chunks). Indexer now re-resolves
metadata immediately before the final replace and short-circuits to
an empty chunk set when skip_indexing is now true. New
`FlipsSkipIndexingMidwayRepo` mock + regression
`concurrent_skip_indexing_metadata_write_wins_against_in_flight_reindex`.
A truly race-free fix (atomic metadata-version-checked replace at the
repo layer) is documented as a follow-up.

MED `native_libsql.rs:491` — metadata-only writes invalidate chunks

Trait method `write_document_metadata` already invalidates chunk rows
when the new metadata sets skip_indexing=true (covered by the MED #33
fix). Documented the contract on the trait and added a libSQL
regression `libsql_metadata_write_setting_skip_indexing_true_clears_existing_chunks`
that pins it. Re-enabling indexing (true → false) requires the caller
to drive a reindex; the repo can't run the chunker/embedder itself.

MED `filesystem.rs:245` — backend skips re-rejection of adapter approval

Already addressed: `RepositoryMemoryBackend::write_document/append`
guards on `if !context.prompt_write_safety_enforced()`, and the
adapter sets that flag on the backend context after running its own
enforcement. Added regression
`backend_does_not_re_reject_adapter_approved_warn_or_bypass`
composing an `AlwaysRejectIfRecheckedPolicy` on the backend with a
permissive default policy on the adapter — benign content reaches
the wrapped repository because the backend's policy is short-circuited.

MED `safety.rs:809` — sink errors only fatal when require_sink=true

Already addressed: `emit_prompt_write_safety_event` checks
`parts.require_sink` before converting a sink error into
`PromptWriteSafetyEventUnavailable`. Added regression
`configured_sink_failure_does_not_block_clean_allow_writes` proving an
Allow outcome on a protected path with a failing sink still persists
(only warn/bypass require a durable audit event).

MED `postgres.rs:121` — legacy postgres agent_id binds as Uuid

Already addressed: the legacy schema declares `agent_id UUID` in this
file's `CREATE TABLE IF NOT EXISTS` (matching the V1 migration), and
`scoped_memory_agent_uuid` parses the string into `Option<uuid::Uuid>`
before binding. Added behavioural regression
`postgres_memory_repository_round_trips_agent_scoped_writes_against_uuid_schema`
that creates an agent-scoped path with a real UUID, writes through the
repo, reads it back, and asserts the listing surfaces it — proving the
UUID bindings work against a real Postgres instance.

Bonus close-out

The earlier fix in MED `indexer.rs:117` exposed that one existing
backend test asserted "0 chunk rows after dim mismatch" — the new
contract is "chunk rows exist but with NULL embeddings". Updated the
assertion to count `embedding IS NOT NULL`, which is the actually
load-bearing invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(memory): address reborn review guardrails

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: serrrfirat <f@nuff.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants