feat: memory scope+visibility contract v1 + embedder default alignment, drift-tested (#28)

[OriNachum]

Summary

Lands the eidetic-cli slices for colleague#291's integration arc (issue #28, slices S9+S2): docs/contract.md, a memory scope+visibility convention (v1) that every consumer of the eidetic memory store — the vendored recall/remember skill wrappers here, colleague's own colleague/memory.py runtime, or a future direct CLI consumer — now pins to (a personal scope per repo named by culture.yaml's suffix, and a public default visibility for in-repo team-shared records). It fixes a live disagreement: the vendored wrapper scripts silently defaulted a no-flag invocation to private while the plain CLI's own argparse default was already public and colleague's runtime hardcodes public — two consumers of the same store disagreeing on the default by accident. Both wrappers now default to public, matching the CLI and colleague.

This is a deliberate reversal of the private-by-default decision recorded in docs/specs/2026-06-23-eidetic-cli-s-remember-recall-skill-wrappers-are-h.md (FIX-4) — recorded honestly in docs/contract.md's History section, not silently overwritten; that spec affirmed private was intentional at the time and only fixed a doc/behavior mismatch around it. Downstream note: recall/remember are first-party to eidetic-cli and fan out to roughly 57 downstream repos (including colleague's own vendored copy) — this fix needs a downstream re-sync/rollout to actually take effect there, which is not part of this PR. Also aligns the embedder code default to the documented reference deployment (port 8002 / Qwen3-Embedding-0.6B, was 8101 / text-embedding-3-small), with a three-way drift test pinning code, docs, and wrapper scripts in agreement.

Test plan

