fix(defect-identity): degrade a redacted origin action to its selector

The full action key read the typed value straight from the trace, where
redaction renders every value typed into one field as the same string, so
two runs that typed different values there collapsed into one identity and
the report said nothing about it. The key now drops a redacted value, falls
back to the selector for that action, and counts the rows it did that to, so
the undercount reads as an undercount.

cmd/internal-tools/defect-identity/identity.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/priyanshujain/sanderling/internal/trace"
 	"github.com/priyanshujain/sanderling/internal/tracecorpus"
+	"github.com/priyanshujain/sanderling/internal/verifier"
 )
 
 // Instance is one defect as the draft identifies it across runs: the property
@@ -23,6 +24,11 @@ type Instance struct {
 	// run count only if one run reported the same property twice, and the
 	// latch says it cannot.
 	Reports int `json:"reports"`
+	// RedactedOrigin marks a row whose origin action reached the trace with its
+	// typed value redacted, so `full` keyed it by selector instead. Two runs
+	// that typed different values into that field are one row here, which makes
+	// the count of such rows a floor rather than a total.
+	RedactedOrigin bool `json:"redacted_origin,omitempty"`
 }
 
 // Unattributed is a violation that carries no origin, so the identity rule
@@ -76,6 +82,18 @@ func (c Corpus) Singletons() int {
 	return count
 }
 
