-
Notifications
You must be signed in to change notification settings - Fork 234
Semantic rebase support #8938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Semantic rebase support #8938
Conversation
…ers etc which doesn't apply to this fork
…oham/semantic-rebase
|
@soham-bentley this contains changes that are not supposed to be in here. I think you pulled in too many changesets during rebase. |
… codeowners etc which doesn't apply to this fork" This reverts commit a96a770.
… rebase folder after rebase if it is empty and added tests
…ts for semantic rebase - Introduced v01.00.03 schema that adds PropC2 as a string to test incompatibility with previous version having PropC2 as int. - Updated test cases to reflect changes in schema versions and properties. - Adjusted expectations in tests to ensure proper handling of incompatible changes during schema pull operations. - Marked certain tests for investigation due to failures related to schema transformations.
|
/azp run iTwin.js |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@rschili PR description says this is WIP. Is this ready for review? |
|
@markschlosseratbentley |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds semantic rebase support to better handle schema-related conflicts by capturing/restoring schema and data changes as “semantic” artifacts during pull/merge, plus adds new tests (including perf tests) to validate behavior.
Changes:
- Introduces semantic rebase folder capture/restore utilities in
BriefcaseManagerand a new rebase flow (resumeSemantic) inTxnManager. - Extends schema-import and briefcase lifecycle logic to support semantic rebase restrictions and cleanup.
- Adds extensive backend tests plus optional perf tests for semantic rebase scenarios.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| full-stack-tests/backend/src/perftest/SemanticRebase.test.ts | Adds skipped perf tests to measure semantic rebase performance under schema/data scenarios. |
| core/common/src/TxnProps.ts | Extends TxnType to include "Schema" as a temporary workaround for native behavior. |
| core/backend/src/test/hubaccess/SemanticRebase.test.ts | Adds an extensive semantic rebase test suite covering many conflict scenarios. |
| core/backend/src/test/hubaccess/Rebase.test.ts | Runs existing rebase/stash tests with and without semantic rebase enabled. |
| core/backend/src/test/SquashSchemaAndDataChanges.test.ts | Adds tests for schema import restrictions and squashing behavior with semantic rebase enabled. |
| core/backend/src/TxnManager.ts | Adds resumeSemantic() and semantic reinstatement logic for schema/data txns. |
| core/backend/src/IModelJsFs.ts | Adds helper to read a file with encoding. |
| core/backend/src/IModelHost.ts | Adds useSemanticRebase configuration option and internal getter. |
| core/backend/src/IModelDb.ts | Adds semantic-rebase-aware schema import rules/locking and rebase-folder cleanup; adds getClassNameFromId. |
| core/backend/src/BriefcaseManager.ts | Adds semantic rebase folder structure + capture/restore of instance patches & schemas; integrates flow into pull/merge. |
| common/changes/@itwin/core-common/*.json | Adds change record (currently with empty comment). |
| common/changes/@itwin/core-backend/*.json | Adds change record for semantic rebase support. |
| common/api/* | Updates API reports for new/changed exported surface. |
| private static capturePatchInstances(db: BriefcaseDb): void { | ||
| const txns = Array.from(db.txns.queryTxns()); | ||
| txns.forEach((txn) => { | ||
| if (txn.type !== "Data") return; | ||
| // already captured(This actually shows that first rebase operation is already done but during that while reinstating this txns, | ||
| // some error happened so the folder still exists so we don't want to capture again) | ||
| if (this.semanticRebaseDataFolderExists(db, txn.id)) return; | ||
| const changedInstances = this.captureChangedInstancesAsJSON(txn.id, db); | ||
| const instancePatches = this.constructPatchInstances(changedInstances, db); | ||
| this.storeChangedInstancesForSemanticRebase(db, txn.id, instancePatches); | ||
| }); | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This captures patches for every "Data" txn on each pull/merge when semantic rebase is active. For long txn histories, repeatedly opening a SqliteChangesetReader + unifiers per txn can become expensive. Consider limiting capture to only the txns that are actually going to be reversed/rebased in the current operation (e.g., based on the list returned by the native reverse/local-changes step), or caching a "captured" marker in the txn table rather than using filesystem existence checks.
common/changes/@itwin/core-common/soham-semantic-rebase_2026-02-10-10-48.json
Show resolved
Hide resolved
…InstanceForSemanticRebase, and InstancePatch
…eature and its limitations
imodel-native: iTwin/imodel-native#1323
Implemented in collaboration with @soham-bentley .
Semantic rebase, which is used in certain conditions and generated a semantic version of the sqlite changeset. We then pull incoming changes and reinstate the local changes, which is capable of solving more complicated conflict cases, like incoming and local schema changes, data transforms on either side etc without locking the full database.
See linked story for detailed description