perf: use shared evaluators between release targets so that cache can be utilized

[adityachoudhari26]

Summary by CodeRabbit

• Performance

  • Improved policy evaluation throughput by concurrently loading release-target inputs and evaluating targets in parallel.
  • Introduced shared evaluator pooling with per-target filtering so only relevant rules run for each target.

• Bug Fixes

  • Prevented unintended modifications to shared rule evaluation data by ensuring each evaluation is copied before per-rule updates.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Reconcile was refactored into staged, concurrent steps: load all release-target inputs, build a shared evaluator set, then concurrently evaluate and persist per-target results using evaluators filtered to each target’s enabled policy rule IDs. Rule evaluations are deep-copied before mutation; evaluateVersion now accepts explicit evaluator slices.

Changes

Cohort / File(s)	Summary
Reconciliation Logic Reorganization apps/workspace-engine/svc/controllers/policyeval/reconcile.go	Refactored Reconcile to: concurrently load release-target inputs, build a shared evaluator set, filter evaluators per target, and concurrently evaluate/persist results. evaluateVersion now receives an explicit []evaluator. Added cloneRuleEvaluation using maps.Copy to deep-copy rule evaluation Details before mutation.

Sequence Diagram

sequenceDiagram
    participant Reconcile as Reconcile
    participant Loader as loadReleaseTargetInputs
    participant Builder as buildSharedEvaluators
    participant Filter as filterEvaluatorsForPolicies
    participant Evaluator as evaluateVersion
    participant Persister as evaluateAndPersist

    Reconcile->>Loader: concurrently load inputs for all targets
    Loader-->>Reconcile: inputs map
    Reconcile->>Builder: build shared evaluator set
    Builder-->>Reconcile: shared evaluators
    loop per release target (concurrent)
        Reconcile->>Filter: filter shared evaluators by target policy IDs
        Filter-->>Reconcile: filtered evaluators
        Reconcile->>Persister: evaluateAndPersist(filtered evaluators, inputs)
        Persister->>Evaluator: evaluateVersion(filtered evaluators, target inputs)
        Evaluator->>Evaluator: cloneRuleEvaluation (deep-copy) before WithRuleId
        Evaluator-->>Persister: evaluation results
        Persister-->>Reconcile: persisted results
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped through code with careful paws and cheer,
Shared evaluators built once, then filtered near.
I copy rules so no one’s state is torn,
Concurrent inputs load at early morn—
A rabbit claps for changes clear! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main optimization: refactoring evaluator instantiation to be shared across release targets to improve cache utilization.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch shared-evaluators

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

apps/workspace-engine/svc/controllers/policyeval/reconcile.go (1)

261-264: Include release-target identity in the worker errors.

load input: ... drops the target entirely, and DeploymentID is the same for every release target in this reconcile, so the later error still doesn't identify which target failed. Including EnvironmentID and ResourceID in both paths would make concurrent failures much easier to triage.

Example

 		g.Go(func() error {
 			r := &reconciler{getter: getter, setter: setter, rt: rt}
 			if err := r.loadInput(ctx); err != nil {
-				return fmt.Errorf("load input: %w", err)
+				return fmt.Errorf(
+					"load input for target env=%s resource=%s: %w",
+					r.rt.EnvironmentID,
+					r.rt.ResourceID,
+					err,
+				)
 			}
 			reconcilers[i] = r
 			return nil
 		})

 		g.Go(func() error {
 			evals := filterEvaluatorsForPolicies(sharedEvals, r.policies)
 			results, err := r.evaluateVersion(ctx, version, evals)
 			if err != nil {
-				return fmt.Errorf("evaluate version for rt %s: %w", r.rt.DeploymentID, err)
+				return fmt.Errorf(
+					"evaluate version for target env=%s resource=%s: %w",
+					r.rt.EnvironmentID,
+					r.rt.ResourceID,
+					err,
+				)
 			}
 			return r.persistEvaluations(ctx, results)
 		})

Also applies to: 298-300

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/workspace-engine/svc/controllers/policyeval/reconcile.go` around lines
261 - 264, The error return after calling r.loadInput currently loses which
release target failed; update the error wrapping to include the release-target
identity (e.g. EnvironmentID and ResourceID) from r (for example
r.ReleaseTarget.EnvironmentID and r.ReleaseTarget.ResourceID) so the message
becomes something like "load input for release target env=%s resource=%s: %w";
make the same change at the other occurrence referenced (the block around lines
298-300) so both error paths include EnvironmentID and ResourceID in their
fmt.Errorf wrappers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/workspace-engine/svc/controllers/policyeval/reconcile.go`:
- Around line 261-264: The error return after calling r.loadInput currently
loses which release target failed; update the error wrapping to include the
release-target identity (e.g. EnvironmentID and ResourceID) from r (for example
r.ReleaseTarget.EnvironmentID and r.ReleaseTarget.ResourceID) so the message
becomes something like "load input for release target env=%s resource=%s: %w";
make the same change at the other occurrence referenced (the block around lines
298-300) so both error paths include EnvironmentID and ResourceID in their
fmt.Errorf wrappers.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98c40bd2-5e28-464d-aa84-8e7532194532

📥 Commits

Reviewing files that changed from the base of the PR and between 76235cc and 14ed452.

📒 Files selected for processing (1)

• apps/workspace-engine/svc/controllers/policyeval/reconcile.go