feat(speculation): rank bestfirst on evidence Scorer

Compose evidence around a nested base from YAML type evidence. Drop the sibling predictor block. Overlay named factors; replace base wholesale.

Author: behinddwalls

service/submitqueue/orchestrator/server/BUILD.bazel

@@ -52,10 +52,9 @@ go_library(
         "//submitqueue/extension/conflict/pathoverlap:go_default_library",
         "//submitqueue/extension/speculation/allocator/sticky:go_default_library",
         "//submitqueue/extension/speculation/generator/bestfirst:go_default_library",
-        "//submitqueue/extension/speculation/predictor:go_default_library",
-        "//submitqueue/extension/speculation/predictor/evidence:go_default_library",
         "//submitqueue/extension/speculation/scorer:go_default_library",
         "//submitqueue/extension/speculation/scorer/composite:go_default_library",
+        "//submitqueue/extension/speculation/scorer/evidence:go_default_library",
         "//submitqueue/extension/speculation/scorer/fake:go_default_library",
         "//submitqueue/extension/speculation/scorer/heuristic:go_default_library",
         "//submitqueue/extension/speculation/speculator:go_default_library",
@@ -123,7 +122,6 @@ go_test(
         "//submitqueue/extension/buildrunner:go_default_library",
         "//submitqueue/extension/changeprovider:go_default_library",
         "//submitqueue/extension/conflict:go_default_library",
-        "//submitqueue/extension/speculation/predictor:go_default_library",
         "//submitqueue/extension/speculation/scorer:go_default_library",
         "//submitqueue/extension/speculation/speculator:go_default_library",
         "//submitqueue/extension/storage:go_default_library",

service/submitqueue/orchestrator/server/config.go

@@ -55,6 +55,7 @@ const (
 
 // Scorer types selectable from configuration.
 const (
+	scorerTypeEvidence  = "evidence"
 	scorerTypeHeuristic = "heuristic"
 	scorerTypeComposite = "composite"
 )
@@ -69,10 +70,7 @@ const (
 // Ways a composite scorer combines its components.
 const combineAvg = "avg"
 
-// Predictor types selectable from configuration.
-const predictorTypeEvidence = "evidence"
-
-// Evidence an evidence predictor prices, as named in configuration. The set is
+// Evidence an evidence scorer prices, as named in configuration. The set is
 // closed: a factor under any other name would be applied to nothing and never
 // noticed.
 const (
@@ -128,7 +126,6 @@ type namedQueueProfileConfig struct {
 	Analyzer       *analyzerConfig       `yaml:"analyzer"`
 	Scorer         *scorerConfig         `yaml:"scorer"`
 	Speculator     *speculatorConfig     `yaml:"speculator"`
-	Predictor      *predictorConfig      `yaml:"predictor"`
 }
 
 // queueProfileConfig is the full set of extensions a queue resolves to.
@@ -138,7 +135,6 @@ type queueProfileConfig struct {
 	Analyzer       analyzerConfig       `yaml:"analyzer"`
 	Scorer         scorerConfig         `yaml:"scorer"`
 	Speculator     speculatorConfig     `yaml:"speculator"`
-	Predictor      predictorConfig      `yaml:"predictor"`
 }
 
 // changeProviderConfig selects how change metadata is fetched. The github and
@@ -228,11 +224,19 @@ type analyzerConfig struct {
 	FailAlways bool `yaml:"failAlways"`
 }
 
-// scorerConfig selects how a queue ranks candidate speculation paths. There is
-// no scoring stage: the scorer feeds the queue's speculator, which is composed
-// from it rather than configured separately.
+// scorerConfig selects how a queue ranks candidate speculation paths. The
+// ranking scorer is evidence wrapping a nested content base. Heuristic and
+// composite belong on base (and on composite components), not at the top level.
 type scorerConfig struct {
 	Type string `yaml:"type"`
+	// Factors revise the base price, one per piece of evidence (evidence only).
+	// An omitted key keeps the inherited value, or 1 if neither defaults nor
+	// the queue named it.
+	Factors map[string]float64 `yaml:"factors"`
+	// Base is the content scorer evidence revises (evidence only). An omitted
+	// base on defaults is the default heuristic; a present base on a queue
+	// replaces the default base wholesale.
+	Base *scorerConfig `yaml:"base"`
 	// Buckets map a batch's total lines changed onto a score (heuristic only).
 	Buckets []bucketConfig `yaml:"buckets"`
 	// Components are the scorers a composite combines, keyed by name.
@@ -250,7 +254,7 @@ type bucketConfig struct {
 }
 
 // speculatorConfig tunes how much CI a queue's speculation may occupy. It has no
-// `type`: there is one speculator, composed from the queue's predictor, and what
+// `type`: there is one speculator, composed from the queue's scorer, and what
 // varies between queues is what it is allowed to spend.
 type speculatorConfig struct {
 	// BuildBudget caps how many builds this queue may have occupying CI at once,
@@ -259,18 +263,6 @@ type speculatorConfig struct {
 	BuildBudget int `yaml:"buildBudget"`
 }
 
-// predictorConfig tunes how a queue turns its scorer's price into the
-// probability the generator ranks on. The scorer being revised is the queue's
-// own, so it is not named again here.
-type predictorConfig struct {
-	Type string `yaml:"type"`
-	// Factors revise the scorer's price, one per piece of evidence and keyed by
-	// evidence name. An omitted key keeps the inherited value, or 1 if neither
-	// defaults nor the queue named it. An omitted predictor block inherits the
-	// whole default, so every factor stays 1 until someone sets one.
-	Factors map[string]float64 `yaml:"factors"`
-}
-
 // loadProfilesConfig reads and validates the profiles configuration at path.
 func loadProfilesConfig(path string) (profilesConfig, error) {
 	data, err := os.ReadFile(path)
@@ -323,7 +315,7 @@ func (c *profilesConfig) normalizeAndValidate() error {
 			}
 		}
 		if q.Scorer != nil {
-			if err := q.Scorer.normalizeAndValidate(where); err != nil {
+			if err := q.Scorer.normalizeOverlay(where); err != nil {
 				return err
 			}
 		}
@@ -332,11 +324,6 @@ func (c *profilesConfig) normalizeAndValidate() error {
 				return err
 			}
 		}
-		if q.Predictor != nil {
-			if err := q.Predictor.normalizeAndValidate(where); err != nil {
-				return err
-			}
-		}
 	}
 	return c.validateGitRepoPaths()
 }
@@ -390,33 +377,33 @@ func (c profilesConfig) resolve(q namedQueueProfileConfig) queueProfileConfig {
 		profile.Analyzer = *q.Analyzer
 	}
 	if q.Scorer != nil {
-		profile.Scorer = *q.Scorer
+		profile.Scorer = overlayScorer(profile.Scorer, *q.Scorer)
 	}
 	if q.Speculator != nil {
 		profile.Speculator = *q.Speculator
 	}
-	if q.Predictor != nil {
-		profile.Predictor = overlayPredictor(profile.Predictor, *q.Predictor)
-	}
 	return profile
 }
 
-// overlayPredictor keeps default factors the queue did not name. A present
-// predictor block is otherwise a normal extension override: type replaces when
-// set, and named factor keys win.
-func overlayPredictor(base, override predictorConfig) predictorConfig {
+// overlayScorer keeps default factors the queue did not name. A present base
+// replaces the default base wholesale. Type stays evidence unless the override
+// names one, which must still be evidence.
+func overlayScorer(base, override scorerConfig) scorerConfig {
 	if override.Type != "" {
 		base.Type = override.Type
 	}
-	if len(override.Factors) == 0 {
-		return base
+	if len(override.Factors) > 0 {
+		merged := maps.Clone(base.Factors)
+		if merged == nil {
+			merged = make(map[string]float64, len(override.Factors))
+		}
+		maps.Copy(merged, override.Factors)
+		base.Factors = merged
 	}
-	merged := maps.Clone(base.Factors)
-	if merged == nil {
-		merged = make(map[string]float64, len(override.Factors))
+	if override.Base != nil {
+		copied := *override.Base
+		base.Base = &copied
 	}
-	maps.Copy(merged, override.Factors)
-	base.Factors = merged
 	return base
 }
 
@@ -436,7 +423,7 @@ func (p *queueProfileConfig) normalizeAndValidate(where string) error {
 	if err := p.Speculator.normalizeAndValidate(where); err != nil {
 		return err
 	}
-	return p.Predictor.normalizeAndValidate(where)
+	return nil
 }
 
 func (c *changeProviderConfig) normalizeAndValidate(where string) error {
@@ -576,10 +563,56 @@ func (a *analyzerConfig) normalizeAndValidate(where string) error {
 	}
 }
 
-// normalizeAndValidate applies defaults and rejects a scorer that could not be
-// built. An empty block is a flat heuristic: every batch scores the same, which
-// is the neutral choice for a queue with no opinion about ordering.
+// normalizeAndValidate applies defaults and rejects a ranking scorer that
+// could not be built. An empty block is evidence wrapping the default
+// heuristic, with every factor neutral.
 func (s *scorerConfig) normalizeAndValidate(where string) error {
+	return s.normalizeRanking(where, true)
+}
+
+// normalizeOverlay validates a queue's scorer override without inventing a
+// base: omitted base means inherit the default base.
+func (s *scorerConfig) normalizeOverlay(where string) error {
+	return s.normalizeRanking(where, false)
+}
+
+func (s *scorerConfig) normalizeRanking(where string, fillBase bool) error {
+	if s.Type == "" {
+		s.Type = scorerTypeEvidence
+	}
+	if s.Type != scorerTypeEvidence {
+		return fmt.Errorf("%s: scorer type %q belongs under base, not at the ranking layer", where, s.Type)
+	}
+	if err := validateFactors(where, s.Factors); err != nil {
+		return err
+	}
+	if s.Base != nil {
+		return s.Base.normalizeContent(where + " base")
+	}
+	if fillBase {
+		s.Base = &scorerConfig{}
+		return s.Base.normalizeContent(where + " base")
+	}
+	return nil
+}
+
+func validateFactors(where string, factors map[string]float64) error {
+	for name, factor := range factors {
+		switch name {
+		case factorPathPassed, factorPathFailed, factorMerging, factorCancelling:
+		default:
+			return fmt.Errorf("%s: unknown scorer factor %q", where, name)
+		}
+		// Zero would permanently pin matching batches to 0; negatives cannot
+		// represent either direction in the factor contract.
+		if !(factor > 0) || math.IsInf(factor, 0) {
+			return fmt.Errorf("%s: scorer factor %q is %v, must be finite and positive", where, name, factor)
+		}
+	}
+	return nil
+}
+
+func (s *scorerConfig) normalizeContent(where string) error {
 	if s.Type == "" {
 		s.Type = scorerTypeHeuristic
 	}
@@ -601,7 +634,7 @@ func (s *scorerConfig) normalizeAndValidate(where string) error {
 			return fmt.Errorf("%s: composite scorer needs at least one component", where)
 		}
 		for name, component := range s.Components {
-			if err := component.normalizeAndValidate(fmt.Sprintf("%s component %q", where, name)); err != nil {
+			if err := component.normalizeContent(fmt.Sprintf("%s component %q", where, name)); err != nil {
 				return err
 			}
 			s.Components[name] = component
@@ -618,31 +651,6 @@ func (s *scorerConfig) normalizeAndValidate(where string) error {
 	return nil
 }
 
-// normalizeAndValidate applies defaults and rejects a predictor that could not
-// be built. An empty block is an evidence predictor with every factor neutral,
-// which prices a batch at exactly its scorer's price.
-func (p *predictorConfig) normalizeAndValidate(where string) error {
-	if p.Type == "" {
-		p.Type = predictorTypeEvidence
-	}
-	if p.Type != predictorTypeEvidence {
-		return fmt.Errorf("%s: unknown predictor type %q", where, p.Type)
-	}
-	for name, factor := range p.Factors {
-		switch name {
-		case factorPathPassed, factorPathFailed, factorMerging, factorCancelling:
-		default:
-			return fmt.Errorf("%s: unknown predictor factor %q", where, name)
-		}
-		// Zero would permanently pin matching batches to 0; negatives cannot
-		// represent either direction in the factor contract.
-		if !(factor > 0) || math.IsInf(factor, 0) {
-			return fmt.Errorf("%s: predictor factor %q is %v, must be finite and positive", where, name, factor)
-		}
-	}
-	return nil
-}
-
 func (s *speculatorConfig) normalizeAndValidate(where string) error {
 	// A negative budget is rejected rather than clamped: sticky would compute no
 	// free slots from it, so the queue would batch and then never build anything,