Drop odd-length flat dead byte; make --analyze score consistent

InnerLayer.split() padded self.data in place to make pairing work, but the
flat (unsplit) solution emits self.data directly, so odd-length flat tables
carried one unreachable trailing padding byte (and the reported cost, computed
from the un-padded length, was one element short of the emission).  Pad a local
copy instead; self.data stays the original array, so the flat solution emits
exactly len(data) elements.  Split solutions are unchanged (pairing still uses
the optimally-padded copy), so solution costs — and pick_solution's choices —
are identical; only the flat array for odd-length tables loses its dead byte.

Separately, `--analyze` computed the displayed Score with floor(log2)
(bit_length()-1) while pick_solution uses exact log2, so the highlighted "Best
solution" could disagree with the minimum-score row.  Use exact log2 and show
two decimals so the ranking is visibly consistent.

Verified: 236 tests pass; a C+Rust round-trip fuzz over all Pareto solutions
reports zero failures; regenerating HarfBuzz's tables yields byte-identical
output in ~10s (the fix is a no-op for multi-level tables).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: behdad

packTab/__init__.py

@@ -1155,18 +1155,22 @@ def split(self):
 
         When padding is needed, the padded element is never accessed
         (guaranteed unreachable), so we choose a padding value that
-        creates the most common pair, maximizing compression.
+        creates the most common pair, maximizing compression.  Padding is
+        applied to a local copy only; ``self.data`` is left untouched so the
+        flat (unsplit) solution emits the exact original array with no
+        unreachable trailing byte.
         """
-        if len(self.data) & 1:
+        data = self.data
+        if len(data) & 1:
             # Smart padding: choose value that creates most common pair.
             # The padded position is never accessed, so this is safe.
-            last_val = self.data[-1]
+            last_val = data[-1]
             padding = self._choose_optimal_padding(last_val)
-            self.data.append(padding)
+            data = data + [padding]
 
         # Collect pairs with frequencies and first occurrence positions
         from collections import Counter
-        pairs = [(self.data[i], self.data[i + 1]) for i in range(0, len(self.data), 2)]
+        pairs = [(data[i], data[i + 1]) for i in range(0, len(data), 2)]
         pair_freq = Counter(pairs)
         first_occurrence = {}
         for i, pair in enumerate(pairs):
@@ -1184,7 +1188,7 @@ def split(self):
             mapping[pair]  # Assigns next sequential ID
 
         # Apply mapping to create child layer data
-        data2 = _combine2(self.data, lambda a, b: mapping[(a, b)])
+        data2 = _combine2(data, lambda a, b: mapping[(a, b)])
 
         self.next = InnerLayer(data2)
 

packTab/__main__.py

@@ -15,6 +15,7 @@
 from . import *
 import argparse
 import sys
+from math import log2
 
 
 def main(args=None):
@@ -205,9 +206,14 @@ def main(args=None):
 
         for i, sol in enumerate(solutions):
             ratio = original_bytes / sol.cost if sol.cost > 0 else float("inf")
-            score = sol.nLookups + compression_values[0] * (sol.fullCost.bit_length() - 1)
+            # Use the same score pick_solution uses for 1..9 (exact log2, not
+            # floor via bit_length), so the highlighted "Best solution" matches
+            # the minimum-score row.
+            score = sol.nLookups + compression_values[0] * (
+                log2(sol.fullCost) if sol.fullCost > 0 else 0
+            )
             print(
-                f"{i+1:<3} {sol.nLookups:<8} {sol.nExtraOps:<9} {sol.cost:<6} {sol.fullCost:<8} {ratio:>6.2f}x {score:>7.1f}"
+                f"{i+1:<3} {sol.nLookups:<8} {sol.nExtraOps:<9} {sol.cost:<6} {sol.fullCost:<8} {ratio:>6.2f}x {score:>8.2f}"
             )
 
         print()

packTab/test.py

@@ -1005,6 +1005,66 @@ def test_help(self):
         assert r.returncode == 0
         assert "packTab" in r.stdout
 
+    def test_analyze_best_matches_min_score(self):
+        # Regression: the displayed Score used floor(log2) while pick_solution
+        # used exact log2, so the highlighted "Best solution" could disagree
+        # with the minimum-score row.  They must now agree for 1..9.
+        import random
+
+        rng = random.Random(555)
+        n = rng.randint(8, 900)
+        hi = rng.choice([3, 7, 15, 31, 63, 127, 255, 1023, 4095])
+        period = rng.choice([2, 3, 4, 5, 6, 8, 12, 16, 24, 32, 48, 64])
+        base = [rng.randint(0, hi) for _ in range(period)]
+        data = [
+            base[i % period] if rng.random() < rng.choice([0.7, 0.85, 0.95])
+            else rng.randint(0, hi)
+            for i in range(n)
+        ]
+        r = self._run("--analyze", "--compression", "6", *[str(v) for v in data])
+        assert r.returncode == 0
+
+        rows, best_idx = [], None
+        for line in r.stdout.splitlines():
+            parts = line.split()
+            # Solution rows look like: "<idx> <lookups> <extra> <bytes>
+            # <fullcost> <ratio>x <score>"; the ratio column ends in 'x'.
+            if len(parts) == 7 and parts[0].isdigit() and parts[5].endswith("x"):
+                rows.append((int(parts[0]), float(parts[6])))
+            elif line.startswith("Best solution"):
+                best_idx = int(line.rsplit("#", 1)[1])
+        assert rows and best_idx is not None
+        min_idx = min(rows, key=lambda t: t[1])[0]
+        assert best_idx == min_idx
+
+
+class TestFlatDeadByte:
+    """Odd-length flat tables must not emit the split() padding byte."""
+
+    def test_flat_odd_length_has_no_padding_byte(self):
+        data = [255, 1, 254, 2, 253, 3, 252, 4, 251]  # odd; flat is smallest
+        sol = pack_table(data, default=0, compression=10)
+        assert sol.nLookups == 1  # flat
+        code = Code("data")
+        sol.genCode(code, "get", language="c", private=False)
+        arrays = list(code.arrays.values())
+        assert arrays, "expected a real array, not an inline constant"
+        assert len(arrays[0].values) == len(data)  # no trailing dead byte
+
+    def test_inner_split_does_not_mutate_data(self):
+        layer = InnerLayer([1, 2, 3, 4, 5])  # odd length
+        assert len(layer.data) == 5  # split() padded a copy, not self.data
+
+    def test_flat_odd_length_roundtrips(self, language):
+        data = [255, 1, 254, 2, 253, 3, 252, 4, 251]
+        sol = pack_table(data, default=0, compression=10)
+        lang = languageClasses[language]()
+        code = Code("data")
+        sol.genCode(code, "get", language=lang, private=False)
+        buf = io.StringIO()
+        code.print_code(file=buf, language=lang)
+        _compile_and_run(buf.getvalue(), data, 0, language)
+
 
 class TestEdgeCases:
     """Test edge cases and boundary conditions."""