Decode covdata in-process for indexing speed, and fix nine review-found bugs

BENCHMARKS.md

@@ -320,11 +320,30 @@ the win scales with init cost times test count.
 Durations also become honest in this mode: they are measured inside the harness
 around `m.Run`, not around a process that spends most of its life initialising.
 
-The follow-up that would turn the wash into a win everywhere is parsing the
-binary covcounters format directly instead of shelling out to covdata per
-window. It is deliberately not attempted here: the format is internal to the Go
-toolchain and version-sensitive, and a maintenance trap should be walked into
-deliberately or not at all.
+The follow-up that turns the wash into a win everywhere is parsing the binary
+covmeta/covcounters formats directly instead of shelling out to covdata per
+window — and it is now built, with the maintenance trap fenced rather than
+walked into blind. Both formats are version-stamped and unchanged since Go
+1.20; the decoder refuses anything it does not recognise — future version,
+foreign magic, meta-hash mismatch, truncation — and any refusal falls back to
+the covdata subprocess path wholesale, so an incompatible future toolchain
+costs time, never correctness. A parity test drives the real toolchain end to
+end and requires the decoded blocks to equal the textfmt path's exactly.
+
+What the subprocess path paid per window was one spawn plus a re-read of the
+package's whole-closure meta plus a text profile dominated by zero-count
+lines the parser then discarded — a cost that scales with tests × closure
+size, which is exactly the product that buries large workspaces. Measured on a
+synthetic workspace of 90 packages where each of 30 test packages closes over
+61 instrumented packages (600 tests, warm build caches, 4 cores):
+
+| Path | Wall | user+sys CPU |
+|---|---|---|
+| covdata subprocess per window | 13.1s | 41.3s |
+| in-process decoding | **8.5s** | **25.7s** |
+
+At a 201-package closure the same shape gives 8.7s against 5.7s. The remaining
+time is compile, link and test execution; the per-test conversion tax is gone.
 
 **The full 353-package index was not measured here, and the reason is worth
 recording.** The first attempt drove load average to 30 on a 4-core box and was

internal/cli/index.go

@@ -96,7 +96,7 @@ func newIndexCommand(opts *options) *cobra.Command {
 				Jobs:        jobs,
 				Timeout:     timeout,
 				Packages:    targets,
-				Progress:    newProgress(cmd.ErrOrStderr(), quiet, opts.painter()),
+				Progress:    newProgress(cmd.ErrOrStderr(), quiet, opts.errPainter()),
 				Legacy:      legacy,
 			})
 			if err != nil {
@@ -108,7 +108,8 @@ func newIndexCommand(opts *options) *cobra.Command {
 		},
 	}
 
-	cmd.Flags().IntVar(&jobs, "jobs", 0, "packages to index concurrently (default: GOMAXPROCS)")
+	cmd.Flags().IntVar(&jobs, "jobs", 0,
+		"packages to index concurrently (default: half of GOMAXPROCS; each worker builds with its share of the cores)")
 	cmd.Flags().DurationVar(&timeout, "timeout", 0, "per-test timeout (default: 60s)")
 	cmd.Flags().StringSliceVar(&packages, "packages", nil, "limit indexing to these import paths")
 	cmd.Flags().BoolVar(&quiet, "quiet", false, "suppress progress output")

internal/cli/mutate.go

@@ -146,7 +146,7 @@ func runMutate(cmd *cobra.Command, opts *options, flags *mutateFlags) error {
 		mutants = mutate.Exclude(mutants, preflight.Failing)
 	}
 
-	execOpts.Progress = newProgress(cmd.ErrOrStderr(), flags.quiet, opts.painter())
+	execOpts.Progress = newProgress(cmd.ErrOrStderr(), flags.quiet, opts.errPainter())
 
 	results, err := mutate.Execute(ctx, mutants, execOpts)
 	if err != nil {

internal/cli/root.go

@@ -72,14 +72,30 @@ type options struct {
 	noColor  bool
 }
 
+func (o *options) colorEnabled() bool {
+	return !o.noColor && os.Getenv("NO_COLOR") == ""
+}
+
+// useColor answers for stdout-bound output and useColorErr for stderr-bound:
+// the two can disagree under redirection — `assay run 2>log` must not write
+// escapes into the log because stdout is still a terminal, and `assay run
+// >results` must not strip color from a summary still heading to one.
 func (o *options) useColor() bool {
-	return !o.noColor && os.Getenv("NO_COLOR") == "" && isTerminal(os.Stdout)
+	return o.colorEnabled() && isTerminal(os.Stdout)
+}
+
+func (o *options) useColorErr() bool {
+	return o.colorEnabled() && isTerminal(os.Stderr)
 }
 
 func (o *options) painter() ui.Painter {
 	return ui.New(o.useColor())
 }
 
+func (o *options) errPainter() ui.Painter {
+	return ui.New(o.useColorErr())
+}
+
 func NewRootCommand() *cobra.Command {
 	opts := &options{}
 

internal/cli/run.go

@@ -27,7 +27,7 @@ func newRunCommand(opts *options) *cobra.Command {
 				return err
 			}
 
-			printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColor())
+			printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColorErr())
 			if err := printer.Summary(summary, opts.verbose); err != nil {
 				return err
 			}

internal/cli/verify.go

@@ -31,7 +31,7 @@ func newVerifyCommand(opts *options) *cobra.Command {
 				return err
 			}
 
-			printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColor())
+			printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColorErr())
 			if err := printer.Summary(summary, opts.verbose); err != nil {
 				return err
 			}

internal/cover/classify.go

@@ -315,7 +315,7 @@ func mergeBlocksOrEmpty(blocks []Block) []Block {
 		return nil
 	}
 
-	return mergeBlocks(blocks)
+	return MergeBlocks(blocks)
 }
 
 func splitLines(source []byte) []string {

internal/cover/profile.go

@@ -43,15 +43,20 @@ func ParseProfile(reader io.Reader) ([]FileBlocks, error) {
 		if len(blocks) == 0 {
 			continue
 		}
-		out = append(out, FileBlocks{ProfileName: profile.FileName, Blocks: mergeBlocks(blocks)})
+		out = append(out, FileBlocks{ProfileName: profile.FileName, Blocks: MergeBlocks(blocks)})
 	}
 
 	sort.Slice(out, func(i, j int) bool { return out[i].ProfileName < out[j].ProfileName })
 
 	return out, nil
 }
 
