Skip to content

Conversation

@adityachoudhari26
Copy link
Contributor

@adityachoudhari26 adityachoudhari26 commented Nov 19, 2025

Summary by CodeRabbit

  • New Features

    • Deployment version selection now supports filtering based on earliest evaluation timestamp, enabling more granular control over which versions are considered during release planning and reconciliation workflows.
    • Enhanced deployment orchestration to leverage earliest-version constraints during the reconciliation process, improving version candidate selection logic.
  • Tests

    • Updated test scenarios to reflect new version evaluation behavior in multi-version deployment workflows.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Walkthrough

The PR introduces an "earliest version for evaluation" feature that enables filtering deployment candidate versions based on creation timestamps. The parameter threads through the reconciliation pipeline: from options constructors through handlers and planner to the versions manager, where it performs time-based filtering on candidate versions.

Changes

Cohort / File(s) Summary
Option Infrastructure
apps/workspace-engine/pkg/workspace/releasemanager/opts.go
Adds earliestVersionForEvaluation field to options struct and introduces WithEarliestVersionForEvaluation() public option constructor.
Handler Integration
apps/workspace-engine/pkg/events/handler/deploymentversion/deploymentversion.go
Updates HandleDeploymentVersionCreated and HandleDeploymentVersionUpdated to pass WithEarliestVersionForEvaluation(deploymentVersion) option to ReconcileTargets.
Planner Chain
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go, deployment_orchestrator.go
Adds planDeploymentConfig.earliestVersionForEvaluation field, WithEarliestVersionForEvaluation() option, and propagates the parameter to GetCandidateVersions invocation. Orchestrator passes option through to planner.
Versions Filtering
apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go
Updates GetCandidateVersions() signature to accept earliestVersionForEvaluation parameter; implements post-filter narrowing using CreatedAt timestamp comparison.
Test Updates
apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
Changes reconciliation trigger from DeploymentVersionUpdate to WorkspaceTick in TestEngine_EnvironmentProgression_MultipleVersions.

Sequence Diagram

sequenceDiagram
    participant Handler as Event Handler
    participant Orch as Orchestrator
    participant Plan as Planner
    participant VM as Versions Manager
    
    Handler->>Orch: Reconcile(WithEarliestVersionForEvaluation(version))
    Note over Orch: Extract earliestVersionForEvaluation<br/>from options
    Orch->>Plan: PlanDeployment(WithEarliestVersionForEvaluation(...))
    Note over Plan: Store in planDeploymentConfig
    Plan->>VM: GetCandidateVersions(releaseTarget,<br/>earliestVersionForEvaluation)
    Note over VM: Filter versions where<br/>CreatedAt >= earliestVersion.CreatedAt
    VM-->>Plan: [filtered candidates]
    Plan-->>Orch: deployment plan
    Orch-->>Handler: reconciliation result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Key areas requiring attention:

  • Versions manager filtering logic — Verify CreatedAt timestamp comparison correctness and nil-safety of earliestVersionForEvaluation
  • Parameter propagation — Confirm the option flows correctly through all layers without being lost
  • Test behavioral change — Understand why switching from DeploymentVersionUpdate to WorkspaceTick trigger preserves intended test semantics
  • Edge cases — Ensure filtering handles nil values and empty candidate lists appropriately

Poem

🐰 Through layers deep, a filter hops,
Time-based wisdom never stops,
Earliest versions mark the way,
Candidates bloom in measured day,
Each tick brings order, tick by tick! 🌱

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 accurately describes the main change: adding an earliest-version-for-evaluation parameter that filters candidate versions to only process newer ones, directly addressing the performance optimization goal.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch less-candidate-versions

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 880e735 and 7b7a959.

📒 Files selected for processing (6)
  • apps/workspace-engine/pkg/events/handler/deploymentversion/deploymentversion.go (2 hunks)
  • apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (3 hunks)
  • apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go (1 hunks)
  • apps/workspace-engine/pkg/workspace/releasemanager/opts.go (2 hunks)
  • apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go (2 hunks)
  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/workspace-engine/**/*.go

📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)

apps/workspace-engine/**/*.go: Do not add extraneous inline comments that state the obvious
Do not add comments that simply restate what the code does
Do not add comments for standard Go patterns (e.g., noting WaitGroup or semaphore usage)
Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods

Files:

  • apps/workspace-engine/pkg/workspace/releasemanager/opts.go
  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
  • apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go
  • apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go
  • apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go
  • apps/workspace-engine/pkg/events/handler/deploymentversion/deploymentversion.go
apps/workspace-engine/**/*_test.go

📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)

Follow the existing test structure used in *_test.go files

Files:

  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 658
File: apps/workspace-engine/pkg/engine/policy/rules/environment-version-rollout/rollout_start_time.go:65-80
Timestamp: 2025-08-19T22:58:15.170Z
Learning: In the environment-version-rollout rule's rollout start time logic, the code intentionally returns the latest ApprovedAt time (first non-nil record from records ordered by updatedAt) rather than the earliest. This is because the goal is to find the latest approval that caused all the rules to pass. While the complete logic for determining "the latest that caused all rules to pass" would be too complex, this approximation works well given that the UI prevents approving if all rules are already passing.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-08-12T20:49:05.086Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.