+// DegradedIdentities counts instances the action key could not be computed for
+// in full, which are the rows a reader has to treat as a lower bound.
+func (c Corpus) DegradedIdentities() int {
+	count := 0
+	for _, instance := range c.Instances {
+		if instance.RedactedOrigin {
+			count++
+		}
+	}
+	return count
+}
+
 func identify(runs []tracecorpus.Run, mode actionKeyMode) (Corpus, error) {
 	corpus := Corpus{Runs: len(runs)}
 	byKey := map[identityKey]*Instance{}
@@ -111,17 +129,19 @@ func identify(runs []tracecorpus.Run, mode actionKeyMode) (Corpus, error) {
 				if detected.Screen == "" {
 					corpus.UnnamedScreen++
 				}
+				action, redactedOrigin := actionKey(origin, mode)
 				key := identityKey{
 					property: property,
-					action:   actionKey(origin, mode),
+					action:   action,
 					screen:   detected.Screen,
 				}
 				instance, seen := byKey[key]
 				if !seen {
 					instance = &Instance{
-						Property:      property,
-						OriginAction:  key.action,
-						WitnessScreen: key.screen,
+						Property:       property,
+						OriginAction:   key.action,
+						WitnessScreen:  key.screen,
+						RedactedOrigin: redactedOrigin,
 					}
 					byKey[key] = instance
 					order = append(order, key)
@@ -162,15 +182,21 @@ func index(steps []trace.Step) map[int]trace.Step {
 	return byIndex
 }
 
-// actionKey renders the action the origin step chose. The action recorded on a
-// line is the one applied after observing it, which is the alignment that
-// makes an origin index name an action at all.
-func actionKey(origin trace.Step, mode actionKeyMode) string {
+// actionKey renders the action the origin step chose, and reports whether the
+// key had to be degraded to the selector. The action recorded on a line is the
+// one applied after observing it, which is the alignment that makes an origin
+// index name an action at all.
+//
+// A typed value the record redacted is the same string for every value typed
+// into that field, so keying on it would merge distinct actions while reading
+// as a whole-action key. The key drops it and says it did, because an identity
+// that cannot be computed has to show as an undercount rather than as a count.
+func actionKey(origin trace.Step, mode actionKeyMode) (string, bool) {
 	if origin.NextAction == nil {
-		return "none"
+		return "none", false
 	}
 	if origin.ActionSkipped != "" {
-		return "none (" + origin.ActionSkipped + ")"
+		return "none (" + origin.ActionSkipped + ")", false
 	}
 	action := *origin.NextAction
 	key := action.Kind
@@ -183,8 +209,11 @@ func actionKey(origin trace.Step, mode actionKeyMode) string {
 		key += fmt.Sprintf(" (%d,%d)", action.X, action.Y)
 	}
 	if mode != byFullAction {
-		return key
+		return key, false
+	}
+	if action.Text == verifier.RedactedInputText {
+		return key + " text=redacted", true
 	}
 	return fmt.Sprintf("%s text=%q at=(%d,%d)->(%d,%d)",
-		key, action.Text, action.X, action.Y, action.ToX, action.ToY)
+		key, action.Text, action.X, action.Y, action.ToX, action.ToY), false
 }

cmd/internal-tools/defect-identity/identity_test.go

@@ -1,10 +1,12 @@
 package main
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/priyanshujain/sanderling/internal/trace"
 	"github.com/priyanshujain/sanderling/internal/tracecorpus"
+	"github.com/priyanshujain/sanderling/internal/verifier"
 )
 
 // TestOneDefectSeenTwiceIsOneInstance: two runs report the same property from
@@ -109,6 +111,74 @@ func TestTheStrictActionKeySplitsWhatTheSelectorKeyMerges(t *testing.T) {
 	}
 }
 
+// TestARedactedTypedValueDegradesTheFullKeyVisibly: two runs typed different
+// values into one field, both reached the trace redacted, and the whole action
+// can no longer tell them apart. The pair is one row, and the report has to say
+// so rather than let it read as one defect found twice.
+func TestARedactedTypedValueDegradesTheFullKeyVisibly(t *testing.T) {
+	first := run(t, 3, violating(1, "/login",
+		typing("id:password", recordedText(t, "hunter2")), "staysSignedIn", 1, 1))
+	second := run(t, 5, violating(1, "/login",
+		typing("id:password", recordedText(t, "correct horse")), "staysSignedIn", 1, 1))
+
+	corpus := identified(t, byFullAction, first, second)
+	if len(corpus.Instances) != 1 {
+		t.Fatalf("instances = %d, want the redacted pair to be one row: %+v",
+			len(corpus.Instances), corpus.Instances)
+	}
+	report := rendered(corpus)
+	if !strings.Contains(report, "1 identity") || !strings.Contains(report, "redacted") {
+		t.Fatalf("report does not say one identity rests on a redacted value:\n%s", report)
+	}
+}
+
+func TestARedactedOriginKeepsTheSelectorApart(t *testing.T) {
+	first := run(t, 3, violating(1, "/login",
+		typing("id:password", recordedText(t, "hunter2")), "staysSignedIn", 1, 1))
+	second := run(t, 5, violating(1, "/login",
+		typing("id:pin", recordedText(t, "hunter2")), "staysSignedIn", 1, 1))
+
+	corpus := identified(t, byFullAction, first, second)
+	if len(corpus.Instances) != 2 {
+		t.Fatalf("instances = %d, want two fields to stay two rows: %+v",
+			len(corpus.Instances), corpus.Instances)
+	}
+	if report := rendered(corpus); !strings.Contains(report, "2 identity") {
+		t.Fatalf("report does not count both degraded identities:\n%s", report)
+	}
+}
+
+func TestARedactedOriginDegradesNothingUnderTheSelectorKey(t *testing.T) {
+	only := run(t, 3, violating(1, "/login",
+		typing("id:password", recordedText(t, "hunter2")), "staysSignedIn", 1, 1))
+
+	if report := rendered(identified(t, bySelector, only)); strings.Contains(report, "redacted") {
+		t.Fatalf("selector key reads no text, so nothing degrades:\n%s", report)
+	}
+}
+
+// recordedText renders a typed value the way the runner records it, so what the
+// key sees is redaction as it really happens and not a placeholder the test
+// wrote itself.
+func recordedText(t *testing.T, typed string) string {
+	t.Helper()
+	recorded := verifier.RecordedActionText(verifier.Action{
+		Kind: verifier.ActionKindInputText,
+		On:   "id:password",
+		Text: typed,
+	}, nil)
+	if recorded == typed {
+		t.Fatalf("typed value %q reached the record unredacted", typed)
+	}
+	return recorded
+}
+
+func rendered(corpus Corpus) string {
+	var report strings.Builder
+	render(&report, corpus)
+	return report.String()
+}
+
 func identified(t *testing.T, mode actionKeyMode, runs ...tracecorpus.Run) Corpus {
 	t.Helper()
 	corpus, err := identify(runs, mode)

cmd/internal-tools/defect-identity/main.go

@@ -100,6 +100,13 @@ func render(out io.Writer, corpus Corpus) {
 
 	fmt.Fprintf(out, "\n%d distinct defect(s) over %d run(s); %d seen in exactly one run\n",
 		len(corpus.Instances), corpus.Runs, corpus.Singletons())
+	if degraded := corpus.DegradedIdentities(); degraded > 0 {
+		fmt.Fprintf(out,
+			"%d identity(ies) rest on the origin selector alone, because the value typed "+
+				"there is redacted in the record; two runs that typed different values into "+
+				"that field read as one, so the count above is a floor for those\n",
+			degraded)
+	}
 	if corpus.UnnamedScreen > 0 {
 		fmt.Fprintf(out,
 			"%d violation(s) witnessed on a screen the app does not name, "+