Skip to content

Remove implicit sequential dependency from typed action#369

Open
adam-cattermole wants to merge 1 commit into
mainfrom
task-dependencies
Open

Remove implicit sequential dependency from typed action#369
adam-cattermole wants to merge 1 commit into
mainfrom
task-dependencies

Conversation

@adam-cattermole

@adam-cattermole adam-cattermole commented May 29, 2026

Copy link
Copy Markdown
Member

DynamicTasks are treated as sequential implicitly rather than trusting the configuration to explicitly define dependence. Their onreply tasks also are tried to execute on response.. but then will enforce sequential ordering if requeued.

Once we migrate from legacy config we can remove the dependencies field entirely and have it explicitly defined.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved action dependency handling in the pipeline compilation process for better execution flow and consistency between different action types.

@adam-cattermole adam-cattermole self-assigned this May 29, 2026
@adam-cattermole adam-cattermole added the bug Something isn't working label May 29, 2026
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f898e6c7-72e5-4cda-8568-2c761e89ddb0

📥 Commits

Reviewing files that changed from the base of the PR and between fd9455e and 9857fa6.

📒 Files selected for processing (1)
  • src/kuadrant/pipeline/blueprint.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/kuadrant/pipeline/blueprint.rs

📝 Walkthrough

Walkthrough

Blueprint::compile now assigns an empty dependencies vector to ActionConfig::Typed actions instead of the positional set derived from the preceding action. Action::compile_typed similarly compiles gRPC on_reply actions with an empty dependency list. A unit test assertion is updated accordingly.

Changes

Typed Action Dependency Isolation

Layer / File(s) Summary
Empty dependency lists for typed actions and on_reply
src/kuadrant/pipeline/blueprint.rs
Blueprint::compile branches on action kind: ActionConfig::Legacy retains positional dependency derivation, while ActionConfig::Typed receives []. Action::compile_typed removes the intra-reply dependency chain, compiling each gRPC on_reply action with an empty dependency vector.
Updated test assertion
src/kuadrant/pipeline/blueprint.rs
mixed_legacy_and_typed_actions_compile changes the typed action's expected dependencies from vec!["0"] to is_empty().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 No more chains for the typed brigade,
Each action stands free, no debt to be paid.
Legacy links still thread as before,
But typed ones now open their own fresh door.
Hop along, dependencies — you're off the hook!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Remove implicit sequential dependency from typed action' directly and specifically describes the main change: eliminating automatic dependency inheritance for typed actions, which is the core modification across the codebase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 task-dependencies

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.

@adam-cattermole adam-cattermole moved this to Ready For Review in Kuadrant May 29, 2026
@adam-cattermole adam-cattermole moved this from Ready For Review to In Progress in Kuadrant Jun 2, 2026
@adam-cattermole adam-cattermole changed the base branch from main to various-cleanups June 3, 2026 16:38
@adam-cattermole adam-cattermole moved this from In Progress to Ready For Review in Kuadrant Jun 3, 2026
Base automatically changed from various-cleanups to main June 22, 2026 12:45
Signed-off-by: Adam Cattermole <a.d.cattermole@gmail.com>
Comment on lines +191 to +195
let dependencies = if i > 0 {
vec![(i - 1).to_string()]
} else {
vec![]
};

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps the dependencies between legacy types but not for the new types.. but dependencies should disappear once we burn Legacy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Ready For Review

Development

Successfully merging this pull request may close these issues.

1 participant