fix(baseline): counts extension closes the duplicate-call ratchet hole

internal/baseline/baseline.go

@@ -18,15 +18,31 @@ import (
 // "1", not the number 1 — flaglint-js's own ADR 008 originally documented
 // this as a number and had to be corrected to match the shipped string
 // implementation; flaglint-go starts from the corrected, actual contract.
+//
+// Counts is the optional v1-additive multiset extension (spec/
+// fingerprint.md in flaglint/spec): fingerprint -> occurrence count.
+// Mitigates the v1 fingerprint's known static-collision limitation (two
+// call sites sharing (callType, flagKey, file) share one fingerprint
+// string), which otherwise lets a brand-new *duplicate* of an
+// already-baselined call slip past --fail-on-new undetected — writers
+// SHOULD emit it (Write always does); readers MUST accept a baseline
+// without it, falling back to pure set semantics (Read/New both do).
 type File struct {
-	Version         string   `json:"version"`
-	CreatedAt       string   `json:"createdAt"`
-	FlaglintVersion string   `json:"flaglintVersion"`
-	Fingerprints    []string `json:"fingerprints"`
+	Version         string         `json:"version"`
+	SchemaVersion   string         `json:"schemaVersion"`
+	CreatedAt       string         `json:"createdAt"`
+	FlaglintVersion string         `json:"flaglintVersion"`
+	Fingerprints    []string       `json:"fingerprints"`
+	Counts          map[string]int `json:"counts,omitempty"`
 }
 
 const currentVersion = "1"
 
+// schemaVersion is the const value flaglint/spec's baseline.v1.schema.json
+// expects for the "schemaVersion" field — a separate, additive field from
+// "version" (the pre-existing "1" string), not a replacement for it.
+const schemaVersion = "baseline.v1"
+
 // Error carries the exit code a baseline failure should terminate the
 // process with — always 2 (invalid input) per the exit-code contract;
 // the type exists so callers can distinguish a baseline-specific failure
@@ -41,27 +57,40 @@ func errf(format string, args ...any) *Error {
 	return &Error{Message: fmt.Sprintf(format, args...)}
 }
 
-// Read loads and validates a baseline file, returning the set of known
-// fingerprints. Returns *Error for a missing file, invalid JSON, wrong
-// version, or a missing/malformed fingerprints array — the caller should
-// treat this as exit code 2.
-func Read(path string) (map[string]bool, error) {
+// Baseline is a parsed baseline file's contents, as needed for --fail-
+// on-new comparison (New). Counts is nil when the file has no "counts"
+// object at all (a baseline written before this feature, or a hand-
+// crafted one) — New falls back to pure set semantics in that case,
+// per spec.
+type Baseline struct {
+	Fingerprints map[string]bool
+	Counts       map[string]int
+}
+
+// Read loads and validates a baseline file, returning its known
+// fingerprint set and (if present) per-fingerprint counts. Returns
+// *Error for a missing file, invalid JSON, wrong version, or a
+// missing/malformed fingerprints array — the caller should treat this
+// as exit code 2. An absent or malformed "counts" object is not an
+// error: it's an optional, additive field, and its absence just means
+// New falls back to set semantics.
+func Read(path string) (Baseline, error) {
 	raw, err := os.ReadFile(path)
 	if err != nil {
 		if os.IsNotExist(err) {
-			return nil, errf("baseline file not found: %s", path)
+			return Baseline{}, errf("baseline file not found: %s", path)
 		}
-		return nil, errf("failed to read baseline file %s: %v", path, err)
+		return Baseline{}, errf("failed to read baseline file %s: %v", path, err)
 	}
 
 	var obj map[string]json.RawMessage
 	if err := json.Unmarshal(raw, &obj); err != nil {
-		return nil, errf("invalid JSON in baseline file: %s", path)
+		return Baseline{}, errf("invalid JSON in baseline file: %s", path)
 	}
 
 	var version string
 	if err := json.Unmarshal(obj["version"], &version); err != nil || version != currentVersion {
-		return nil, errf(`unsupported baseline version in %s. Expected version "1"`, path)
+		return Baseline{}, errf(`unsupported baseline version in %s. Expected version "1"`, path)
 	}
 
 	// A literal JSON `null` for fingerprints must be rejected the same as a
@@ -74,38 +103,63 @@ func Read(path string) (map[string]bool, error) {
 	// so its equivalent check rejects null for free.
 	rawFingerprints, ok := obj["fingerprints"]
 	if !ok || strings.TrimSpace(string(rawFingerprints)) == "null" {
-		return nil, errf("baseline file is missing a valid 'fingerprints' array: %s", path)
+		return Baseline{}, errf("baseline file is missing a valid 'fingerprints' array: %s", path)
 	}
 	var fingerprints []string
 	if json.Unmarshal(rawFingerprints, &fingerprints) != nil {
-		return nil, errf("baseline file is missing a valid 'fingerprints' array: %s", path)
+		return Baseline{}, errf("baseline file is missing a valid 'fingerprints' array: %s", path)
 	}
 
 	set := make(map[string]bool, len(fingerprints))
 	for _, fp := range fingerprints {
 		set[fp] = true
 	}
-	return set, nil
+
+	// "counts" is optional and additive — a missing or explicitly-null
+	// object is simply absent (nil), not an error; only a *present but
+	// malformed* one (wrong JSON shape) is worth surfacing, since that
+	// suggests real corruption rather than an older/hand-written file.
+	var counts map[string]int
+	if rawCounts, ok := obj["counts"]; ok && strings.TrimSpace(string(rawCounts)) != "null" {
+		if err := json.Unmarshal(rawCounts, &counts); err != nil {
+			return Baseline{}, errf("baseline file has a malformed 'counts' object: %s", path)
+		}
+	}
+
+	return Baseline{Fingerprints: set, Counts: counts}, nil
+}
+
+// CountFingerprints returns how many times each fingerprint appears in
+// fingerprints — the shared shape both Write (baselining today's counts)
+// and a --fail-on-new caller (computing the current scan's own counts to
+// compare against a baseline's) need.
+func CountFingerprints(fingerprints []string) map[string]int {
+	counts := make(map[string]int, len(fingerprints))
+	for _, fp := range fingerprints {
+		counts[fp]++
+	}
+	return counts
 }
 
 // Write deduplicates and sorts fingerprints and writes them to path,
+// along with each fingerprint's occurrence count (see File.Counts),
 // creating parent directories as needed.
 func Write(path string, fingerprints []string, flaglintVersion string) error {
-	seen := map[string]bool{}
-	deduped := make([]string, 0, len(fingerprints))
-	for _, fp := range fingerprints {
-		if !seen[fp] {
-			seen[fp] = true
-			deduped = append(deduped, fp)
-		}
+	counts := CountFingerprints(fingerprints)
+
+	deduped := make([]string, 0, len(counts))
+	for fp := range counts {
+		deduped = append(deduped, fp)
 	}
 	sort.Strings(deduped)
 
 	file := File{
 		Version:         currentVersion,
+		SchemaVersion:   schemaVersion,
 		CreatedAt:       time.Now().UTC().Format(time.RFC3339),
 		FlaglintVersion: flaglintVersion,
 		Fingerprints:    deduped,
+		Counts:          counts,
 	}
 
 	content, err := json.MarshalIndent(file, "", "  ")
@@ -122,13 +176,40 @@ func Write(path string, fingerprints []string, flaglintVersion string) error {
 	return nil
 }
 
-// New returns the fingerprints in current that are not present in known.
-func New(current []string, known map[string]bool) []string {
+// New returns every fingerprint that represents a genuinely new finding
+// beyond known: either the fingerprint is entirely absent from known's
+// set (existing, pre-counts set semantics), or — when known carries the
+// optional counts extension — the current scan's occurrence count for
+// that fingerprint exceeds the baselined one. The latter is what closes
+// the "ratchet hole" a bare fingerprint set can't see on its own: a
+// brand-new *duplicate* of an already-baselined call shares that call's
+// fingerprint (the v1 format's known static-collision limitation — see
+// spec/fingerprint.md) and would otherwise silently pass --fail-on-new.
+//
+// currentCounts is the current scan's own per-fingerprint occurrence
+// count (CountFingerprints over every usage's fingerprint, unfiltered by
+// known). A fingerprint present in known.Fingerprints but absent from
+// known.Counts (only possible for a malformed/hand-edited baseline, since
+// Write always emits a complete, consistent counts object) is treated as
+// a baselined count of 0, per spec — the conservative, "don't silently
+// pass debt through" fallback. When known.Counts is nil altogether (a
+// baseline written before this feature, or a hand-crafted one), this
+// falls back to pure set semantics — matching the spec's "readers MUST
+// accept baselines without it" requirement.
+func New(currentCounts map[string]int, known Baseline) []string {
 	newFindings := []string{}
-	for _, fp := range current {
-		if !known[fp] {
+	for fp, count := range currentCounts {
+		if !known.Fingerprints[fp] {
+			newFindings = append(newFindings, fp)
+			continue
+		}
+		if known.Counts == nil {
+			continue // no counts extension in play — pure set semantics
+		}
+		if count > known.Counts[fp] { // absent from Counts -> 0, per spec
 			newFindings = append(newFindings, fp)
 		}
 	}
+	sort.Strings(newFindings)
 	return newFindings
 }

internal/baseline/baseline_test.go

@@ -15,12 +15,21 @@ func TestWriteRead_roundTrip(t *testing.T) {
 		t.Fatalf("Write() error = %v", err)
 	}
 
-	set, err := Read(path)
+	got, err := Read(path)
 	if err != nil {
 		t.Fatalf("Read() error = %v", err)
 	}
-	if len(set) != 3 || !set["a"] || !set["b"] || !set["c"] {
-		t.Errorf("Read() = %v, want {a,b,c}", set)
+	if len(got.Fingerprints) != 3 || !got.Fingerprints["a"] || !got.Fingerprints["b"] || !got.Fingerprints["c"] {
+		t.Errorf("Read().Fingerprints = %v, want {a,b,c}", got.Fingerprints)
+	}
+	want := map[string]int{"a": 1, "b": 2, "c": 1}
+	if len(got.Counts) != len(want) {
+		t.Fatalf("Read().Counts = %v, want %v", got.Counts, want)
+	}
+	for k, v := range want {
+		if got.Counts[k] != v {
+			t.Errorf("Read().Counts[%q] = %d, want %d", k, got.Counts[k], v)
+		}
 	}
 }
 
@@ -50,6 +59,9 @@ func TestWrite_dedupedAndSorted(t *testing.T) {
 	if f.Version != "1" {
 		t.Errorf("Version = %q, want string \"1\"", f.Version)
 	}
+	if f.SchemaVersion != "baseline.v1" {
+		t.Errorf("SchemaVersion = %q, want baseline.v1 (flaglint/spec's baseline.v1.schema.json)", f.SchemaVersion)
+	}
 }
 
 func TestWrite_versionIsJSONString(t *testing.T) {
@@ -76,6 +88,31 @@ func TestWrite_createsParentDirectories(t *testing.T) {
 	}
 }
 
+func TestWrite_emitsCounts(t *testing.T) {
+	// A duplicate call (two occurrences sharing one v1 fingerprint) must
+	// be recorded as count 2, not silently collapsed to 1 the way the
+	// deduplicated fingerprints array itself is — this is the whole point
+	// of the counts extension (spec/fingerprint.md's "ratchet hole" fix).
+	path := filepath.Join(t.TempDir(), "baseline.json")
+	if err := Write(path, []string{"dup", "dup", "single"}, "1.0.0"); err != nil {
+		t.Fatalf("Write() error = %v", err)
+	}
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var f File
+	if err := json.Unmarshal(raw, &f); err != nil {
+		t.Fatal(err)
+	}
+	if f.Counts["dup"] != 2 {
+		t.Errorf("Counts[dup] = %d, want 2", f.Counts["dup"])
+	}
+	if f.Counts["single"] != 1 {
+		t.Errorf("Counts[single] = %d, want 1", f.Counts["single"])
+	}
+}
+
 func TestRead_missingFile(t *testing.T) {
 	_, err := Read(filepath.Join(t.TempDir(), "does-not-exist.json"))
 	if err == nil {
@@ -144,9 +181,40 @@ func TestRead_missingFingerprintsArray(t *testing.T) {
 	}
 }
 
+func TestRead_noCountsIsNilNotError(t *testing.T) {
+	// A baseline written before this feature (or hand-crafted) has no
+	// "counts" field at all — readers MUST accept it (spec), falling back
+	// to pure set semantics rather than erroring.
+	path := filepath.Join(t.TempDir(), "baseline.json")
+	if err := os.WriteFile(path, []byte(`{"version": "1", "fingerprints": ["a"]}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	got, err := Read(path)
+	if err != nil {
+		t.Fatalf("Read() error = %v, want nil", err)
+	}
+	if got.Counts != nil {
+		t.Errorf("Counts = %v, want nil", got.Counts)
+	}
+	if !got.Fingerprints["a"] {
+		t.Error("Fingerprints[a] = false, want true")
+	}
+}
+
+func TestRead_malformedCountsRejected(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "baseline.json")
+	if err := os.WriteFile(path, []byte(`{"version": "1", "fingerprints": ["a"], "counts": "not-an-object"}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	_, err := Read(path)
+	if err == nil {
+		t.Fatal("Read() error = nil, want error for malformed counts object")
+	}
+}
+
 func TestNew(t *testing.T) {
-	known := map[string]bool{"a": true, "b": true}
-	got := New([]string{"a", "b", "c", "d"}, known)
+	known := Baseline{Fingerprints: map[string]bool{"a": true, "b": true}}
+	got := New(CountFingerprints([]string{"a", "b", "c", "d"}), known)
 	want := []string{"c", "d"}
 	if len(got) != len(want) {
 		t.Fatalf("New() = %v, want %v", got, want)
@@ -159,11 +227,64 @@ func TestNew(t *testing.T) {
 }
 
 func TestNew_noneNewReturnsEmptyNotNil(t *testing.T) {
-	got := New([]string{"a"}, map[string]bool{"a": true})
+	known := Baseline{Fingerprints: map[string]bool{"a": true}}
+	got := New(CountFingerprints([]string{"a"}), known)
 	if got == nil {
 		t.Error("New() = nil, want empty non-nil slice")
 	}
 	if len(got) != 0 {
 		t.Errorf("New() = %v, want empty", got)
 	}
 }
+
+func TestNew_noCountsExtensionFallsBackToSetSemantics(t *testing.T) {
+	// known.Counts is nil (baseline predates this feature) — a duplicate
+	// occurrence of an already-known fingerprint must NOT be flagged as
+	// new; that's exactly what the counts extension exists to change,
+	// and its absence must mean the old, pre-feature behavior.
+	known := Baseline{Fingerprints: map[string]bool{"dup": true}}
+	got := New(CountFingerprints([]string{"dup", "dup"}), known)
+	if len(got) != 0 {
+		t.Errorf("New() = %v, want empty (no counts extension in play)", got)
+	}
+}
+
+func TestNew_duplicateCallExceedingBaselinedCountIsNew(t *testing.T) {
+	// The "ratchet hole" fix itself: a fingerprint already in the baseline
+	// set, baselined at count 1, now appearing twice in the current scan —
+	// a genuinely new duplicate call site — must be flagged as new.
+	known := Baseline{
+		Fingerprints: map[string]bool{"dup": true},
+		Counts:       map[string]int{"dup": 1},
+	}
+	got := New(CountFingerprints([]string{"dup", "dup"}), known)
+	if len(got) != 1 || got[0] != "dup" {
+		t.Errorf("New() = %v, want [dup]", got)
+	}
+}
+
+func TestNew_countNotExceedingBaselineIsNotNew(t *testing.T) {
+	known := Baseline{
+		Fingerprints: map[string]bool{"dup": true},
+		Counts:       map[string]int{"dup": 2},
+	}
+	got := New(CountFingerprints([]string{"dup", "dup"}), known)
+	if len(got) != 0 {
+		t.Errorf("New() = %v, want empty (current count does not exceed baselined count)", got)
+	}
+}
+
+func TestNew_fingerprintAbsentFromCountsTreatedAsZero(t *testing.T) {
+	// A fingerprint present in the set but absent from counts (only
+	// possible for a malformed/hand-edited baseline, since Write always
+	// emits a complete, consistent counts object) must be treated as
+	// baselined at count 0, per spec — the conservative fallback.
+	known := Baseline{
+		Fingerprints: map[string]bool{"a": true, "b": true},
+		Counts:       map[string]int{"a": 1}, // "b" deliberately absent
+	}
+	got := New(CountFingerprints([]string{"a", "b"}), known)
+	if len(got) != 1 || got[0] != "b" {
+		t.Errorf("New() = %v, want [b]", got)
+	}
+}