memory: port sync dedup fix, tolerate missing transcripts, document rem workflow - #154
Conversation
Reviewer's GuideThis PR ports the sync deduplication fix by matching exact serialized Hermes entries and comparing SHA-256 fingerprints of normalized content, makes missing transcript files non-fatal, and updates the memory README to document the canonical upstream model and report-first Sequence diagram for tolerant transcript readingsequenceDiagram
participant Hook
participant Reader
participant Transcript
Hook->>Reader: read_transcript(path)
Reader->>Transcript: Check path.exists()
alt transcript missing
Reader-->>Hook: [], None, None
else transcript present
Reader->>Transcript: Read transcript
Reader-->>Hook: Messages and metadata
end
Flow diagram for exact memory sync deduplicationflowchart TD
A[Hermes memory entries] --> B[Read vault knowledge]
B --> C[Find exact serialized entries]
C --> D[Normalize content]
D --> E[SHA-256 fingerprints]
E --> F{Fingerprint already exported?}
F -->|No| G[Append entry to vault]
F -->|Yes| H[Skip duplicate]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="memory/lib/sync.py" line_range="71-73" />
<code_context>
+ occupied: list[tuple[int, int]] = []
+ fingerprints: set[bytes] = set()
+ for entry in sorted(memory_entries, key=len, reverse=True):
+ pattern = re.compile(
+ rf"(?m)^- {re.escape(entry)}\n(?=- |\n## Sync |\Z)"
+ )
+ for match in pattern.finditer(text):
+ start, end = match.span()
+ if any(start < used_end and used_start < end for used_start, used_end in occupied):
</code_context>
<issue_to_address>
**issue (bug_risk):** When `sessions/knowledge.md` contains valid Markdown list items separated by a blank line, the regex does not recognize the exported entry because it requires the next line to start immediately with `- ` or `## Sync`. `memory_to_vault` therefore appends the same fact again on every sync.
**Triggers:** When an existing knowledge export has blank lines between bullet entries.
**Suggested fix:** Allow whitespace between serialized entries, or parse the sync sections as Markdown bullets instead of requiring adjacent `- ` lines.
```suggestion
pattern = re.compile(
rf"(?m)^- {re.escape(entry)}\n(?=\s*(?:- |## Sync |\Z))"
)
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the stricter serialized-entry matching can fail to recognize facts already present in the vault, causing duplicate records to be written. Reverting prevents further duplicates, but records already added remain and require cleanup or rerunning consolidation.
Blocking findings: memory/lib/sync.py:73
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| pattern = re.compile( | ||
| rf"(?m)^- {re.escape(entry)}\n(?=- |\n## Sync |\Z)" | ||
| ) |
There was a problem hiding this comment.
issue (bug_risk): When sessions/knowledge.md contains valid Markdown list items separated by a blank line, the regex does not recognize the exported entry because it requires the next line to start immediately with - or ## Sync. memory_to_vault therefore appends the same fact again on every sync.
Triggers: When an existing knowledge export has blank lines between bullet entries.
Suggested fix: Allow whitespace between serialized entries, or parse the sync sections as Markdown bullets instead of requiring adjacent - lines.
| pattern = re.compile( | |
| rf"(?m)^- {re.escape(entry)}\n(?=- |\n## Sync |\Z)" | |
| ) | |
| pattern = re.compile( | |
| rf"(?m)^- {re.escape(entry)}\n(?=\s*(?:- |## Sync |\Z))" | |
| ) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c998e83c37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rem dream # consolidation report (report-only) | ||
| rem dream --apply # collapse exact-duplicate records | ||
| rem search "quota preferences" # semantic search via memsearch | ||
| rem sync # flush the vault via knowledge-sync |
There was a problem hiding this comment.
When a user configures the knowledge vault with a public Git remote and follows this new rem sync workflow, knowledge-sync stages every vault change with git add -A and pushes it, even though the vault contains private session and profile data. This rewrite removes the repository's only warning that the vault must never be pushed publicly, so retain a prominent private-remote warning alongside this command.
Useful? React with 👍 / 👎.
| fingerprints: set[bytes] = set() | ||
| for entry in sorted(memory_entries, key=len, reverse=True): | ||
| pattern = re.compile( | ||
| rf"(?m)^- {re.escape(entry)}\n(?=- |\n## Sync |\Z)" |
There was a problem hiding this comment.
When Hermes changes only an entry's case or punctuation after an earlier export (for example, prefers pnpm. becoming Prefers pnpm), this exact-text regex cannot find the existing bullet even though normalize() treats both forms as identical. Each subsequent sync therefore appends another duplicate section; preserve normalized comparison while parsing complete serialized entries, and cover normalized variants in a focused sync test.
AGENTS.md reference: AGENTS.md:L42-L46
Useful? React with 👍 / 👎.
| `memory/lib`, `memory/hooks`, and `memory/tools`; `dotagents sync` builds the | ||
| tools and this repo carries the tested reference implementations. |
There was a problem hiding this comment.
For users installing the release CLI, dotagents setup walks StarterAssets, whose embed list contains only memory/hooks and memory/lib, not memory/tools; consequently the created ~/.agents repository has no tool sources and dotagents sync has nothing to build. The newly documented rem workflow therefore fails with command-not-found after the documented setup, so either include the tools in the starter assets or document the separate installation step.
AGENTS.md reference: AGENTS.md:L48-L50
Useful? React with 👍 / 👎.
1 participant
Summary by Sourcery
Make memory synchronization deduplicate exported facts reliably, tolerate unavailable transcripts, and document the current report-first workflow.
Bug Fixes:
Enhancements:
remworkflow, memory tiers, layout, and relationship to deployed user repositories.Documentation:
remworkflow and setup documentation.