-func mergeBlocks(blocks []Block) []Block {
+// MergeBlocks sorts blocks and coalesces overlapping or line-adjacent ones,
+// the same normalisation ParseProfile applies to text profiles. Callers that
+// assemble blocks from another source (the binary covdata decoder) use it so
+// both paths emit identical shapes. The input must be non-empty; it is
+// modified in place.
+func MergeBlocks(blocks []Block) []Block {
 	sort.Slice(blocks, func(i, j int) bool {
 		if blocks[i].StartLine != blocks[j].StartLine {
 			return blocks[i].StartLine < blocks[j].StartLine

internal/flake/flake_test.go

@@ -1,6 +1,7 @@
 package flake
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"sync"
@@ -175,6 +176,33 @@ func TestCompactionKeepsTheNewestObservations(t *testing.T) {
 	assert.Equal(t, int64(50), minBusy, "the newest observations are the ones kept")
 }
 
+// TestCompactionKeepsTheNewestAppendsOnTimestampTies pins the tie order: a
+// preflight stamps one clock reading across a whole batch, so a group over the
+// cap can be all-ties — and compaction must keep the latest appends, not the
+// order the file happened to serve them in.
+func TestCompactionKeepsTheNewestAppendsOnTimestampTies(t *testing.T) {
+	j, err := OpenJournal(t.TempDir(), "/root")
+	require.NoError(t, err)
+
+	var all []Observation
+	for i := range maxPerTest + 10 {
+		o := obs("p", "TestTied", "fp", VerdictPass, 7)
+		o.Detail = fmt.Sprintf("append-%03d", i)
+		all = append(all, o)
+	}
+	require.NoError(t, j.Record(all))
+
+	require.NoError(t, j.compactLocked())
+
+	got, err := j.Load()
+	require.NoError(t, err)
+	require.Len(t, got, maxPerTest)
+	for _, o := range got {
+		assert.NotEqual(t, "append-000", o.Detail,
+			"the oldest appends are the ones compaction must discard")
+	}
+}
+
 func TestParseVerboseOutputTakesTopLevelVerdictsOnly(t *testing.T) {
 	output := []byte(`=== RUN   TestAlpha
 --- PASS: TestAlpha (0.00s)

internal/flake/journal.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"sort"
 
 	"github.com/zeebo/blake3"
@@ -162,6 +163,11 @@ func (j *Journal) compactLocked() error {
 
 	kept := make([]Observation, 0, len(observations))
 	for _, group := range grouped {
+		// The group arrives in append order, oldest first, and observations
+		// batched in one write share a timestamp. Reversing before the stable
+		// sort makes the newest appends win ties; without it a group trimmed
+		// at the cap kept its stalest evidence and discarded its newest.
+		slices.Reverse(group)
 		sort.SliceStable(group, func(i, j int) bool { return group[i].Unix > group[j].Unix })
 		if len(group) > maxPerTest {
 			group = group[:maxPerTest]

internal/index/collect_test.go

@@ -222,6 +222,43 @@ func TestFingerprintDiffersPerPackage(t *testing.T) {
 	assert.Equal(t, calc, fp.For(testfixture.Module+"/calc"), "fingerprints must be stable")
 }
 
+func TestFingerprintTracksWorkspaceRoot(t *testing.T) {
+	h := setup(t)
+	other := setup(t)
+
+	require.NotEqual(t, h.root, other.root)
+
+	same := index.NewFingerprinter(h.graph, h.digests, nil).For(testfixture.Module + "/calc")
+	moved := index.NewFingerprinter(other.graph, other.digests, nil).For(testfixture.Module + "/calc")
+
+	assert.NotEqual(t, same, moved,
+		"records store absolute paths, so an identical tree at another root must not share records")
+}
+
+// TestFingerprintScopesTestdataModuleFilesToTheirPackage pins the ownership
+// rule for fixture module files: a go.mod under a package's testdata belongs
+// to that package. Classified as unowned it was hashed into every package's
+// fingerprint, and editing one fixture invalidated the entire index.
+func TestFingerprintScopesTestdataModuleFilesToTheirPackage(t *testing.T) {
+	h := setup(t)
+
+	before := index.NewFingerprinter(h.graph, h.digests, nil)
+	calcBefore := before.For(testfixture.Module + "/calc")
+	repoBefore := before.For(testfixture.Module + "/repo")
+
+	digests := make(map[string]string, len(h.digests)+1)
+	for path, digest := range h.digests {
+		digests[path] = digest
+	}
+	digests["calc/testdata/proj/go.mod"] = "0000000000000000000000000000000000000001"
+
+	after := index.NewFingerprinter(h.graph, digests, nil)
+	assert.NotEqual(t, calcBefore, after.For(testfixture.Module+"/calc"),
+		"the fixture belongs to calc and must invalidate it")
+	assert.Equal(t, repoBefore, after.For(testfixture.Module+"/repo"),
+		"a calc fixture must not invalidate unrelated packages")
+}
+
 func TestFingerprintTracksDependencies(t *testing.T) {
 	h := setup(t)
 	before := index.NewFingerprinter(h.graph, h.digests, nil).For(testfixture.Module + "/svc")