From c985d44976e24d8171a1e20180c5448c912b49da Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Fri, 20 Mar 2026 20:22:52 +0100 Subject: [PATCH 1/3] fix: make multi-repo time attribution repo-aware When a project has multiple repos, time attribution broke in two ways: idle gaps from one repo were incorrectly applied to segments in another repo (precise mode), and commits were orphaned when a cross-repo checkout terminated the session they belonged to (both modes). Introduce synthetic checkouts from commit-based repo switches so that commits in different repos are never orphaned. Make all dedup, commit matching, and idle gap logic repo-aware by comparing branch+repo pairs instead of branch alone. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/timetrack/segment.go | 165 ++++++++--- internal/timetrack/segment_test.go | 443 +++++++++++++++++++++++++++++ internal/timetrack/timetrack.go | 8 +- 3 files changed, 582 insertions(+), 34 deletions(-) diff --git a/internal/timetrack/segment.go b/internal/timetrack/segment.go index f2edf27..badb3bd 100644 --- a/internal/timetrack/segment.go +++ b/internal/timetrack/segment.go @@ -12,45 +12,73 @@ import ( type idleGap struct { stop time.Time // activity_stop timestamp (last file change before idle) start time.Time // activity_start timestamp (first file change after idle) + repo string // repo this gap belongs to (empty = applies to all) } // buildIdleGaps pairs activity_stop and activity_start entries into idle gaps. +// Stops and starts are grouped by repo and paired within each group. // Gaps are sorted chronologically by stop time. func buildIdleGaps(stops []entry.ActivityStopEntry, starts []entry.ActivityStartEntry) []idleGap { - // Sort stops and starts by timestamp - sortedStops := make([]entry.ActivityStopEntry, len(stops)) - copy(sortedStops, stops) - sort.Slice(sortedStops, func(i, j int) bool { - return sortedStops[i].Timestamp.Before(sortedStops[j].Timestamp) - }) + // Group stops by repo + stopsByRepo := make(map[string][]entry.ActivityStopEntry) + for _, s := range stops { + stopsByRepo[s.Repo] = append(stopsByRepo[s.Repo], s) + } - sortedStarts := make([]entry.ActivityStartEntry, len(starts)) - copy(sortedStarts, starts) - sort.Slice(sortedStarts, func(i, j int) bool { - return sortedStarts[i].Timestamp.Before(sortedStarts[j].Timestamp) - }) + // Group starts by repo + startsByRepo := make(map[string][]entry.ActivityStartEntry) + for _, s := range starts { + startsByRepo[s.Repo] = append(startsByRepo[s.Repo], s) + } + + // Collect all repo keys + repos := make(map[string]bool) + for r := range stopsByRepo { + repos[r] = true + } - // Pair each stop with the next start that comes after it var gaps []idleGap - startIdx := 0 - for _, stop := range sortedStops { - // Find the first start after this stop - for startIdx < len(sortedStarts) && !sortedStarts[startIdx].Timestamp.After(stop.Timestamp) { - startIdx++ + for repo := range repos { + repoStops := stopsByRepo[repo] + repoStarts := startsByRepo[repo] + if len(repoStarts) == 0 { + continue } - if startIdx < len(sortedStarts) { - gaps = append(gaps, idleGap{ - stop: stop.Timestamp, - start: sortedStarts[startIdx].Timestamp, - }) - startIdx++ + + // Sort stops and starts by timestamp + sort.Slice(repoStops, func(i, j int) bool { + return repoStops[i].Timestamp.Before(repoStops[j].Timestamp) + }) + sort.Slice(repoStarts, func(i, j int) bool { + return repoStarts[i].Timestamp.Before(repoStarts[j].Timestamp) + }) + + // Pair each stop with the next start that comes after it + startIdx := 0 + for _, stop := range repoStops { + for startIdx < len(repoStarts) && !repoStarts[startIdx].Timestamp.After(stop.Timestamp) { + startIdx++ + } + if startIdx < len(repoStarts) { + gaps = append(gaps, idleGap{ + stop: stop.Timestamp, + start: repoStarts[startIdx].Timestamp, + repo: repo, + }) + startIdx++ + } } } + + // Sort gaps chronologically by stop time + sort.Slice(gaps, func(i, j int) bool { + return gaps[i].stop.Before(gaps[j].stop) + }) return gaps } // trimSegmentsByIdleGaps removes idle periods from checkout segments. -// For each segment, idle gaps that overlap are used to split or trim the segment. +// For each segment, only idle gaps matching the segment's repo are applied. func trimSegmentsByIdleGaps(segments []sessionSegment, stops []entry.ActivityStopEntry, starts []entry.ActivityStartEntry) []sessionSegment { if len(stops) == 0 || len(starts) == 0 { return segments @@ -63,12 +91,31 @@ func trimSegmentsByIdleGaps(segments []sessionSegment, stops []entry.ActivitySto var result []sessionSegment for _, seg := range segments { - trimmed := applyGapsToSegment(seg, gaps) + filtered := filterGapsByRepo(gaps, seg.repo) + trimmed := applyGapsToSegment(seg, filtered) result = append(result, trimmed...) } return result } +// filterGapsByRepo returns gaps that apply to the given segment repo. +// Rules: +// - Gap with empty repo → applies to all segments (backward compat / log deduction) +// - Segment with empty repo → affected by all gaps (conservative fallback) +// - Otherwise → gap applies only if repos match +func filterGapsByRepo(gaps []idleGap, segRepo string) []idleGap { + if segRepo == "" { + return gaps + } + var filtered []idleGap + for _, g := range gaps { + if g.repo == "" || g.repo == segRepo { + filtered = append(filtered, g) + } + } + return filtered +} + // applyGapsToSegment applies all overlapping idle gaps to a single segment, // potentially splitting it into multiple sub-segments. func applyGapsToSegment(seg sessionSegment, gaps []idleGap) []sessionSegment { @@ -134,12 +181,52 @@ type sessionSegment struct { message string // commit message, empty for uncommitted trailing segment } +// buildSyntheticCheckouts creates synthetic checkout entries from commits to +// detect repo switches. When consecutive commits are on different repos, a +// synthetic checkout is placed at the midpoint to split the timeline. +func buildSyntheticCheckouts(commits []entry.CommitEntry) []entry.CheckoutEntry { + if len(commits) == 0 { + return nil + } + + sorted := make([]entry.CommitEntry, len(commits)) + copy(sorted, commits) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Timestamp.Before(sorted[j].Timestamp) + }) + + var synthetic []entry.CheckoutEntry + for i := 1; i < len(sorted); i++ { + prev := sorted[i-1] + curr := sorted[i] + if prev.Repo == "" || curr.Repo == "" { + continue + } + if prev.Repo == curr.Repo { + continue + } + // Different repos — create synthetic checkout at midpoint + mid := prev.Timestamp.Add(curr.Timestamp.Sub(prev.Timestamp) / 2) + synthetic = append(synthetic, entry.CheckoutEntry{ + ID: "synthetic-" + curr.ID, + Type: "checkout", + Timestamp: mid, + Next: curr.Branch, + Repo: curr.Repo, + }) + } + return synthetic +} + // buildCheckoutSegments splits checkout sessions by commits to produce // finer-grained time segments. Each commit creates a segment from the previous // boundary to the commit timestamp. Time is attributed backwards from the commit // — work before a commit is attributed to that commit. Trailing time after the // last commit becomes an unnamed segment (uncommitted work). // +// Synthetic checkouts are injected from commit-based repo switches so that +// commits in different repos are never orphaned. +// // When no commits exist within a session, the entire session becomes one segment. func buildCheckoutSegments( checkouts []entry.CheckoutEntry, @@ -149,18 +236,23 @@ func buildCheckoutSegments( ) []sessionSegment { loc := now.Location() + // Merge synthetic checkouts from commit-based repo switches + synthetic := buildSyntheticCheckouts(commits) + allCheckouts := append(checkouts, synthetic...) + // Sort checkouts chronologically - sorted := make([]entry.CheckoutEntry, len(checkouts)) - copy(sorted, checkouts) + sorted := make([]entry.CheckoutEntry, len(allCheckouts)) + copy(sorted, allCheckouts) sort.Slice(sorted, func(i, j int) bool { return sorted[i].Timestamp.Before(sorted[j].Timestamp) }) - // Deduplicate: skip consecutive checkouts to the same branch + // Deduplicate: skip consecutive checkouts to the same branch+repo if len(sorted) > 0 { deduped := []entry.CheckoutEntry{sorted[0]} for i := 1; i < len(sorted); i++ { - if cleanBranchName(sorted[i].Next) != cleanBranchName(sorted[i-1].Next) { + if cleanBranchName(sorted[i].Next) != cleanBranchName(sorted[i-1].Next) || + sorted[i].Repo != sorted[i-1].Repo { deduped = append(deduped, sorted[i]) } } @@ -182,6 +274,7 @@ func buildCheckoutSegments( if lastBeforeIdx >= 0 { pairs = append(pairs, checkoutRange{ branch: cleanBranchName(sorted[lastBeforeIdx].Next), + repo: sorted[lastBeforeIdx].Repo, from: monthStart, }) } @@ -190,6 +283,7 @@ func buildCheckoutSegments( if c.Timestamp.After(monthStart) && !c.Timestamp.After(monthEnd) { pairs = append(pairs, checkoutRange{ branch: cleanBranchName(c.Next), + repo: c.Repo, from: c.Timestamp, }) } @@ -224,13 +318,14 @@ func buildCheckoutSegments( continue } - // Find commits within this session's time range on the same branch + // Find commits within this session's time range on the same branch+repo var sessionCommits []entry.CommitEntry for _, c := range sortedCommits { if c.Timestamp.Before(p.from) || !c.Timestamp.Before(p.to) { continue } - if cleanBranchName(c.Branch) == p.branch { + if cleanBranchName(c.Branch) == p.branch && + (c.Repo == "" || p.repo == "" || c.Repo == p.repo) { sessionCommits = append(sessionCommits, c) } } @@ -239,6 +334,7 @@ func buildCheckoutSegments( // No commits — single segment for the whole session segments = append(segments, sessionSegment{ branch: p.branch, + repo: p.repo, from: p.from, to: p.to, }) @@ -250,9 +346,13 @@ func buildCheckoutSegments( for _, c := range sessionCommits { commitTime := c.Timestamp.Truncate(time.Minute) if commitTime.After(boundary) { + repo := c.Repo + if repo == "" { + repo = p.repo + } segments = append(segments, sessionSegment{ branch: p.branch, - repo: c.Repo, + repo: repo, from: boundary, to: commitTime, message: c.Message, @@ -265,6 +365,7 @@ func buildCheckoutSegments( if boundary.Before(p.to) { segments = append(segments, sessionSegment{ branch: p.branch, + repo: p.repo, from: boundary, to: p.to, }) diff --git a/internal/timetrack/segment_test.go b/internal/timetrack/segment_test.go index 19d1af3..af4ba75 100644 --- a/internal/timetrack/segment_test.go +++ b/internal/timetrack/segment_test.go @@ -397,3 +397,446 @@ func TestTrimSegmentsByIdleGaps_NoOverlap(t *testing.T) { assert.Len(t, result, 1) assert.Equal(t, segments[0], result[0]) } + +// --- Synthetic checkout tests --- + +func TestBuildSyntheticCheckouts_NoCommits(t *testing.T) { + result := buildSyntheticCheckouts(nil) + assert.Nil(t, result) +} + +func TestBuildSyntheticCheckouts_SingleRepo(t *testing.T) { + commits := []entry.CommitEntry{ + {ID: "c1", Timestamp: t9am, Branch: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: t10am, Branch: "main", Repo: "/repoA"}, + {ID: "c3", Timestamp: t11am, Branch: "feat", Repo: "/repoA"}, + } + result := buildSyntheticCheckouts(commits) + assert.Nil(t, result) +} + +func TestBuildSyntheticCheckouts_MultiRepo(t *testing.T) { + commits := []entry.CommitEntry{ + {ID: "c1", Timestamp: t9am, Branch: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: t11am, Branch: "feat", Repo: "/repoB"}, + } + result := buildSyntheticCheckouts(commits) + assert.Len(t, result, 1) + + // Midpoint of 9:00 and 11:00 = 10:00 + assert.Equal(t, t10am, result[0].Timestamp) + assert.Equal(t, "feat", result[0].Next) + assert.Equal(t, "/repoB", result[0].Repo) +} + +func TestBuildSyntheticCheckouts_ConsecutiveSameRepo(t *testing.T) { + commits := []entry.CommitEntry{ + {ID: "c1", Timestamp: t9am, Branch: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: t10am, Branch: "feat", Repo: "/repoA"}, + {ID: "c3", Timestamp: t11am, Branch: "main", Repo: "/repoA"}, + } + result := buildSyntheticCheckouts(commits) + assert.Nil(t, result) +} + +func TestBuildSyntheticCheckouts_EmptyRepoSkipped(t *testing.T) { + commits := []entry.CommitEntry{ + {ID: "c1", Timestamp: t9am, Branch: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: t10am, Branch: "feat", Repo: ""}, + {ID: "c3", Timestamp: t11am, Branch: "main", Repo: "/repoB"}, + } + // c1→c2: c2 has empty repo, skip + // c2→c3: c2 has empty repo, skip + result := buildSyntheticCheckouts(commits) + assert.Nil(t, result) +} + +// --- Repo-aware deduplication tests --- + +func TestBuildCheckoutSegments_SameBranchDifferentRepos(t *testing.T) { + year, month := 2025, time.January + daysInMonth := 31 + + // Two checkouts to "main" but in different repos — should NOT be deduplicated + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoB"}, + {ID: "c3", Timestamp: time.Date(2025, 1, 2, 15, 0, 0, 0, time.UTC), Next: "feat", Repo: "/repoA"}, + } + + segments := buildCheckoutSegments(checkouts, nil, year, month, daysInMonth, afterMonth(year, month)) + + // Should have 3 segments: main@repoA, main@repoB, feat@repoA + assert.Equal(t, 3, len(segments)) + assert.Equal(t, "main", segments[0].branch) + assert.Equal(t, "/repoA", segments[0].repo) + assert.Equal(t, "main", segments[1].branch) + assert.Equal(t, "/repoB", segments[1].repo) + assert.Equal(t, "feat", segments[2].branch) + assert.Equal(t, "/repoA", segments[2].repo) +} + +// --- Repo-aware commit matching tests --- + +func TestBuildCheckoutSegments_CommitMatchesByRepo(t *testing.T) { + year, month := 2025, time.January + daysInMonth := 31 + + // Two overlapping sessions on "main" in different repos + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoB"}, + } + + // Commit on main in repoB — should only match the repoB session + commits := []entry.CommitEntry{ + {ID: "cm1", Timestamp: time.Date(2025, 1, 2, 13, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoB", Message: "fix in repoB"}, + } + + segments := buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, afterMonth(year, month)) + + // repoA session: 9:00-12:00, no commits → single segment + repoASegs := filterSegmentsByRepo(segments, "/repoA") + assert.Equal(t, 1, len(repoASegs)) + assert.Equal(t, "", repoASegs[0].message) // no commit message + + // repoB session: 12:00-end, split by commit at 13:00 + repoBSegs := filterSegmentsByRepo(segments, "/repoB") + assert.Equal(t, 2, len(repoBSegs)) + assert.Equal(t, "fix in repoB", repoBSegs[0].message) + assert.Equal(t, "", repoBSegs[1].message) // trailing +} + +// --- Repo-aware idle gap tests --- + +func TestTrimSegmentsByIdleGaps_DifferentRepoNotApplied(t *testing.T) { + segments := []sessionSegment{ + {branch: "main", repo: "/repoA", from: t9am, to: t12pm, message: "work"}, + } + + // Idle gap from repoB — should NOT affect repoA segment + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: t10am, Repo: "/repoB"}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: t11am, Repo: "/repoB"}, + } + + result := trimSegmentsByIdleGaps(segments, stops, starts) + assert.Len(t, result, 1) + assert.Equal(t, t9am, result[0].from) + assert.Equal(t, t12pm, result[0].to) +} + +func TestTrimSegmentsByIdleGaps_SameRepoApplied(t *testing.T) { + segments := []sessionSegment{ + {branch: "main", repo: "/repoA", from: t9am, to: t12pm, message: "work"}, + } + + // Idle gap from repoA — SHOULD affect repoA segment + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: t10am, Repo: "/repoA"}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: t11am, Repo: "/repoA"}, + } + + result := trimSegmentsByIdleGaps(segments, stops, starts) + assert.Len(t, result, 2) + assert.Equal(t, t9am, result[0].from) + assert.Equal(t, t10am, result[0].to) + assert.Equal(t, t11am, result[1].from) + assert.Equal(t, t12pm, result[1].to) +} + +func TestTrimSegmentsByIdleGaps_MultiRepoMixed(t *testing.T) { + segments := []sessionSegment{ + {branch: "main", repo: "/repoA", from: t9am, to: t12pm, message: "workA"}, + {branch: "feat", repo: "/repoB", from: t9am, to: t12pm, message: "workB"}, + } + + // repoA idle gap 10:00-11:00, repoB idle gap 9:30-10:30 + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: t10am, Repo: "/repoA"}, + {ID: "s2", Timestamp: t930, Repo: "/repoB"}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: t11am, Repo: "/repoA"}, + {ID: "a2", Timestamp: t1030, Repo: "/repoB"}, + } + + result := trimSegmentsByIdleGaps(segments, stops, starts) + + // repoA: [9:00-10:00, 11:00-12:00] + repoAResult := filterSegmentsByRepo(result, "/repoA") + assert.Len(t, repoAResult, 2) + assert.Equal(t, t9am, repoAResult[0].from) + assert.Equal(t, t10am, repoAResult[0].to) + assert.Equal(t, t11am, repoAResult[1].from) + assert.Equal(t, t12pm, repoAResult[1].to) + + // repoB: [9:00-9:30, 10:30-12:00] + repoBResult := filterSegmentsByRepo(result, "/repoB") + assert.Len(t, repoBResult, 2) + assert.Equal(t, t9am, repoBResult[0].from) + assert.Equal(t, t930, repoBResult[0].to) + assert.Equal(t, t1030, repoBResult[1].from) + assert.Equal(t, t12pm, repoBResult[1].to) +} + +func TestTrimSegmentsByIdleGaps_EmptyRepoBackwardCompat(t *testing.T) { + // Segment with empty repo should be affected by ALL gaps + segments := []sessionSegment{ + {branch: "main", repo: "", from: t9am, to: t12pm, message: "work"}, + } + + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: t10am, Repo: "/repoA"}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: t11am, Repo: "/repoA"}, + } + + result := trimSegmentsByIdleGaps(segments, stops, starts) + assert.Len(t, result, 2) + assert.Equal(t, t9am, result[0].from) + assert.Equal(t, t10am, result[0].to) + assert.Equal(t, t11am, result[1].from) + assert.Equal(t, t12pm, result[1].to) +} + +func TestBuildIdleGaps_PairsPerRepo(t *testing.T) { + // Stops and starts from different repos should be paired independently + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: t9am, Repo: "/repoA"}, + {ID: "s2", Timestamp: t930, Repo: "/repoB"}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: t10am, Repo: "/repoA"}, + {ID: "a2", Timestamp: t1030, Repo: "/repoB"}, + } + + gaps := buildIdleGaps(stops, starts) + assert.Len(t, gaps, 2) + + // Both repos should have their own gap + repoAGaps := filterGapsByRepoExact(gaps, "/repoA") + assert.Len(t, repoAGaps, 1) + assert.Equal(t, t9am, repoAGaps[0].stop) + assert.Equal(t, t10am, repoAGaps[0].start) + + repoBGaps := filterGapsByRepoExact(gaps, "/repoB") + assert.Len(t, repoBGaps, 1) + assert.Equal(t, t930, repoBGaps[0].stop) + assert.Equal(t, t1030, repoBGaps[0].start) +} + +func TestBuildIdleGaps_CrossRepoPairingPrevented(t *testing.T) { + // Stop from repoA should NOT pair with start from repoB + stops := []entry.ActivityStopEntry{ + {ID: "s1", Timestamp: t9am, Repo: "/repoA"}, + } + starts := []entry.ActivityStartEntry{ + {ID: "a1", Timestamp: t10am, Repo: "/repoB"}, + } + + gaps := buildIdleGaps(stops, starts) + // repoA has stop but no start in its repo → no gap + assert.Len(t, gaps, 0) +} + +func TestFilterGapsByRepo(t *testing.T) { + gaps := []idleGap{ + {stop: t9am, start: t10am, repo: "/repoA"}, + {stop: t10am, start: t11am, repo: "/repoB"}, + {stop: t11am, start: t12pm, repo: ""}, // empty repo = universal + } + + // Filter for repoA: should get repoA gap + empty-repo gap + filtered := filterGapsByRepo(gaps, "/repoA") + assert.Len(t, filtered, 2) + assert.Equal(t, "/repoA", filtered[0].repo) + assert.Equal(t, "", filtered[1].repo) + + // Filter for repoB: should get repoB gap + empty-repo gap + filtered = filterGapsByRepo(gaps, "/repoB") + assert.Len(t, filtered, 2) + assert.Equal(t, "/repoB", filtered[0].repo) + assert.Equal(t, "", filtered[1].repo) + + // Filter with empty repo: should get ALL gaps + filtered = filterGapsByRepo(gaps, "") + assert.Len(t, filtered, 3) +} + +// --- Integration tests --- + +func TestBuildCheckoutSegments_MultiRepoWithCommits(t *testing.T) { + year, month := 2025, time.January + daysInMonth := 31 + + // Single checkout to main@repoA, then commits alternate repos + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + } + + commits := []entry.CommitEntry{ + {ID: "cm1", Timestamp: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA", Message: "commit in A"}, + {ID: "cm2", Timestamp: time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), Branch: "feat", Repo: "/repoB", Message: "commit in B"}, + {ID: "cm3", Timestamp: time.Date(2025, 1, 2, 14, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA", Message: "back to A"}, + } + + now := time.Date(2025, 1, 2, 16, 0, 0, 0, time.UTC) + segments := buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, now) + + // Synthetic checkouts injected at midpoints: + // cm1@repoA(10:00) → cm2@repoB(12:00): midpoint=11:00, checkout to feat@repoB + // cm2@repoB(12:00) → cm3@repoA(14:00): midpoint=13:00, checkout to main@repoA + // + // Timeline: + // main@repoA 9:00-11:00 (commit at 10:00 splits: [9:00-10:00 "commit in A", 10:00-11:00 trailing]) + // feat@repoB 11:00-13:00 (commit at 12:00 splits: [11:00-12:00 "commit in B", 12:00-13:00 trailing]) + // main@repoA 13:00-16:00 (commit at 14:00 splits: [13:00-14:00 "back to A", 14:00-16:00 trailing]) + + repoASegs := filterSegmentsByRepo(segments, "/repoA") + repoBSegs := filterSegmentsByRepo(segments, "/repoB") + + // repoA: 4 segments [9:00-10:00, 10:00-11:00, 13:00-14:00, 14:00-16:00] + assert.Equal(t, 4, len(repoASegs), "repoA segments") + assert.Equal(t, "commit in A", repoASegs[0].message) + assert.Equal(t, time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), repoASegs[0].from) + assert.Equal(t, time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), repoASegs[0].to) + assert.Equal(t, "", repoASegs[1].message) // trailing + assert.Equal(t, time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), repoASegs[1].from) + assert.Equal(t, time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), repoASegs[1].to) + assert.Equal(t, "back to A", repoASegs[2].message) + assert.Equal(t, time.Date(2025, 1, 2, 13, 0, 0, 0, time.UTC), repoASegs[2].from) + assert.Equal(t, time.Date(2025, 1, 2, 14, 0, 0, 0, time.UTC), repoASegs[2].to) + assert.Equal(t, "", repoASegs[3].message) // trailing + assert.Equal(t, time.Date(2025, 1, 2, 14, 0, 0, 0, time.UTC), repoASegs[3].from) + assert.Equal(t, time.Date(2025, 1, 2, 16, 0, 0, 0, time.UTC), repoASegs[3].to) + + // repoB: 2 segments [11:00-12:00, 12:00-13:00] + assert.Equal(t, 2, len(repoBSegs), "repoB segments") + assert.Equal(t, "feat", repoBSegs[0].branch) + assert.Equal(t, "commit in B", repoBSegs[0].message) + assert.Equal(t, time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), repoBSegs[0].from) + assert.Equal(t, time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), repoBSegs[0].to) + assert.Equal(t, "", repoBSegs[1].message) // trailing + assert.Equal(t, time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), repoBSegs[1].from) + assert.Equal(t, time.Date(2025, 1, 2, 13, 0, 0, 0, time.UTC), repoBSegs[1].to) + + // Verify no time double-counting: total = 9:00-16:00 = 420 minutes + totalMins := 0 + for _, s := range segments { + totalMins += int(s.to.Sub(s.from).Minutes()) + } + assert.Equal(t, 420, totalMins) +} + +func TestBuildCheckoutSegments_CommitRepoFallbackToCheckoutRange(t *testing.T) { + year, month := 2025, time.January + daysInMonth := 31 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + {ID: "c2", Timestamp: time.Date(2025, 1, 2, 15, 0, 0, 0, time.UTC), Next: "feat", Repo: "/repoA"}, + } + + // Commit with empty repo — should inherit /repoA from the checkout range + commits := []entry.CommitEntry{ + {ID: "cm1", Timestamp: time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), Branch: "main", Repo: "", Message: "legacy commit"}, + } + + segments := buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, afterMonth(year, month)) + + mainSegs := filterSegments(segments, "main") + assert.Equal(t, 2, len(mainSegs)) + + // Commit segment should inherit repo from checkout range + assert.Equal(t, "/repoA", mainSegs[0].repo) + assert.Equal(t, "legacy commit", mainSegs[0].message) + + // Trailing segment should also have checkout range's repo + assert.Equal(t, "/repoA", mainSegs[1].repo) +} + +func TestBuildReport_MultiRepoNoDoubleCounting(t *testing.T) { + year, month := 2025, time.January + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 = 480 min + + // Single checkout, commits alternate between two repos + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + } + + commits := []entry.CommitEntry{ + {ID: "cm1", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA", Message: "work in A"}, + {ID: "cm2", Timestamp: time.Date(2025, 1, 2, 13, 0, 0, 0, time.UTC), Branch: "feat", Repo: "/repoB", Message: "work in B"}, + {ID: "cm3", Timestamp: time.Date(2025, 1, 2, 15, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA", Message: "more A"}, + } + + now := afterMonth(year, month) + report := BuildReport(checkouts, nil, commits, days, year, month, now, nil) + + // Total across all rows should equal schedule capacity (480 min), not exceed it + totalMins := 0 + for _, row := range report.Rows { + totalMins += row.TotalMinutes + } + assert.Equal(t, 480, totalMins, "total time should equal schedule capacity without double-counting") + + // Should have two rows: main (repoA time) and feat (repoB time) + assert.Equal(t, 2, len(report.Rows)) +} + +func TestBuildDetailedReport_MultiRepoWithCommits(t *testing.T) { + year, month := 2025, time.January + from := time.Date(year, month, 1, 0, 0, 0, 0, time.UTC) + to := time.Date(year, month, 31, 0, 0, 0, 0, time.UTC) + days := []schedule.DaySchedule{workday(year, month, 2)} // 9-17 = 480 min + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + } + + commits := []entry.CommitEntry{ + {ID: "cm1", Timestamp: time.Date(2025, 1, 2, 11, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA", Message: "work in A"}, + {ID: "cm2", Timestamp: time.Date(2025, 1, 2, 13, 0, 0, 0, time.UTC), Branch: "feat", Repo: "/repoB", Message: "work in B"}, + {ID: "cm3", Timestamp: time.Date(2025, 1, 2, 15, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA", Message: "more A"}, + } + + report := BuildDetailedReport(checkouts, nil, commits, days, from, to, afterMonth(year, month)) + + // Total across all rows should equal 480 min + totalMins := 0 + for _, row := range report.Rows { + totalMins += row.TotalMinutes + } + assert.Equal(t, 480, totalMins, "total time should equal schedule capacity") + + // Should have entries for both main and feat + assert.Equal(t, 2, len(report.Rows)) +} + +// --- Test helpers --- + +func filterSegmentsByRepo(segments []sessionSegment, repo string) []sessionSegment { + var result []sessionSegment + for _, s := range segments { + if s.repo == repo { + result = append(result, s) + } + } + return result +} + +func filterGapsByRepoExact(gaps []idleGap, repo string) []idleGap { + var result []idleGap + for _, g := range gaps { + if g.repo == repo { + result = append(result, g) + } + } + return result +} diff --git a/internal/timetrack/timetrack.go b/internal/timetrack/timetrack.go index 8012256..99f6012 100644 --- a/internal/timetrack/timetrack.go +++ b/internal/timetrack/timetrack.go @@ -197,11 +197,12 @@ func buildCheckoutBucket( return sorted[i].Timestamp.Before(sorted[j].Timestamp) }) - // Deduplicate: skip consecutive checkouts to the same branch + // Deduplicate: skip consecutive checkouts to the same branch+repo if len(sorted) > 0 { deduped := []entry.CheckoutEntry{sorted[0]} for i := 1; i < len(sorted); i++ { - if cleanBranchName(sorted[i].Next) != cleanBranchName(sorted[i-1].Next) { + if cleanBranchName(sorted[i].Next) != cleanBranchName(sorted[i-1].Next) || + sorted[i].Repo != sorted[i-1].Repo { deduped = append(deduped, sorted[i]) } } @@ -222,6 +223,7 @@ func buildCheckoutBucket( if lastBeforeIdx >= 0 { pairs = append(pairs, checkoutRange{ branch: cleanBranchName(sorted[lastBeforeIdx].Next), + repo: sorted[lastBeforeIdx].Repo, from: monthStart, }) } @@ -230,6 +232,7 @@ func buildCheckoutBucket( if c.Timestamp.After(monthStart) && !c.Timestamp.After(monthEnd) { pairs = append(pairs, checkoutRange{ branch: cleanBranchName(c.Next), + repo: c.Repo, from: c.Timestamp, }) } @@ -487,6 +490,7 @@ func BuildDetailedReport( type checkoutRange struct { branch string + repo string from time.Time to time.Time } From 00c6dfedaf7c62775dbe1d048cae7684de23bd84 Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Mon, 23 Mar 2026 15:19:12 +0100 Subject: [PATCH 2/3] fix: prevent caller slice mutation in buildCheckoutSegments Use safe allocation (make + append) instead of append(checkouts, synthetic...) which could mutate the caller's backing array. Also set the Previous field on synthetic checkouts and add per-row minute assertions plus a dedicated mutation regression test. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/timetrack/segment.go | 10 ++++--- internal/timetrack/segment_test.go | 42 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/internal/timetrack/segment.go b/internal/timetrack/segment.go index badb3bd..d5baccc 100644 --- a/internal/timetrack/segment.go +++ b/internal/timetrack/segment.go @@ -211,6 +211,7 @@ func buildSyntheticCheckouts(commits []entry.CommitEntry) []entry.CheckoutEntry ID: "synthetic-" + curr.ID, Type: "checkout", Timestamp: mid, + Previous: prev.Branch, Next: curr.Branch, Repo: curr.Repo, }) @@ -236,13 +237,14 @@ func buildCheckoutSegments( ) []sessionSegment { loc := now.Location() - // Merge synthetic checkouts from commit-based repo switches + // Merge synthetic checkouts from commit-based repo switches. + // Allocate a new slice to avoid mutating the caller's backing array. synthetic := buildSyntheticCheckouts(commits) - allCheckouts := append(checkouts, synthetic...) + sorted := make([]entry.CheckoutEntry, 0, len(checkouts)+len(synthetic)) + sorted = append(sorted, checkouts...) + sorted = append(sorted, synthetic...) // Sort checkouts chronologically - sorted := make([]entry.CheckoutEntry, len(allCheckouts)) - copy(sorted, allCheckouts) sort.Slice(sorted, func(i, j int) bool { return sorted[i].Timestamp.Before(sorted[j].Timestamp) }) diff --git a/internal/timetrack/segment_test.go b/internal/timetrack/segment_test.go index af4ba75..b8808fc 100644 --- a/internal/timetrack/segment_test.go +++ b/internal/timetrack/segment_test.go @@ -425,8 +425,10 @@ func TestBuildSyntheticCheckouts_MultiRepo(t *testing.T) { // Midpoint of 9:00 and 11:00 = 10:00 assert.Equal(t, t10am, result[0].Timestamp) + assert.Equal(t, "main", result[0].Previous) assert.Equal(t, "feat", result[0].Next) assert.Equal(t, "/repoB", result[0].Repo) + assert.Equal(t, "checkout", result[0].Type) } func TestBuildSyntheticCheckouts_ConsecutiveSameRepo(t *testing.T) { @@ -788,6 +790,16 @@ func TestBuildReport_MultiRepoNoDoubleCounting(t *testing.T) { // Should have two rows: main (repoA time) and feat (repoB time) assert.Equal(t, 2, len(report.Rows)) + + // Synthetic checkouts: midpoint at 12:00, midpoint at 14:00 + // main@repoA: 9:00-12:00 (180 min) + 14:00-17:00 (180 min) = 360 min + // feat@repoB: 12:00-14:00 = 120 min + rowMain := findRow(report, "main") + rowFeat := findRow(report, "feat") + assert.NotNil(t, rowMain) + assert.NotNil(t, rowFeat) + assert.Equal(t, 360, rowMain.TotalMinutes) + assert.Equal(t, 120, rowFeat.TotalMinutes) } func TestBuildDetailedReport_MultiRepoWithCommits(t *testing.T) { @@ -817,6 +829,36 @@ func TestBuildDetailedReport_MultiRepoWithCommits(t *testing.T) { // Should have entries for both main and feat assert.Equal(t, 2, len(report.Rows)) + + // Verify per-row distribution matches synthetic checkout midpoints + rowMain := findDetailedRow(report, "main") + rowFeat := findDetailedRow(report, "feat") + assert.NotNil(t, rowMain) + assert.NotNil(t, rowFeat) + assert.Equal(t, 360, rowMain.TotalMinutes) + assert.Equal(t, 120, rowFeat.TotalMinutes) +} + +func TestBuildCheckoutSegments_DoesNotMutateCallerSlice(t *testing.T) { + year, month := 2025, time.January + daysInMonth := 31 + + checkouts := []entry.CheckoutEntry{ + {ID: "c1", Timestamp: time.Date(2025, 1, 2, 9, 0, 0, 0, time.UTC), Next: "main", Repo: "/repoA"}, + } + commits := []entry.CommitEntry{ + {ID: "cm1", Timestamp: time.Date(2025, 1, 2, 10, 0, 0, 0, time.UTC), Branch: "main", Repo: "/repoA"}, + {ID: "cm2", Timestamp: time.Date(2025, 1, 2, 12, 0, 0, 0, time.UTC), Branch: "feat", Repo: "/repoB"}, + } + + origLen := len(checkouts) + origFirst := checkouts[0] + + buildCheckoutSegments(checkouts, commits, year, month, daysInMonth, afterMonth(year, month)) + + // Caller's slice must not be mutated by synthetic checkout injection + assert.Equal(t, origLen, len(checkouts)) + assert.Equal(t, origFirst, checkouts[0]) } // --- Test helpers --- From b5a90ad8fe7847c734e8a430a0c5f4850abac4ce Mon Sep 17 00:00:00 2001 From: Dawid Zbinski Date: Mon, 23 Mar 2026 15:56:23 +0100 Subject: [PATCH 3/3] docs: clarify buildCheckoutBucket and buildIdleGaps behavior Address code review feedback with two inline comment additions: - buildCheckoutBucket: note that it does not inject synthetic checkouts from commits, directing readers to the segment-based pipeline instead - buildIdleGaps: explain why only stop-repos are iterated (starts without a preceding stop cannot form a gap) Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/timetrack/segment.go | 3 ++- internal/timetrack/timetrack.go | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/timetrack/segment.go b/internal/timetrack/segment.go index d5baccc..00565c4 100644 --- a/internal/timetrack/segment.go +++ b/internal/timetrack/segment.go @@ -31,7 +31,8 @@ func buildIdleGaps(stops []entry.ActivityStopEntry, starts []entry.ActivityStart startsByRepo[s.Repo] = append(startsByRepo[s.Repo], s) } - // Collect all repo keys + // Collect repo keys from stops only — a start without a preceding stop + // cannot form a gap, so repos with only starts are correctly skipped. repos := make(map[string]bool) for r := range stopsByRepo { repos[r] = true diff --git a/internal/timetrack/timetrack.go b/internal/timetrack/timetrack.go index 99f6012..bbc25b7 100644 --- a/internal/timetrack/timetrack.go +++ b/internal/timetrack/timetrack.go @@ -184,6 +184,8 @@ func buildLogBucket(logs []entry.Entry, year int, month time.Month) (map[string] // buildCheckoutBucket computes per-branch, per-day minutes from checkout entries // clipped to schedule windows. Schedule window times are interpreted in the // timezone of `now` (the user's local timezone). +// NOTE: Does not inject synthetic checkouts from commits. For multi-repo-aware +// attribution, use buildCheckoutSegments → buildSegmentBucket instead. func buildCheckoutBucket( checkouts []entry.CheckoutEntry, year int, month time.Month, daysInMonth int,