Skip to content

eidetic-memory: refresh wrappers (eidetic 0.10) + memory-discipline convention#5

Open
OriNachum wants to merge 1 commit into
mainfrom
rollout/eidetic-memory-0.10
Open

eidetic-memory: refresh wrappers (eidetic 0.10) + memory-discipline convention#5
OriNachum wants to merge 1 commit into
mainfrom
rollout/eidetic-memory-0.10

Conversation

@OriNachum

Copy link
Copy Markdown
Contributor

Added

  • Memory-discipline "Conventions and workflow" section in CLAUDE.md — a
    per-task recall-before / remember-after convention (scope localized to this
    repo's nick) so the vendored remember / recall skills are actually used,
    not just present: /recall before non-trivial work to build on prior
    decisions instead of re-deriving them, and /remember when a non-obvious
    decision, constraint, fix-and-why, or hard-won gotcha surfaces. The section
    documents this repo's memory as in-repo and public — records resolve to
    <repo-root>/.eidetic/memory (committed, team- and mesh-shared). Inserted
    idempotently (skipped if already present), slotted under an existing
    "Conventions and workflow" heading when one exists, else appended.

Changed

  • Refreshed the remember + recall wrappers from eidetic-cli 0.10.0
    (cite-don't-import) — picks up eidetic's project-local store default: the
    files backend now resolves per record by visibility — PUBLIC records inside a
    git repo go to <repo-root>/.eidetic/memory (committed, team-shared), PRIVATE
    records (or any record outside a repo) go to $HOME/.eidetic/memory (never
    committed), an explicit EIDETIC_DATA_DIR still wins, and recall reads both
    stores and merges. Also carries the 0.9.3 hardening (interactive-stdin guard,
    help as a search term, SIGPIPE-safe suffix parsing). Recipe policy
    override (the wrappers here are NOT byte-verbatim):
    the injected default
    visibility is flipped from eidetic's private to public, so a plain
    /remember lands the note in ./.eidetic/memory in this repo, kept as part
    of the repo — pass --visibility private to route a record to $HOME
    instead. remember drives eidetic remember (idempotent upsert of one JSON
    record or an NDJSON batch on stdin); recall drives eidetic recall with
    four search modes (exact / approximate / keyword / hybrid). Each SKILL.md is
    localized only in the illustrative --scope <nick> examples (Provenance keeps
    "First-party to eidetic-cli"). Runtime dep: the eidetic CLI on PATH (else a
    local eidetic-cli checkout with uv) — eidetic >= 0.10.0 for the
    in-repo routing; on an older CLI the public records still work but are stored
    in $HOME/.eidetic/memory instead of in-repo. Propagated by rollout-cli's
    eidetic-memory recipe.

- **Memory-discipline "Conventions and workflow" section in `CLAUDE.md`** — a
  per-task *recall-before / remember-after* convention (scope localized to this
  repo's nick) so the vendored `remember` / `recall` skills are actually used,
  not just present: `/recall` before non-trivial work to build on prior
  decisions instead of re-deriving them, and `/remember` when a non-obvious
  decision, constraint, fix-and-why, or hard-won gotcha surfaces. The section
  documents this repo's memory as **in-repo and public** — records resolve to
  `<repo-root>/.eidetic/memory` (committed, team- and mesh-shared). Inserted
  idempotently (skipped if already present), slotted under an existing
  "Conventions and workflow" heading when one exists, else appended.

### Changed

- **Refreshed the `remember` + `recall` wrappers from eidetic-cli 0.10.0**
  (cite-don't-import) — picks up eidetic's **project-local store default**: the
  files backend now resolves per record by visibility — PUBLIC records inside a
  git repo go to `<repo-root>/.eidetic/memory` (committed, team-shared), PRIVATE
  records (or any record outside a repo) go to `$HOME/.eidetic/memory` (never
  committed), an explicit `EIDETIC_DATA_DIR` still wins, and recall reads both
  stores and merges. Also carries the 0.9.3 hardening (interactive-stdin guard,
  `help` as a search term, SIGPIPE-safe suffix parsing). **Recipe policy
  override (the wrappers here are NOT byte-verbatim):** the injected default
  visibility is flipped from eidetic's `private` to **`public`**, so a plain
  `/remember` lands the note in `./.eidetic/memory` in this repo, kept as part
  of the repo — pass `--visibility private` to route a record to `$HOME`
  instead. `remember` drives `eidetic remember` (idempotent upsert of one JSON
  record or an NDJSON batch on stdin); `recall` drives `eidetic recall` with
  four search modes (exact / approximate / keyword / hybrid). Each `SKILL.md` is
  localized only in the illustrative `--scope <nick>` examples (Provenance keeps
  "First-party to eidetic-cli"). Runtime dep: the `eidetic` CLI on PATH (else a
  local eidetic-cli checkout with `uv`) — **`eidetic >= 0.10.0`** for the
  in-repo routing; on an older CLI the public records still work but are stored
  in `$HOME/.eidetic/memory` instead of in-repo. Propagated by rollout-cli's
  `eidetic-memory` recipe.
@sonarqubecloud

Copy link
Copy Markdown

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Refresh eidetic memory wrappers to 0.10 + document recall/remember workflow
✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

Description

• Refresh vendored remember/recall wrappers to eidetic-cli 0.10 store routing behavior.
• Default wrapper visibility to public for shared, in-repo memory; warn on missing scope.
• Document per-task recall-before/remember-after convention and bump version/changelog.
Diagram

graph TD
  U(["Developer / Agent"]) --> R["recall.sh"] --> E["eidetic CLI"]
  U --> M["remember.sh"] --> E
  E --> G{"In git repo?"}
  G -->|"public"| S1[("Repo memory .eidetic/")]
  G -->|"private or no repo"| S2[("Home memory $HOME/.eidetic/")]

  subgraph Legend
    direction LR
    _p["Process"] ~~~ _d[("Data store")] ~~~ _x{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep wrappers byte-verbatim; enforce public via docs only
  • ➕ Zero local divergence from upstream vendored scripts
  • ➕ Lower chance of merge conflicts on future wrapper refreshes
  • ➖ Relies on humans to always pass --visibility public
  • ➖ Shared in-repo memory goal is easier to miss / inconsistently applied
2. Use environment defaults instead of wrapper-injected flags
  • ➕ Policy can be set per repo without changing wrapper behavior
  • ➕ Easier to flip between public/private in different environments
  • ➖ Requires eidetic to support the needed env-based defaults consistently
  • ➖ Harder to debug because behavior depends on ambient environment
3. Separate commands: remember-public / remember-private (and recall equivalents)
  • ➕ Makes visibility explicit and reduces accidental public writes
  • ➕ Allows different UX defaults without warnings/branching logic
  • ➖ More commands to maintain and teach
  • ➖ Still needs wrapper logic duplication unless centralized

Recommendation: Given the stated goal of making memory team-shared and repo-portable, the PR’s approach (defaulting wrapper visibility to public + documenting the workflow) is reasonable and likely to drive actual adoption. The main tradeoff is privacy/safety: the added stderr warnings help, but reviewers should confirm the public-default policy is acceptable for this repo and that contributors understand --visibility private routes to $HOME and is never committed.

Files changed (5) +142 / -27

Enhancement (2) +74 / -26
recall.shUpdate recall wrapper for eidetic 0.10 routing and safer CLI UX +35/-13

Update recall wrapper for eidetic 0.10 routing and safer CLI UX

• Updates store-location documentation to reflect eidetic 0.10’s repo-public vs home-private routing and merged reads. Tightens UX and robustness: treat empty query as an error (keeping 'help' as a valid search term), simplify missing-CLI error output, make culture.yaml suffix parsing SIGPIPE-safe under pipefail, and default injected visibility to public (policy override) with a warning when no suffix resolves.

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

remember.shUpdate remember wrapper for eidetic 0.10 routing, stdin guard, and public default +39/-13

Update remember wrapper for eidetic 0.10 routing, stdin guard, and public default

• Updates store-location documentation for eidetic 0.10 routing semantics. Adds an interactive-stdin guard to avoid hanging when neither a JSON argument nor piped NDJSON is provided, simplifies missing-CLI error output, makes suffix parsing SIGPIPE-safe, and flips the injected default visibility to public (policy override) while warning if no culture.yaml suffix resolves and visibility wasn’t specified.

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

Documentation (2) +67 / -0
CHANGELOG.mdAdd 0.3.0 changelog entry for eidetic-memory refresh and workflow convention +39/-0

Add 0.3.0 changelog entry for eidetic-memory refresh and workflow convention

• Prepends a 0.3.0 entry describing the new memory-discipline convention in CLAUDE.md and the refreshed remember/recall wrappers aligned with eidetic-cli 0.10 behavior and policy override.

CHANGELOG.md

CLAUDE.mdDocument recall-before/remember-after memory-discipline workflow +28/-0

Document recall-before/remember-after memory-discipline workflow

• Adds a new “Conventions and workflow” section defining a per-task habit: run /recall before non-trivial work and /remember when non-obvious decisions or gotchas surface. Explicitly documents that this repo’s eidetic memory is in-repo/public by default (./.eidetic/memory) and how to opt into private storage.

CLAUDE.md

Other (1) +1 / -1
pyproject.tomlBump project version to 0.3.0 +1/-1

Bump project version to 0.3.0

• Updates the package version from 0.2.0 to 0.3.0 to reflect the documented release changes.

pyproject.toml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 20 rules

Grey Divider


Action required

1. Remember help text wrong 🐞 Bug ≡ Correctness
Description
remember.sh injects --visibility public by default (when a culture.yaml suffix is resolved),
but its usage() text still says records default to a PRIVATE personal scope and that `--visibility
public is an opt-in. This contradiction can mislead callers about whether a default /remember`
will write public/shared records.
Code

.claude/skills/remember/scripts/remember.sh[R62-64]

+Records default to this agent's PRIVATE personal scope (--scope from the
+culture.yaml suffix); pass --visibility public to contribute to the shared
+public pool. Every flag is forwarded verbatim to `eidetic remember`.
Evidence
The help text claims private-by-default semantics, but the script injects --visibility public when
a culture suffix is found, so the documented default cannot match actual behavior.

.claude/skills/remember/scripts/remember.sh[60-65]
.claude/skills/remember/scripts/remember.sh[139-149]

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

### Issue description
`remember.sh`'s `usage()` output still documents a private-by-default behavior, but the script now injects `--visibility public` when it resolves a `culture.yaml` suffix. This makes the help text misleading and contradicts actual runtime behavior.

### Issue Context
The wrapper was updated to default visibility to `public` (policy override) so that a plain remember lands in the repo-local store when in a git repo.

### Fix Focus Areas
- .claude/skills/remember/scripts/remember.sh[60-65]
- .claude/skills/remember/scripts/remember.sh[139-149]

### What to change
- Update the `usage()` text to state that the wrapper defaults to `--visibility public` (when injecting `--scope <suffix>`), and instruct users to pass `--visibility private` to keep records private.
- Ensure the wording matches the actual injected flags and the new repo-local/public behavior described elsewhere in the repo.

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



Remediation recommended

2. Recall docs stale 🐞 Bug ⚙ Maintainability
Description
recall.sh now injects --visibility public by default when a culture.yaml suffix is resolved,
but recall/SKILL.md still documents a private-default (--visibility private) and claims no-flag
recall includes private records by default. This can cause agents/users to misinterpret empty
results or think private notes are being searched when they are not.
Code

.claude/skills/recall/scripts/recall.sh[R139-145]

+        # rollout-cli eidetic-memory recipe POLICY OVERRIDE (not eidetic's
+        # upstream private default): default to PUBLIC, so a plain recall queries
+        # the in-repo public pool (<repo>/.eidetic/memory) this repo writes to.
+        # Pass --visibility private to also surface this agent's private ($HOME)
+        # notes. The two-store read model reads both dirs regardless.
+        has_flag --visibility "$@" || SCOPE_ARGS+=(--visibility public)
+    elif ! has_flag --visibility "$@"; then
Evidence
The wrapper code injects --visibility public by default, while the skill docs still state the
wrapper defaults to private visibility and will return private records on a no-flag recall.

.claude/skills/recall/scripts/recall.sh[134-145]
.claude/skills/recall/SKILL.md[12-18]
.claude/skills/recall/SKILL.md[121-128]

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 recall wrapper changed its default visibility injection to `public`, but the recall skill documentation still describes the old private-default behavior and expected results.

### Issue Context
This repo intentionally overrides upstream defaults so a plain recall/remember operates on the repo’s public pool by default.

### Fix Focus Areas
- .claude/skills/recall/scripts/recall.sh[134-153]
- .claude/skills/recall/SKILL.md[12-18]
- .claude/skills/recall/SKILL.md[121-128]

### What to change
- Update `recall/SKILL.md` to match the wrapper’s actual defaults (visibility `public` when scope suffix is injected).
- Adjust the narrative about what a no-flag recall returns (it should not claim private records are included unless `--visibility private` is used, if that’s the actual CLI behavior).
- Consider also updating any mention of the store location (`~/.eidetic/memory` vs repo-local `.eidetic/memory`) to match the new routing semantics.

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


Grey Divider

Qodo Logo

Comment on lines +62 to +64
Records default to this agent's PRIVATE personal scope (--scope from the
culture.yaml suffix); pass --visibility public to contribute to the shared
public pool. Every flag is forwarded verbatim to `eidetic remember`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Remember help text wrong 🐞 Bug ≡ Correctness

remember.sh injects --visibility public by default (when a culture.yaml suffix is resolved),
but its usage() text still says records default to a PRIVATE personal scope and that `--visibility
public is an opt-in. This contradiction can mislead callers about whether a default /remember`
will write public/shared records.
Agent Prompt
### Issue description
`remember.sh`'s `usage()` output still documents a private-by-default behavior, but the script now injects `--visibility public` when it resolves a `culture.yaml` suffix. This makes the help text misleading and contradicts actual runtime behavior.

### Issue Context
The wrapper was updated to default visibility to `public` (policy override) so that a plain remember lands in the repo-local store when in a git repo.

### Fix Focus Areas
- .claude/skills/remember/scripts/remember.sh[60-65]
- .claude/skills/remember/scripts/remember.sh[139-149]

### What to change
- Update the `usage()` text to state that the wrapper defaults to `--visibility public` (when injecting `--scope <suffix>`), and instruct users to pass `--visibility private` to keep records private.
- Ensure the wording matches the actual injected flags and the new repo-local/public behavior described elsewhere in the repo.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant