fix(efficiency): count add-then-delete bytes as waste

CHANGELOG.md

@@ -8,6 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Changed
+- Efficiency analysis now counts a file that is added in one layer and deleted
+  in a later layer as wasted space. Deleting a file in a later layer records a
+  whiteout but does not reclaim the earlier layer's bytes — they remain stored
+  in the image and are transferred on every pull. Such files now contribute to
+  `wastedBytes`, appear in the wasted-files list (JSON export, TUI waste
+  navigator `w`), and lower the efficiency score. Scores may drop for images
+  built with the common "download, build, then `rm`" pattern; the score formula,
+  CI rules, thresholds, and exit codes are unchanged.
 - The interactive file tree now caches its flatten/filter/sort result between
   redraws, so holding a navigation key or scrolling a large image's tree stays
   responsive instead of recomputing the whole tree on every frame. The cache

docs/ci-integration.md

@@ -24,6 +24,12 @@ Rules come from `.layerx.yaml` (in the working directory) and can be
 overridden inline with CLI flags for the three global thresholds. If
 `.layerx.yaml` is missing, built-in defaults apply.
 
+The efficiency figure counts a file that is added in one layer and deleted in
+a later one as wasted — the delete records a whiteout but never reclaims the
+earlier layer's bytes, so they still ship. A score may therefore be lower than
+a tool that only counts files duplicated across layers. The rules and
+thresholds themselves are unchanged.
+
 Two ways to invoke CI mode:
 
 ```bash

docs/configuration.md

@@ -123,6 +123,11 @@ decoder is strict.
 Range validation rejects `NaN`, `±Inf`, negative byte counts, and floats
 outside `[0, 1]`.
 
+Wasted bytes include files added in one layer and deleted in a later one: the
+delete records a whiteout but never reclaims the earlier layer's bytes, so they
+still ship in the image. A score may be lower than a tool that counts only
+files duplicated across layers.
+
 ### `path-rules` — flat form (mapping)
 
 | Field | Type | Default | Required | Description |

docs/json-export.md

@@ -122,7 +122,7 @@ Schema version `1.0.1`. Pretty-printed with two-space indent.
 | Field | Type | Description |
 |---|---|---|
 | `score` | float64 | `1.0 − wastedBytes / (liveBytes + wastedBytes)`, in `[0.0, 1.0]`. Higher is better. |
-| `wastedBytes` | int64 | Bytes duplicated across layers (file-path appears in more than one layer; only first occurrence counts as "live"). |
+| `wastedBytes` | int64 | Bytes that ship in the image but are not live in the final filesystem. Two cases: a file rewritten across layers (only the last copy is live; earlier copies are waste), and a file added in one layer then deleted in a later one (the delete records a whiteout but never reclaims the earlier layer's bytes, so the whole file is waste). |
 | `wastedFiles` | array | Per-file waste breakdown. Always `[]` when empty (never `null`). Sorted by `totalWasted` descending, then `path` ascending. |
 
 #### `efficiency.wastedFiles[]`

image/efficiency.go

@@ -18,10 +18,12 @@ type EfficiencyResult struct {
 // layers. It stacks layers internally; prefer EfficiencyFromAnalysis when the
 // caller already has stacked trees.
 //
-// A file at the same path counted across runs separated by deletion (whiteout
-// or opaque whiteout) is NOT considered wasted: the deleted copy was properly
-// cleaned up before the new copy appeared. Within a single run (no deletion
-// in between), all but the last occurrence are wasted.
+// Within a run of a path's occurrences (a span with no deletion in between),
+// all but the last occurrence are wasted — the last is the copy that survives.
+// A run that ends in a deletion (whiteout or opaque whiteout) has ALL of its
+// occurrences wasted: the bytes were written into earlier layers, still ship in
+// the image, and are never reclaimed by the later deletion, yet none of them
+// survive into the final filesystem.
 func Efficiency(layers []Layer) *EfficiencyResult {
 	if len(layers) == 0 {
 		return &EfficiencyResult{Score: 1.0}
@@ -56,6 +58,16 @@ type efficiencyOccurrence struct {
 	size     int64
 }
 
+// pathRun is one contiguous span of a path's Added/Modified occurrences.
+// endedInDeletion records why the span closed: true when a whiteout, opaque
+// whiteout, or absence removed the path (nothing from the run survives into the
+// final image), false when the run reached the top of the layer stack still
+// live. The charge rule differs between the two — see computeEfficiency.
+type pathRun struct {
+	occ             []efficiencyOccurrence
+	endedInDeletion bool
+}
+
 func computeEfficiency(layers []Layer, stacked []*FileTree) *EfficiencyResult {
 	// Build a path→FileNode index per stacked snapshot once. pathRuns then does
 	// O(1) lookups instead of recursing through the tree once per (path,
@@ -89,21 +101,36 @@ func computeEfficiency(layers []Layer, stacked []*FileTree) *EfficiencyResult {
 		var pathWaste int64
 		var occurrenceCount int
 		for _, run := range runs {
-			for _, occ := range run {
-				// LayerCount documents "how many layers contributed bytes".
-				// Zero-size occurrences (hardlink replacements that extend a
-				// run only to keep the earlier real-file bytes chargeable)
-				// are not byte-contributors and must not inflate the count.
-				if occ.size > 0 {
-					occurrenceCount++
+			// A run that ended in deletion ships every one of its copies with
+			// nothing surviving into the final image, so all occurrences are
+			// waste. A run still live at the top of the stack keeps its last
+			// occurrence (the copy present in the image); only the earlier,
+			// shadowed copies are waste.
+			charged := run.occ
+			if !run.endedInDeletion {
+				if len(run.occ) < 2 {
+					// Single live occurrence with no prior copy in this run: no
+					// waste, and the path's reinstall copy must not inflate
+					// LayerCount for waste entries produced by earlier deleted runs.
+					continue
 				}
+				charged = run.occ[:len(run.occ)-1]
 			}
-			if len(run) < 2 {
-				continue
-			}
-			for _, occ := range run[:len(run)-1] {
+			for _, occ := range charged {
 				pathWaste += occ.size
 			}
+			// LayerCount counts byte-contributing occurrences across charged
+			// copies only. For deleted runs, every occurrence is charged. For
+			// live runs, the surviving last copy is excluded from charged but
+			// is still a real byte-contributor visible to the user, so include
+			// all non-zero occurrences in the run (not just the charged slice).
+			// The single-occurrence live run above is skipped entirely, so
+			// reinstalled copies from a separate run do not inflate the count.
+			for _, occ := range run.occ {
+				if occ.size > 0 {
+					occurrenceCount++
+				}
+			}
 		}
 		if pathWaste == 0 {
 			// Path appears in multiple layers but every duplicate has size 0
@@ -189,34 +216,41 @@ func indexTree(node *FileNode, idx map[string]*FileNode) {
 // occurrence is recorded only at snapshots where the path was Added or
 // Modified — the layer that actually wrote new bytes. Unchanged carryover does
 // not contribute.
-func pathRuns(path string, indices []map[string]*FileNode) [][]efficiencyOccurrence {
-	var runs [][]efficiencyOccurrence
+//
+// Each run records whether it ended in a deletion. A run flushed by a Removed
+// node, an absence, or a nil index is marked endedInDeletion — none of its
+// bytes survive into the final image, yet all of them shipped, so
+// computeEfficiency charges every occurrence. A run flushed only by reaching
+// the end of the layer stack is still live; its last occurrence is the copy
+// present in the final image and is not waste.
+func pathRuns(path string, indices []map[string]*FileNode) []pathRun {
+	var runs []pathRun
 	var cur []efficiencyOccurrence
 
-	flush := func() {
+	flush := func(deleted bool) {
 		if len(cur) > 0 {
-			runs = append(runs, cur)
+			runs = append(runs, pathRun{occ: cur, endedInDeletion: deleted})
 			cur = nil
 		}
 	}
 
 	for i, idx := range indices {
 		if idx == nil {
-			flush()
+			flush(true)
 			continue
 		}
 		node, ok := idx[path]
 		switch {
 		case !ok:
-			flush()
+			flush(true)
 		case node.DiffType == Removed:
-			flush()
+			flush(true)
 		case node.DiffType == Added || node.DiffType == Modified:
 			cur = append(cur, efficiencyOccurrence{layerIdx: i, size: node.Size})
 		}
 		// Unchanged carryover: skip (no new bytes; don't flush).
 	}
-	flush()
+	flush(false)
 	return runs
 }
 

image/efficiency_test.go

@@ -139,13 +139,11 @@ func TestEfficiency_StableOrderOnEqualWaste(t *testing.T) {
 	}
 }
 
-// install_clean_reinstall: a file is added, deleted via whiteout, and a
-// different file appears at the same path. The two distinct files live in
-// separate runs and neither is wasted. Pre-Round-8 the algorithm counted
-// both copies as a duplicate occurrence of the same path and flagged the
-// first as waste — the canonical apt-get install + apt-get clean +
-// apt-get install bug.
-func TestEfficiency_InstallCleanReinstall_NoWaste(t *testing.T) {
+// install -> clean -> reinstall: the first copy is added, deleted by a
+// whiteout, then a fresh copy is added at the same path. The deleted first
+// copy still ships in its layer and is never reclaimed, so it is wasted; the
+// reinstall is a fresh live run and is not.
+func TestEfficiency_InstallCleanReinstall_FirstCopyWasted(t *testing.T) {
 	layers := []Layer{
 		{Index: 0, Size: 100, Tree: makeTree(
 			makeDir("var", "/var",
@@ -164,9 +162,13 @@ func TestEfficiency_InstallCleanReinstall_NoWaste(t *testing.T) {
 		)},
 	}
 	result := Efficiency(layers)
-	assert.Equal(t, int64(0), result.WastedBytes,
-		"a deletion between two writes means the first copy was properly cleaned up — not wasted")
-	assert.Empty(t, result.WastedFiles)
+	assert.Equal(t, int64(100), result.WastedBytes,
+		"the deleted first copy ships in layer 0 and is never reclaimed by the later whiteout")
+	require.Len(t, result.WastedFiles, 1)
+	assert.Equal(t, "/var/x", result.WastedFiles[0].Path)
+	assert.Equal(t, int64(100), result.WastedFiles[0].TotalWasted)
+	assert.Equal(t, 1, result.WastedFiles[0].LayerCount,
+		"only the deleted copy in layer 0 is waste; the reinstall in layer 2 is the live keeper")
 }
 
 // duplicate_in_same_run: a file is added, then modified in the next layer
@@ -192,10 +194,11 @@ func TestEfficiency_DuplicateInSameRun_IsWasted(t *testing.T) {
 	assert.Equal(t, int64(100), result.WastedFiles[0].TotalWasted)
 }
 
-// install -> modify -> clean -> reinstall: only the install->modify pair
-// inside the first run contributes waste; the post-clean reinstall is a
-// fresh run with one occurrence (no waste).
-func TestEfficiency_InstallModifyCleanReinstall_OnlyFirstRunWasted(t *testing.T) {
+// install -> modify -> clean -> reinstall: the first run (install then modify,
+// no deletion in between) ends in a whiteout. Both of its copies shipped and
+// neither survives, so both are wasted (100 + 80). The post-clean reinstall is
+// a fresh live run and is not.
+func TestEfficiency_InstallModifyCleanReinstall_FirstRunFullyWasted(t *testing.T) {
 	layers := []Layer{
 		{Index: 0, Size: 100, Tree: makeTree(
 			makeDir("etc", "/etc",
@@ -219,12 +222,14 @@ func TestEfficiency_InstallModifyCleanReinstall_OnlyFirstRunWasted(t *testing.T)
 		)},
 	}
 	result := Efficiency(layers)
-	assert.Equal(t, int64(100), result.WastedBytes,
-		"first run has occurrences (0,100)+(1,80); only the size-100 layer-0 copy is shadowed; the post-clean reinstall starts a new run")
+	assert.Equal(t, int64(180), result.WastedBytes,
+		"the first run (100 then 80) ends in a whiteout, so both shipped copies are wasted; the reinstall is a fresh live run")
 }
 
-// Opaque whiteout breaks a run just like an explicit per-file whiteout.
-func TestEfficiency_OpaqueWhiteout_BreaksRun(t *testing.T) {
+// Opaque whiteout ends a run just like an explicit per-file whiteout: the copy
+// written before the opaque marker still ships and is never reclaimed, so it is
+// wasted. The post-opaque copy is a fresh live run.
+func TestEfficiency_OpaqueWhiteout_PreOpaqueCopyWasted(t *testing.T) {
 	layers := []Layer{
 		{Index: 0, Size: 100, Tree: makeTree(
 			makeDir("var", "/var",
@@ -249,8 +254,61 @@ func TestEfficiency_OpaqueWhiteout_BreaksRun(t *testing.T) {
 		)},
 	}
 	result := Efficiency(layers)
-	assert.Equal(t, int64(0), result.WastedBytes,
-		"opaque whiteout should reset run; the post-opaque copy is fresh")
+	assert.Equal(t, int64(100), result.WastedBytes,
+		"the copy written before the opaque whiteout ships and is never reclaimed")
+	require.Len(t, result.WastedFiles, 1)
+	assert.Equal(t, "/var/cache/x", result.WastedFiles[0].Path)
+}
+
+// Pure add-then-delete: a file is added in one layer and whiteouted in the
+// next, with no reintroduction. Every byte shipped in the earlier layer and
+// none survives — the whole thing is waste, and the path is listed even though
+// it is absent from the final image.
+func TestEfficiency_AddThenDelete_FullyWasted(t *testing.T) {
+	layers := []Layer{
+		{Index: 0, Size: 1000, Tree: makeTree(
+			makeFile("big", "/big", 1000),
+		)},
+		{Index: 1, Size: 0, Tree: makeTree(
+			makeFile(".wh.big", "/.wh.big", 0),
+		)},
+	}
+	result := Efficiency(layers)
+	assert.Equal(t, int64(1000), result.WastedBytes,
+		"an added-then-deleted file ships its bytes and is never reclaimed")
+	require.Len(t, result.WastedFiles, 1)
+	assert.Equal(t, "/big", result.WastedFiles[0].Path)
+	assert.Equal(t, int64(1000), result.WastedFiles[0].TotalWasted)
+	assert.Equal(t, 1, result.WastedFiles[0].LayerCount)
+}
+
+// add -> modify -> delete, with no reintroduction: both shipped copies are in
+// one run that ends in deletion, so both are charged.
+func TestEfficiency_AddModifyThenDelete_BothCopiesWasted(t *testing.T) {
+	layers := []Layer{
+		{Index: 0, Size: 100, Tree: makeTree(makeFile("x", "/x", 100))},
+		{Index: 1, Size: 70, Tree: makeTree(makeFile("x", "/x", 70))},
+		{Index: 2, Size: 0, Tree: makeTree(makeFile(".wh.x", "/.wh.x", 0))},
+	}
+	result := Efficiency(layers)
+	assert.Equal(t, int64(170), result.WastedBytes,
+		"both the added and modified copies ship and neither survives the whiteout")
+	require.Len(t, result.WastedFiles, 1)
+	assert.Equal(t, int64(170), result.WastedFiles[0].TotalWasted)
+	assert.Equal(t, 2, result.WastedFiles[0].LayerCount)
+}
+
+// Regression guard: a file added once and never duplicated or deleted stays
+// live and contributes zero waste. The deletion-aware charge rule must not
+// touch the ordinary single-occurrence live case.
+func TestEfficiency_SingleLiveFile_NoWaste(t *testing.T) {
+	layers := []Layer{
+		{Index: 0, Size: 500, Tree: makeTree(makeFile("only", "/only", 500))},
+	}
+	result := Efficiency(layers)
+	assert.Equal(t, int64(0), result.WastedBytes)
+	assert.Empty(t, result.WastedFiles)
+	assert.Equal(t, 1.0, result.Score)
 }
 
 // regular_file_replaced_by_hardlink: a file is added as real bytes in layer 0