fix: roll back proposals when embedding generation fails - #141
Open
GautamSharma99 wants to merge 1 commit into
Open
fix: roll back proposals when embedding generation fails#141GautamSharma99 wants to merge 1 commit into
GautamSharma99 wants to merge 1 commit into
Conversation
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.
Summary
This PR fixes proposal-apply atomicity when graph-object embedding generation fails.
Previously, greplica proposal apply persisted the memory commit and proposal records before generating embeddings. If embedding
generation then failed—for example because the local model could not load or an external embedding request failed—the command
reported an error even though the proposal had already been written to SQLite.
The failed proposal could not be safely retried because its graph-object IDs already existed.
Problem
The proposal-apply path performed these operations in order:
Step 4 can fail independently of SQLite persistence. Because steps 2 and 3 had already committed, an embedding failure left partial
proposal state behind while reporting the overall operation as failed.
This could leave:
Root cause
KnowledgeGraphService.applyProposal() called createMemoryCommit() and createProposalRecords() separately before awaiting
GraphContextBuilder.ensureForGraph().
There was no rollback path for proposal data if embedding generation rejected.
Additionally, memory-commit creation and proposal-record creation were separate persistence operations, so a record-write failure
could leave an orphan memory commit.
Changes
Make commit and proposal-record creation atomic
Added SqliteRepository.createMemoryCommitWithProposal().
This method creates the memory commit and its proposal records within one SQLite transaction. If writing any component, flow, claim,
source, edge, or membership fails, the entire transaction is rolled back, including the memory commit.
Roll back proposal data after embedding failures
Added SqliteRepository.rollbackProposalRecords().
If embedding generation fails after the proposal has been persisted, the service now removes the data created by that apply attempt
in a compensating SQLite transaction:
The original embedding error is then rethrown so callers still receive the actual failure.
Avoid orphan commits during anchor fingerprinting
Code-anchor fingerprints are now computed before creating the memory commit. A fingerprinting failure therefore cannot leave an
empty or orphaned commit behind.
Add deterministic regression coverage
Added scripts/check-proposal-apply-atomicity.js and included it in the default npm test chain.
The regression test injects a context builder that:
It then verifies that:
Behavior before this PR
proposal records persisted
↓
embedding generation fails
↓
command reports failure
↓
proposal data remains in SQLite
↓
retry fails because IDs already exist
Behavior after this PR
proposal records persisted atomically
↓
embedding generation fails
↓
proposal data and generated embeddings are rolled back
↓
original embedding error is reported
↓
same proposal can be retried safely
Testing
The following checks pass:
npm test
npm run typecheck
The full test suite includes:
Scope
This PR is limited to proposal-apply failure handling and its regression coverage.
It does not change: