fix: make merge_evidence attach evidence instead of replacing content - #303
Merged
Conversation
MERGE currently rewrites the target memory with the candidate text, which loses specificity for a relation the judge already called equivalent. Pin the intended contract first: the target keeps its own title and body, the new version carries the candidate sources, the exact projection reflects the retained text, REVISE still writes the candidate text, the sanitized view revalidation still rejects drifted content, and two candidates merging into one memory still produce two distinct transitions.
MergeMemoryCandidate delegated to the REVISE executor, so a MERGE wrote the candidate's title and body over the target memory. The judge only reaches merge_evidence through relation equivalent, so by construction the candidate carries no new information and overwriting can only lose specificity; the outcome exists to attach the candidate's sources to a new version. It also made ReviseMemoryFromCandidate redundant, since the two transitions then differed only in their evidence-tier gate. MERGE now writes the target's current title and body (memory.title plus prior_version.body) into the new version. REVISE is unchanged. Kept as one executor with the varying decision extracted into _candidate_revision_content rather than forking a second function: the two transitions share the whole work-claim, fence, lock, projection and commit pipeline and differ in exactly one value, so duplicating it would let the security fences drift apart. _revalidated_sanitized_view still runs for MERGE and still raises on drift - only what is written changed. Idempotency is unaffected: _candidate_revision_fingerprint keys on the candidate id as subject plus the candidate fence, so two candidates merging into the same memory stay two transitions even though neither now decides the written text. The new version's _content_hash and the exact projection are computed from the retained text, so retrieval reflects what the memory kept. On the deterministic EXACT_IDENTITY path the candidate and target already match after NFKC/casefold/whitespace normalization, so the only visible change is that the target keeps its own casing and spacing.
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Bundle ReportBundle size has no change ✅ Affected Assets, Files, and Routes:view changes for bundle: engram-frontend-client-array-pushAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #303 +/- ##
==========================================
+ Coverage 91.03% 91.05% +0.02%
==========================================
Files 222 222
Lines 23196 23201 +5
==========================================
+ Hits 21116 21126 +10
+ Misses 2080 2075 -5 ☔ View full report in Codecov by Harness. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
merge_evidencenow merges evidence. It keeps the target memory's title and body andattaches the candidate's sources to a new version.
revise_memoryis unchanged.Why
MergeMemoryCandidatedelegated to_execute_candidate_revision— the same function thatimplements REVISE — which sets
memory.titleandmemory.bodyfrom the candidate. Amerge was a full content replacement wearing the name "merge".
That is wrong for this outcome specifically:
equivalent, with reason codeequivalent_claim.By definition the candidate carries no new information, so overwriting can only lose
specificity — a target saying "audit logs 90 days, debug 30 days" gets replaced by an
"equivalent" candidate saying "audit logs retained 90 days".
ReviseMemoryFromCandidatealready exists for the "update the text" case. If MERGE alsorewrote text, REVISE differed from it only by an evidence-tier gate.
The concrete symptom is now pinned as a test on the deterministic
EXACT_IDENTITYpath:These normalize to the same claim, so the gate merges them — and the old code replaced the
memory's title with the candidate's mangled casing.
Urgency
Merge fires 69 times all-time today because almost every attempt is denied. The companion
change (#A, deterministic outcome derivation) puts
merge_evidenceat rung 4 and makes itreachable for every
supportedcandidate — against a backlog of 5,824 mostly-duplicateproposed candidates. Shipping #A alone would knowingly amplify a destructive path. Both
must reach the stand together.
How
A single branch in
_candidate_revision_content: for MERGE the written text comes from thecurrent target state (
memory.title,prior_version.body); for REVISE nothing changes._revalidated_sanitized_viewstill runs for MERGE and still raises on mismatch — it isa security fence re-checking that the candidate's sanitized content has not changed since
the deterministic gate. Only what gets written changed.
Idempotency is untouched:
_candidate_revision_fingerprintkeys onsubject_id=candidate_idplus the candidate fence and the candidate's own title/body, noneof which this change affects. Two different candidates merging into the same memory still
commit two distinct transitions — pinned by a test.
Tests
No pre-existing test had to be modified, which is itself evidence that nothing relied on the
overwrite. Added coverage:
full_textcarries the retained text and does not contain thecandidate's
EXACT_IDENTITYpath keeps the target's wording