Conversation
- Use Iceberg snapshot-log to determine which snapshots were natively committed on main vs merged/fast-forwarded from other branches - Snapshots in main's ancestry but not in snapshot-log are placed on their originating branch's row instead of main's row - Main branch links now drawn between consecutive snapshot-log entries - Divergence links only drawn when child actually leaves main - Fix label placement for 2 labels on same node: stack vertically instead of horizontal spread to avoid overlap with adjacent nodes
WalkthroughEnhances graph visualization in TableVersioningVisualization.vue by introducing snapshot-log awareness for tracking native main branch snapshots. Adds mainSnapshotLogIds computed property, modifies graphNodes to conditionally populate main ancestry based on snapshot-log presence, updates graphLinks with main ownership tracking and divergence detection, and implements vertical label stacking for 2-label scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/TableVersioningVisualization.vue`:
- Around line 1251-1266: The ancestry loop is double-drawing the fork edge
(making it brighter) because the same mainOwned -> first-non-main pair is
emitted again in the "diverge" pass; update the diverge-pass logic (the code
that creates "diverge-" links, referenced alongside the existing ancestry loop
and addLink calls) to skip creating a link when both the childId and parentId
are in mainOwnedIds or when branch === mainBr/branch.type === 'branch' matches
the same condition used above, so the addLink call is not invoked twice for the
same segment.
- Around line 1215-1235: The current logic builds mainLogOrdered from
snapshotLog chronological entries which can reintroduce snapshot IDs out of
topological order; instead derive the ordered list from mainBr.ancestry filtered
by mainSnapshotLogIds so the order follows ancestry (topology) and only includes
IDs present in the log. Replace the creation of mainLogOrdered with a filter
over mainBr.ancestry (using mainSnapshotLogIds or logIds.has) and then iterate
that ordered array to get parentNode/childNode from nodeMap and call
addLink(parentNode, childNode, mainBr.color, 0.8).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbeab3a1-7167-412a-8ddf-9f2daa005a02
📒 Files selected for processing (1)
src/components/TableVersioningVisualization.vue
| // Determine which snapshot IDs are "on main" for link-ownership purposes. | ||
| // If snapshot-log is available, use it; otherwise fall back to full ancestry. | ||
| const logIds = mainSnapshotLogIds.value; | ||
| const mainOwnedIds: Set<number> = | ||
| logIds.size > 0 && mainBr ? logIds : mainBr ? new Set(mainBr.ancestry) : new Set<number>(); | ||
|
|
||
| // For main branch: draw links between consecutive snapshot-log entries | ||
| // (skips snapshots that were merged in from other branches). | ||
| if (mainBr && logIds.size > 0) { | ||
| const snapshotLog = (props.table.metadata as any)?.['snapshot-log'] as | ||
| | Array<{ 'snapshot-id': number }> | ||
| | undefined; | ||
| if (snapshotLog) { | ||
| const mainLogOrdered = snapshotLog | ||
| .map((entry) => entry['snapshot-id']) | ||
| .filter((id) => mainBr.ancestry.includes(id)); | ||
| for (let i = 0; i < mainLogOrdered.length - 1; i++) { | ||
| const parentNode = nodeMap.get(mainLogOrdered[i]); | ||
| const childNode = nodeMap.get(mainLogOrdered[i + 1]); | ||
| if (parentNode && childNode) addLink(parentNode, childNode, mainBr.color, 0.8); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Apache Iceberg snapshot-log semantics: is snapshot-loga time-ordered history ofcurrent-snapshot-id, and can the same snapshot-idappear multiple times afterrollbackToorsetCurrentSnapshot?
💡 Result:
- Yes (conceptually):
snapshot-logis a chronological record of changes to the table’scurrent-snapshot-id(i.e., when a given snapshot became “current”). The spec says: each time thecurrent-snapshot-idis changed, a new entry should be added usinglast-updated-msand the newcurrent-snapshot-id. It’s also used to resolve “TIMESTAMP AS OF” queries by finding the snapshot just prior to a timestamp. [1] - It is not a permanent full history: when snapshots are expired, entries before an expired snapshot “should be removed,” so the log can be truncated. [1]
- Duplicates are allowed/expected: because the rule is “append an entry on every change of
current-snapshot-id” (not “only when it changes to a never-before-current snapshot”), the samesnapshot-idcan appear multiple times if you move the current pointer back and forth—e.g., viarollbackToorsetCurrentSnapshot(both change what snapshot is current rather than creating a new snapshot). This follows directly from the spec’s “add an entry each time it is changed” behavior. [1] - Ordering: implementations/documentation commonly state entries are ordered by timestamp (chronological). [2]
Sources
[1] Apache Iceberg Table Spec – table metadata fields (current-snapshot-id, snapshot-log) and time-travel behavior
[2] DeepWiki: “Snapshot system and time travel” (Iceberg) – notes snapshot-log is chronological / sorted by timestamp
🏁 Script executed:
cd src/components && wc -l TableVersioningVisualization.vueRepository: lakekeeper/console-components
Length of output: 110
🏁 Script executed:
cd src/components && sed -n '1200,1250p' TableVersioningVisualization.vueRepository: lakekeeper/console-components
Length of output: 2306
🏁 Script executed:
cd src/components && sed -n '1,100p' TableVersioningVisualization.vue | head -50Repository: lakekeeper/console-components
Length of output: 2275
🏁 Script executed:
cd src/components && sed -n '1,400p' TableVersioningVisualization.vue | grep -n -A 5 -B 5 "branches\|ancestry\|snapshot"Repository: lakekeeper/console-components
Length of output: 3556
🏁 Script executed:
cd src/components && rg "ancestry" TableVersioningVisualization.vue -n -B 2 -A 2 | head -80Repository: lakekeeper/console-components
Length of output: 3442
🏁 Script executed:
cd src/components && rg "const branches\|\.value.*branches" TableVersioningVisualization.vue -n -B 3 -A 10Repository: lakekeeper/console-components
Length of output: 55
🏁 Script executed:
cd src/components && rg "traceAncestry" TableVersioningVisualization.vue -n -B 3 -A 15 | head -80Repository: lakekeeper/console-components
Length of output: 2172
🏁 Script executed:
cd src/components && rg "function traceAncestry|const traceAncestry" TableVersioningVisualization.vue -n -B 2 -A 20Repository: lakekeeper/console-components
Length of output: 890
Use ancestry order instead of snapshot-log chronological order for main branch links.
Iceberg's snapshot-log is a chronological record of current-snapshot-id changes, where the same snapshot ID can legitimately reappear after rollbackTo() or setCurrentSnapshot(). Using this raw log order to draw graph links produces self-links or backward edges. Instead, filter mainBr.ancestry by logIds to get the correct topological ordering of snapshots natively committed to main.
🛠️ Suggested change
- const snapshotLog = (props.table.metadata as any)?.['snapshot-log'] as
- | Array<{ 'snapshot-id': number }>
- | undefined;
- if (snapshotLog) {
- const mainLogOrdered = snapshotLog
- .map((entry) => entry['snapshot-id'])
- .filter((id) => mainBr.ancestry.includes(id));
+ const mainLogOrdered = [...mainBr.ancestry].reverse().filter((id) => logIds.has(id));
+ if (mainLogOrdered.length > 1) {
for (let i = 0; i < mainLogOrdered.length - 1; i++) {
const parentNode = nodeMap.get(mainLogOrdered[i]);
const childNode = nodeMap.get(mainLogOrdered[i + 1]);
if (parentNode && childNode) addLink(parentNode, childNode, mainBr.color, 0.8);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine which snapshot IDs are "on main" for link-ownership purposes. | |
| // If snapshot-log is available, use it; otherwise fall back to full ancestry. | |
| const logIds = mainSnapshotLogIds.value; | |
| const mainOwnedIds: Set<number> = | |
| logIds.size > 0 && mainBr ? logIds : mainBr ? new Set(mainBr.ancestry) : new Set<number>(); | |
| // For main branch: draw links between consecutive snapshot-log entries | |
| // (skips snapshots that were merged in from other branches). | |
| if (mainBr && logIds.size > 0) { | |
| const snapshotLog = (props.table.metadata as any)?.['snapshot-log'] as | |
| | Array<{ 'snapshot-id': number }> | |
| | undefined; | |
| if (snapshotLog) { | |
| const mainLogOrdered = snapshotLog | |
| .map((entry) => entry['snapshot-id']) | |
| .filter((id) => mainBr.ancestry.includes(id)); | |
| for (let i = 0; i < mainLogOrdered.length - 1; i++) { | |
| const parentNode = nodeMap.get(mainLogOrdered[i]); | |
| const childNode = nodeMap.get(mainLogOrdered[i + 1]); | |
| if (parentNode && childNode) addLink(parentNode, childNode, mainBr.color, 0.8); | |
| } | |
| // Determine which snapshot IDs are "on main" for link-ownership purposes. | |
| // If snapshot-log is available, use it; otherwise fall back to full ancestry. | |
| const logIds = mainSnapshotLogIds.value; | |
| const mainOwnedIds: Set<number> = | |
| logIds.size > 0 && mainBr ? logIds : mainBr ? new Set(mainBr.ancestry) : new Set<number>(); | |
| // For main branch: draw links between consecutive snapshot-log entries | |
| // (skips snapshots that were merged in from other branches). | |
| if (mainBr && logIds.size > 0) { | |
| const mainLogOrdered = [...mainBr.ancestry].reverse().filter((id) => logIds.has(id)); | |
| if (mainLogOrdered.length > 1) { | |
| for (let i = 0; i < mainLogOrdered.length - 1; i++) { | |
| const parentNode = nodeMap.get(mainLogOrdered[i]); | |
| const childNode = nodeMap.get(mainLogOrdered[i + 1]); | |
| if (parentNode && childNode) addLink(parentNode, childNode, mainBr.color, 0.8); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/TableVersioningVisualization.vue` around lines 1215 - 1235,
The current logic builds mainLogOrdered from snapshotLog chronological entries
which can reintroduce snapshot IDs out of topological order; instead derive the
ordered list from mainBr.ancestry filtered by mainSnapshotLogIds so the order
follows ancestry (topology) and only includes IDs present in the log. Replace
the creation of mainLogOrdered with a filter over mainBr.ancestry (using
mainSnapshotLogIds or logIds.has) and then iterate that ordered array to get
parentNode/childNode from nodeMap and call addLink(parentNode, childNode,
mainBr.color, 0.8).
| for (let i = 0; i < branch.ancestry.length - 1; i++) { | ||
| const childId = branch.ancestry[i]; | ||
| const parentId = branch.ancestry[i + 1]; | ||
| const child = nodeMap.get(childId); | ||
| const parent = nodeMap.get(parentId); | ||
| if (!child || !parent) continue; | ||
| // If both child and parent are owned by main, skip — main draws its own links | ||
| if ( | ||
| branch !== mainBr && | ||
| branch.type === 'branch' && | ||
| mainOwnedIds.has(childId) && | ||
| mainOwnedIds.has(parentId) | ||
| ) { | ||
| continue; | ||
| } | ||
| addLink(parent, child, hasTagOnDropped ? TAG_COLOR : branch.color, opacity); |
There was a problem hiding this comment.
Avoid double-drawing the fork edge.
The ancestry walk already emits the mainOwned -> first non-main pair, so the extra diverge- pass redraws the exact same segment. The split edge ends up brighter/thicker than the rest of the branch.
🛠️ Suggested change
- // Divergence from main: draw a colored link where the branch leaves main's owned set.
- // Only draw if the child snapshot is NOT owned by main (i.e. an actual fork exists).
- if (branch.type === 'branch' && branch.name !== 'main' && branch.name !== 'master') {
- if (mainBr) {
- for (let i = 0; i < branch.ancestry.length; i++) {
- if (
- mainOwnedIds.has(branch.ancestry[i]) &&
- i > 0 &&
- !mainOwnedIds.has(branch.ancestry[i - 1])
- ) {
- const from = nodeMap.get(branch.ancestry[i]);
- const to = nodeMap.get(branch.ancestry[i - 1]);
- if (from && to) addLink(from, to, branch.color, 0.8, 'diverge-');
- break;
- }
- }
- }
- }Also applies to: 1270-1283
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/TableVersioningVisualization.vue` around lines 1251 - 1266,
The ancestry loop is double-drawing the fork edge (making it brighter) because
the same mainOwned -> first-non-main pair is emitted again in the "diverge"
pass; update the diverge-pass logic (the code that creates "diverge-" links,
referenced alongside the existing ancestry loop and addLink calls) to skip
creating a link when both the childId and parentId are in mainOwnedIds or when
branch === mainBr/branch.type === 'branch' matches the same condition used
above, so the addLink call is not invoked twice for the same segment.
1 participant
Uses Iceberg snapshot-log metadata to correctly assign branch visualization rows and links after fast-forward merges.
After fast-forwarding main to include commits from another branch, the visualization incorrectly showed all merged snapshots on main row. Now uses snapshot-log to distinguish natively committed snapshots from merged ones, and places them on correct branch rows.
Summary by CodeRabbit
Bug Fixes
New Features