• uv run pytest -n auto -q — 359 passed, 2 skipped (live Neo4j not reachable; cross-backend ranking now data-refinery's concern)
• tests/test_contract_drift.py and tests/test_wrapper_convention.py cross-check docs/contract.md's machine-readable block against both wrapper scripts' literal defaults and the CLI's actual argparse default
• tests/test_embed_default_drift.py pins the embedder default three ways (code, docs, wrapper scripts)

Honest limit: this PR ships the convention and the local wrapper fix only; the ~57-repo downstream propagation of the wrapper default flip is separate follow-up work, not included here.

🤖 Generated with Claude Code

• colleague (Claude)

[qodo-code-review]

PR Summary by Qodo

Pin memory scope/visibility convention v1; default wrappers to public; drift-test embedder

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Document v1 memory scope/visibility convention (culture.yaml suffix scope; default visibility
 public).
• Fix vendored recall/remember wrappers to default to public visibility (no-flag behavior).
• Align embedder defaults to reference deployment and add drift/roundtrip tests to prevent
 regressions.

Diagram

graph TD
Contract["docs/contract.md"] --> ContractDrift["tests/test_contract_drift.py"] --> CLI["eidetic CLI"]
ContractDrift --> Wrappers["recall.sh / remember.sh"] --> WrapperDocs["SKILL.md docs"]
Contract --> EmbedDrift["tests/test_embed_default_drift.py"] --> EmbedPy["eidetic/memory/embed.py"] --> Wrappers
Wrappers --> WrapperConv["tests/test_wrapper_convention.py"] --> CLI

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Stop injecting --visibility in wrappers (let CLI default apply)

• ➕ Eliminates one explicit default source; fewer places to drift
• ➕ Keeps wrapper behavior aligned automatically if CLI defaults change
• ➖ Less explicit wrapper behavior; changes are harder to audit by reading the script
• ➖ Still requires drift protection for docs/contract vs actual CLI behavior

2. Make CLI read defaults from a single config/contract source

• ➕ Single source of truth for defaults across CLI and wrappers (if wrappers can consume it)
• ➕ Reduces long-term maintenance burden as convention evolves
• ➖ Requires designing runtime-readable config and potentially adding parsing complexity/deps
• ➖ Harder to keep wrappers (shell) and Python consumers synchronized without extra tooling

Recommendation: Current approach (explicit contract + drift tests + wrapper fix) is the best fit: it resolves a real cross-consumer behavioral mismatch immediately and adds automated guardrails against regressions without introducing new runtime dependencies. Consider the “don’t inject --visibility” simplification later if you want fewer explicit defaults, but the added drift tests already mitigate the primary risk.

Files changed (12) +744 / -73

Enhancement (1) +10 / -2

embed.pyAlign embedder code defaults to reference endpoint/model +10/-2

Align embedder code defaults to reference endpoint/model

• Updates '_DEFAULT_BASE_URL' and '_DEFAULT_MODEL' to 'http://localhost:8002/v1' and 'Qwen/Qwen3-Embedding-0.6B' to match docs and wrapper exports. Adds explanatory comments and relies on new drift tests to keep values synchronized.

eidetic/memory/embed.py

Bug fix (2) +64 / -41

recall.shFlip recall wrapper default visibility to public and remove suffix-missing warning +33/-19

Flip recall wrapper default visibility to public and remove suffix-missing warning

• Changes the injected default from '--visibility private' to '--visibility public' when a culture.yaml suffix is resolved. Removes the warning path for missing suffix (now consistent with CLI default) and adds commentary on embedder env defaults alignment.

.claude/skills/recall/scripts/recall.sh

remember.shFlip remember wrapper default visibility to public and remove suffix-missing warning +31/-22

Flip remember wrapper default visibility to public and remove suffix-missing warning

• Changes the injected default from '--visibility private' to '--visibility public' when a culture.yaml suffix is resolved. Drops the previous stderr warning for missing suffix and adds commentary about embedder env default alignment.

.claude/skills/remember/scripts/remember.sh

Tests (3) +391 / -0

test_contract_drift.pyAdd drift test pinning visibility defaults across contract/wrappers/CLI +113/-0

Add drift test pinning visibility defaults across contract/wrappers/CLI

• Parses the machine-readable convention block in 'docs/contract.md' and asserts it matches the wrappers’ injected defaults and the CLI’s actual argparse defaults (via real register+parse_args). Prevents future silent divergence across the three surfaces.

tests/test_contract_drift.py

test_embed_default_drift.pyAdd drift test pinning embedder defaults across contract/code/wrappers +79/-0

Add drift test pinning embedder defaults across contract/code/wrappers

• Extracts embedder default URL/model from 'docs/contract.md' and asserts it matches 'eidetic/memory/embed.py' constants and wrapper script ':=' exports. Guards against future drift of reference deployment defaults.

tests/test_embed_default_drift.py

test_wrapper_convention.pyAdd integration tests for wrapper vs raw CLI mutual visibility by default +199/-0

Add integration tests for wrapper vs raw CLI mutual visibility by default

• Shells out to both wrapper scripts and 'python -m eidetic' with an isolated 'EIDETIC_DATA_DIR' and no visibility flags. Verifies both write public records by default and that a no-flag public recall sees both, proving cross-surface compatibility.

tests/test_wrapper_convention.py

Documentation (5) +278 / -29

SKILL.mdUpdate recall skill docs to reflect public-by-default convention +20/-13

Update recall skill docs to reflect public-by-default convention

• Updates documentation to describe the PERSONAL, PUBLIC default scope/visibility and references the new contract doc. Clarifies how to query private records explicitly.

.claude/skills/recall/SKILL.md

SKILL.mdUpdate remember skill docs to reflect public-by-default convention +23/-16

Update remember skill docs to reflect public-by-default convention

• Rewrites guidance so no-flag remember writes team-shared (public) records by default and private requires explicit '--visibility private'. Adds a pointer to 'docs/contract.md' as the source of truth.

.claude/skills/remember/SKILL.md

CHANGELOG.mdAdd 0.11.0 entry documenting contract, wrapper default flip, and embed default alignment +31/-0

Add 0.11.0 entry documenting contract, wrapper default flip, and embed default alignment

• Introduces release notes for v0.11.0 describing the new convention document, the deliberate default-visibility reversal in wrappers, and the embedder default alignment plus drift testing.

CHANGELOG.md

README.mdDocument embedder reference deployment alignment and env-var precedence +20/-0

Document embedder reference deployment alignment and env-var precedence

• Adds a detailed section explaining the reference embedder default, how it is now consistent across code/docs/wrappers, and how mesh discovery injects env defaults without overriding operator exports.

README.md

contract.mdAdd v1 memory scope+visibility contract with machine-readable blocks +184/-0

Add v1 memory scope+visibility contract with machine-readable blocks

• Adds the convention document defining per-repo scope naming via culture.yaml suffix and public default visibility, plus a store-resolution table and versioned history. Includes machine-readable blocks for scope/visibility and embedder defaults consumed by drift tests.

docs/contract.md

Other (1) +1 / -1

pyproject.tomlBump project version to 0.11.0 +1/-1

Bump project version to 0.11.0

• Updates the package version to reflect the new contract, default behavior change, and test additions.

pyproject.toml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 spec: one memory scope + visibility convention, drift-tested on both sides [colleague#291/S9]

✅ Compliance rules (platform): 32 rules

✅ Skills: version-bump

1. Wrapper test bypasses store resolution ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

tests/test_wrapper_convention.py forces EIDETIC_DATA_DIR, so it doesn’t validate that the
vendored wrapper and raw CLI resolve the same default store location under the contract’s normal
resolution rules (git-repo public -> <repo-root>/.eidetic/memory, otherwise HOME). This weakens
the intended drift protection because path-resolution regressions could pass while defaults still
“match” under the override.

Code

tests/test_wrapper_convention.py[R59-67]

+def _base_env(data_dir: Path) -> dict:
+    """A minimal, isolated environment: EIDETIC_DATA_DIR pinned to a throwaway
+    dir so this test never touches a real store, and no live embed server
+    dependency (ingest never needs one; keyword recall doesn't either)."""
+    env = dict(os.environ)
+    env["EIDETIC_DATA_DIR"] = str(data_dir)
+    env.pop("EIDETIC_EMBED_URL", None)
+    env.pop("EIDETIC_EMBED_MODEL", None)
+    return env

Relevance

⭐⭐ Medium

PR23 focused on hermetic routing tests; no precedent requiring default store-resolution w/o
EIDETIC_DATA_DIR.

PR-#23

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1345474 requires a drift/integration test that shells both the vendored wrapper and
raw CLI with no flags and asserts they write to the same resolved store location under the agreed
convention. The added test explicitly sets EIDETIC_DATA_DIR, which overrides normal resolution and
therefore does not validate the default repo-vs-HOME routing behavior.

Eidetic-side integration test enforces wrapper defaults match raw CLI and contract
tests/test_wrapper_convention.py[59-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/test_wrapper_convention.py` currently sets `EIDETIC_DATA_DIR`, which short-circuits the store-resolution algorithm and prevents the test from verifying the contract’s default store-location behavior (repo-local for public-in-repo, HOME otherwise) across wrapper vs raw CLI.

## Issue Context
Compliance requires an integration test that compares vendored wrapper vs raw CLI behavior with no flags and asserts they write to the same resolved store location. Using `EIDETIC_DATA_DIR` makes the location trivially identical and will not catch regressions in git-repo detection or visibility-based routing.

## Fix Focus Areas
- tests/test_wrapper_convention.py[59-157]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Wrapper test version skew ✓ Resolved 🐞 Bug ☼ Reliability

Description

tests/test_wrapper_convention.py can run the wrappers against a globally-installed eidetic
(preferred by resolve_eidetic) while comparing against the in-tree code via python -m eidetic,
making the new integration test flaky or validating the wrong version. This is especially likely on
developer machines (or CI images) that already have eidetic on PATH.

Code

tests/test_wrapper_convention.py[R59-129]

+def _base_env(data_dir: Path) -> dict:
+    """A minimal, isolated environment: EIDETIC_DATA_DIR pinned to a throwaway
+    dir so this test never touches a real store, and no live embed server
+    dependency (ingest never needs one; keyword recall doesn't either)."""
+    env = dict(os.environ)
+    env["EIDETIC_DATA_DIR"] = str(data_dir)
+    env.pop("EIDETIC_EMBED_URL", None)
+    env.pop("EIDETIC_EMBED_MODEL", None)
+    return env
+
+
+def _raw_eidetic(*args: str) -> list[str]:
+    """Invoke the raw `eidetic` CLI via the current interpreter (no wrapper,
+    no scope/visibility injection) — the "unaware direct consumer" surface."""
+    return [sys.executable, "-m", "eidetic", *args]
+
+
+def _all_records(data_dir: Path) -> list[dict]:
+    """Read every record from every *.jsonl file under data_dir (any scope)."""
+    records: list[dict] = []
+    if not data_dir.exists():
+        return records
+    for f in data_dir.glob("*.jsonl"):
+        for line in f.read_text(encoding="utf-8").splitlines():
+            line = line.strip()
+            if line:
+                records.append(json.loads(line))
+    return records
+
+
+def _visibility_of(records: list[dict], record_id: str) -> str | None:
+    for r in records:
+        if r["id"] == record_id:
+            return r["scope"]["visibility"]
+    return None
+
+
+def test_wrapper_and_raw_cli_default_to_the_same_visibility(tmp_path: Path) -> None:
+    data_dir = tmp_path / "store"
+    env = _base_env(data_dir)
+
+    wrapper_record = {
+        "id": "wrap-conv-1",
+        "text": "wrapper convention record",
+        "type": "note",
+    }
+    raw_record = {
+        "id": "raw-conv-1",
+        "text": "raw cli convention record",
+        "type": "note",
+    }
+
+    # The vendored wrapper: no --scope, no --visibility. It resolves the
+    # culture.yaml suffix itself (walking up from the script's own location),
+    # so it doesn't matter that we run from the repo root — that just matches
+    # how an agent invokes it in practice.
+    wrapper_result = _run(
+        ["bash", str(REMEMBER_WRAPPER), json.dumps(wrapper_record), "--json"],
+        cwd=REPO_ROOT,
+        env=env,
+    )
+    assert wrapper_result.returncode == 0, wrapper_result.stderr
+
+    # The raw CLI, exactly as an unaware direct consumer would call it: no
+    # --scope, no --visibility.
+    raw_result = _run(
+        _raw_eidetic("remember", json.dumps(raw_record), "--json"),
+        cwd=REPO_ROOT,
+        env=env,
+    )
+    assert raw_result.returncode == 0, raw_result.stderr

Relevance

⭐⭐⭐ High

Team fixes hermetic env issues in tests (PR23); PATH-vs-module version skew would be
flaky/unacceptable.

PR-#23

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test invokes the wrappers via bash remember.sh/bash recall.sh without controlling how they
resolve eidetic, while the comparison path uses sys.executable -m eidetic. The wrapper scripts
explicitly prefer a PATH eidetic before using the project checkout, so the two invocations can
execute different versions.

tests/test_wrapper_convention.py[70-129]
.claude/skills/remember/scripts/remember.sh[24-43]
.claude/skills/recall/scripts/recall.sh[17-37]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`tests/test_wrapper_convention.py` shells `.claude/.../remember.sh` / `recall.sh`, but those scripts resolve `eidetic` from `PATH` first. The test’s “raw CLI” path uses `python -m eidetic` (in-tree), so the two surfaces can run different code versions.

### Issue Context
- Wrapper scripts prefer `eidetic` on PATH, then fall back to `uv run --project <checkout> eidetic`.
- The new integration test intends to compare wrapper behavior to the current checkout’s CLI behavior.

### Fix Focus Areas
- tests/test_wrapper_convention.py[59-129]
- .claude/skills/remember/scripts/remember.sh[24-48]
- .claude/skills/recall/scripts/recall.sh[17-43]

### Suggested fix
Choose one:
1) Add an env override to the wrapper scripts (e.g. `EIDETIC_CMD` or `EIDETIC_WRAPPER_EIDETIC`) that, when set, is used verbatim as the executable/argv prefix. In the test, set it to `[sys.executable, "-m", "eidetic"]` (or a string form) so both paths use the same code.
2) Change wrapper resolution order **when a local eidetic-cli checkout is detected**: prefer `uv run --project <checkout> eidetic` over a PATH-installed `eidetic`. (This keeps vendored copies unchanged, but makes in-repo wrappers deterministic for tests/dev.)
3) In the test, adjust `env["PATH"]` to exclude any global `eidetic` while still including `uv` + `bash` (harder to do portably, so (1) or (2) is safer).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Test reads store files ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

tests/test_wrapper_convention.py asserts visibility by scraping *.jsonl files directly from
EIDETIC_DATA_DIR, coupling the test to the backend’s on-disk layout. Since eidetic delegates
storage to data_refinery’s store adapter, this test can fail after backend/layout changes even if
the CLI contract remains correct.

Code

tests/test_wrapper_convention.py[R76-86]

+def _all_records(data_dir: Path) -> list[dict]:
+    """Read every record from every *.jsonl file under data_dir (any scope)."""
+    records: list[dict] = []
+    if not data_dir.exists():
+        return records
+    for f in data_dir.glob("*.jsonl"):
+        for line in f.read_text(encoding="utf-8").splitlines():
+            line = line.strip()
+            if line:
+                records.append(json.loads(line))
+    return records

Relevance

⭐⭐⭐ High

Repo explicitly moved away from file-touching/layout coupling (PR15); direct *.jsonl scraping is
brittle.

PR-#15
PR-#23

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test explicitly parses *.jsonl from the store directory, but eidetic’s backend module
documents that it delegates storage to data_refinery.store and does not specify a stable file
layout itself.

tests/test_wrapper_convention.py[76-86]
eidetic/memory/backend.py[1-20]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The integration test reads `*.jsonl` files directly to find record visibilities. That makes the test depend on internal storage layout details instead of the supported API/CLI surface.

### Issue Context
`eidetic/memory/backend.py` explicitly delegates storage to `data_refinery.store`, so eidetic does not “own” the concrete file naming/layout contract.

### Fix Focus Areas
- tests/test_wrapper_convention.py[76-93]
- eidetic/memory/backend.py[1-20]

### Suggested fix
Replace `_all_records()` + `_visibility_of()` JSONL scraping with an API-level read.

For example (in-process):
- Temporarily set `os.environ["EIDETIC_DATA_DIR"] = str(data_dir)`
- Call `get_backend("files").all()` (which returns `Record` objects) and locate the records by id to check `record.scope.visibility`.

This still validates that the subprocess write happened, but avoids depending on the JSONL file layout.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud