-
Notifications
You must be signed in to change notification settings - Fork 1
fix: use snapshot-log for branch visualization row assignment #95
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1046,6 +1046,26 @@ const tags = computed<TagMeta[]>(() => { | |
| return result; | ||
| }); | ||
|
|
||
| // ─── Computed: snapshot-log IDs (snapshots natively committed on main) ──────── | ||
| // The snapshot-log tracks the history of current-snapshot-id (always the main | ||
| // branch). Snapshots in main's ancestry but NOT in the snapshot-log were | ||
| // committed on other branches and later merged/fast-forwarded into main. | ||
| const mainSnapshotLogIds = computed<Set<number>>(() => { | ||
| const snapshotLog = (props.table.metadata as any)?.['snapshot-log'] as | ||
| | Array<{ 'snapshot-id': number }> | ||
| | undefined; | ||
| if (!snapshotLog || snapshotLog.length === 0) return new Set(); | ||
| const mainBranch = branches.value.find((b) => b.name === 'main' || b.name === 'master'); | ||
| if (!mainBranch) return new Set(); | ||
| const ancestrySet = new Set(mainBranch.ancestry); | ||
| const result = new Set<number>(); | ||
| snapshotLog.forEach((entry) => { | ||
| const sid = entry['snapshot-id']; | ||
| if (ancestrySet.has(sid)) result.add(sid); | ||
| }); | ||
| return result; | ||
| }); | ||
|
|
||
| // ─── Computed: nodes ───────────────────────────────────────────────────────── | ||
|
|
||
| const graphNodes = computed<GraphNode[]>(() => { | ||
|
|
@@ -1060,7 +1080,16 @@ const graphNodes = computed<GraphNode[]>(() => { | |
| const droppedBranches = branches.value.filter((b) => b.type === 'dropped'); | ||
|
|
||
| const snapshotRow = new Map<number, number>(); | ||
| if (mainBranch) mainBranch.ancestry.forEach((id) => snapshotRow.set(id, 0)); | ||
| const logIds = mainSnapshotLogIds.value; | ||
| if (mainBranch) { | ||
| mainBranch.ancestry.forEach((id) => { | ||
| // If snapshot-log is available, only assign to main row if the snapshot | ||
| // was natively committed on main (appears in snapshot-log). | ||
| if (logIds.size === 0 || logIds.has(id)) { | ||
| snapshotRow.set(id, 0); | ||
| } | ||
| }); | ||
| } | ||
| namedBranches.forEach((branch, idx) => { | ||
| branch.ancestry.forEach((id) => { | ||
| if (!snapshotRow.has(id)) snapshotRow.set(id, -(idx + 1)); | ||
|
|
@@ -1183,39 +1212,71 @@ const graphLinks = computed<GraphLink[]>(() => { | |
| // Build links for each branch | ||
| const mainBr = branches.value.find((b) => b.name === 'main' || b.name === 'master'); | ||
|
|
||
| // 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); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| branches.value.forEach((branch) => { | ||
| // Tagged-dropped branches get higher opacity (teal links) | ||
| const hasTagOnDropped = | ||
| branch.type === 'dropped' && | ||
| branch.ancestry.some((id) => tags.value.some((t) => t.snapshotId === id)); | ||
| const opacity = branch.type === 'dropped' ? (hasTagOnDropped ? 0.7 : 0.5) : 0.8; | ||
|
|
||
| // (stop once we hit a snapshot owned by main, to avoid coloring main's links) | ||
| const mainAncestrySet = mainBr ? new Set(mainBr.ancestry) : new Set<number>(); | ||
| 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 on main, skip — main will draw its own links | ||
| if ( | ||
| branch !== mainBr && | ||
| branch.type === 'branch' && | ||
| mainAncestrySet.has(childId) && | ||
| mainAncestrySet.has(parentId) | ||
| ) { | ||
| continue; | ||
| // For main when snapshot-log links were already drawn above, skip ancestry walk | ||
| if (branch === mainBr && logIds.size > 0) { | ||
| // Main's links already drawn from snapshot-log; skip to divergence/dropped | ||
| } else { | ||
| // Walk ancestry and draw links (skip pairs where both are on main) | ||
| 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); | ||
|
Comment on lines
+1251
to
+1266
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid double-drawing the fork edge. The ancestry walk already emits the 🛠️ 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 |
||
| } | ||
| addLink(parent, child, hasTagOnDropped ? TAG_COLOR : branch.color, opacity); | ||
| } | ||
|
|
||
| // Divergence from main | ||
| // 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) { | ||
| const mainSet = new Set(mainBr.ancestry); | ||
| for (let i = 0; i < branch.ancestry.length; i++) { | ||
| if (mainSet.has(branch.ancestry[i]) && i > 0) { | ||
| 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-'); | ||
|
|
@@ -1551,10 +1612,12 @@ function renderChart() { | |
| { dx: 0, dy: 65, anchor: 'middle' }, | ||
| ]; | ||
|
|
||
| // Helper to pick slots based on label count | ||
| // Helper to pick slots based on label count. | ||
| // For 2 labels, stack vertically (centered) to avoid horizontal offset | ||
| // that visually overlaps with adjacent nodes (spacingX=90, old dx=±70). | ||
| function pickSlots(pool: typeof aboveSlots, count: number) { | ||
| if (count === 1) return [pool[0]]; | ||
| if (count === 2) return [pool[1], pool[2]]; | ||
| if (count === 2) return [pool[0], { dx: 0, dy: pool[0].dy + 20, anchor: 'middle' }]; | ||
| if (count === 3) return [pool[1], pool[0], pool[2]]; | ||
| return Array.from({ length: count }, (_, i) => pool[i % pool.length]); | ||
| } | ||
|
|
||
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.
🧩 Analysis chain
🌐 Web query:
Apache Iceberg snapshot-log semantics: issnapshot-loga time-ordered history ofcurrent-snapshot-id, and can the samesnapshot-idappear multiple times afterrollbackToorsetCurrentSnapshot?💡 Result:
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]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]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:
Repository: lakekeeper/console-components
Length of output: 110
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 2306
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 2275
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 3556
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 3442
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 55
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 2172
🏁 Script executed:
Repository: lakekeeper/console-components
Length of output: 890
Use ancestry order instead of snapshot-log chronological order for main branch links.
Iceberg's
snapshot-logis a chronological record ofcurrent-snapshot-idchanges, where the same snapshot ID can legitimately reappear afterrollbackTo()orsetCurrentSnapshot(). Using this raw log order to draw graph links produces self-links or backward edges. Instead, filtermainBr.ancestrybylogIdsto get the correct topological ordering of snapshots natively committed to main.🛠️ Suggested change
📝 Committable suggestion
🤖 Prompt for AI Agents