Applied to files:

  • apps/workspace-engine/pkg/workspace/releasemanager/opts.go
  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
  • apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go
  • apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go
  • apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go
  • apps/workspace-engine/pkg/events/handler/deploymentversion/deploymentversion.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*_test.go : Follow the existing test structure used in *_test.go files

Applied to files:

  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
📚 Learning: 2025-10-24T00:02:29.723Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 696
File: apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go:63-67
Timestamp: 2025-10-24T00:02:29.723Z
Learning: In workspace-engine replay mode (apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go), jobs are created with Pending status even during replay because job updates are still accepted and processed. If a job remains Pending after replay completes, it genuinely is still pending because no updates occurred during replay.

Applied to files:

  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*.go : Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods

Applied to files:

  • apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go
🧬 Code graph analysis (6)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (3)
apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (1)
  • TriggerReason (78-78)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
  • EntityRelation (234-242)
  • DeploymentVersion (211-222)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
  • WithEarliestVersionForEvaluation (76-80)
apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go (1)
apps/workspace-engine/pkg/events/handler/handler.go (1)
  • WorkspaceTick (88-88)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
  • EntityRelation (234-242)
  • DeploymentVersion (211-222)
apps/workspace-engine/pkg/workspace/releasemanager/trace/recorder.go (1)
  • ReconcileTarget (15-38)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (1)
  • WithEarliestVersionForEvaluation (39-43)
apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go (2)
apps/workspace-engine/pkg/workspace/releasemanager/manager.go (1)
  • Manager (26-32)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
  • ReleaseTarget (560-564)
  • DeploymentVersion (211-222)
  • CreatedAt (73-73)
apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go (2)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
  • WithEarliestVersionForEvaluation (76-80)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (1)
  • WithEarliestVersionForEvaluation (39-43)
apps/workspace-engine/pkg/events/handler/deploymentversion/deploymentversion.go (3)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (2)
  • WithTrigger (33-37)
  • WithEarliestVersionForEvaluation (39-43)
apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (2)
  • WithTrigger (151-155)
  • TriggerVersionCreated (88-88)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
  • WithEarliestVersionForEvaluation (76-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Typecheck
  • GitHub Check: Lint
  • GitHub Check: workspace-engine-tests
  • GitHub Check: tests
  • GitHub Check: build (linux/amd64)
🔇 Additional comments (6)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (1)

14-14: LGTM! Clean addition following established patterns.

The new earliestVersionForEvaluation field and option constructor follow the same pattern as existing options in this file.

Also applies to: 39-43

apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go (1)

28-28: LGTM! Filtering logic is correct.

The filtering logic correctly handles the sorted list (newest first) by breaking early once an older version is encountered. The comparison using Before() properly includes versions with CreatedAt >= earliestVersionForEvaluation.CreatedAt.

Note: The code assumes earliestVersionForEvaluation belongs to the same deployment as releaseTarget.DeploymentId. This is safe given the current callers (deployment version create/update handlers), but consider adding a defensive check or comment documenting this assumption if the function becomes more widely used.

Also applies to: 73-85

apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go (1)

96-96: LGTM! Option correctly threaded to planner.

The earliestVersionForEvaluation option is properly passed through to the planner, following the same pattern as other options like resourceRelationships and recorder.

apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)

61-61: LGTM! Clean option implementation and threading.

The earliestVersionForEvaluation field, option constructor, and usage all follow established patterns in this file. The option is correctly applied in the configuration loop and passed through to the version manager.

Also applies to: 76-80, 114-114

apps/workspace-engine/pkg/events/handler/deploymentversion/deploymentversion.go (1)

42-44: LGTM! Appropriate use of earliestVersionForEvaluation.

The handlers correctly pass the deployment version as earliestVersionForEvaluation for create and update events, limiting reconciliation to newer versions. The delete handler appropriately omits this option, allowing all versions to be reconsidered.

Both create and update handlers use TriggerVersionCreated, which is the only version trigger type defined in the codebase and reflects the intended behavior of treating version updates as creation events for reconciliation purposes.

apps/workspace-engine/test/e2e/engine_environment_progression_soak_test.go (1)

668-669: Verify test behavior aligns with feature intent.

The change from DeploymentVersionUpdate(version2) to WorkspaceTick(nil) is significant. With WorkspaceTick, no earliestVersionForEvaluation is passed, allowing all versions (including v1.0.0) to be reconsidered. This enables v1.0.0 to progress to production despite v2.0.0's recent deployment.

Please confirm this correctly reflects the intended behavior: when a workspace tick occurs, should the system reconsider all versions for progression, not just versions newer than a specific point?


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

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

@adityachoudhari26 adityachoudhari26 merged commit dccc1a3 into main Nov 19, 2025
8 checks passed
@adityachoudhari26 adityachoudhari26 deleted the less-candidate-versions branch November 19, 2025 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants