optimized the sort_loops in module.py and also improved readability

[Davda-James]

Proposed change

Reafactoring sort_loops in module.py, made it more readable and optimized.

no separate issue was there for it, suggestion by @pUrGe12 in PR #1146

Type of change

• Code refactoring without any functionality changes

Checklist

• I've followed the [contributing guidelines][contributing-guidelines]
• I've run make pre-commit, it didn't generate any changes
• I've run make test, all tests passed locally

[coderabbitai]

Summary by CodeRabbit

• Bug Fixes
  • Corrected execution ordering for steps, ensuring those without dependencies run first, followed by temporary-dependent steps, then normal dependencies. Prevents premature execution and related errors.
• Refactor
  • Simplified step scheduling logic for clearer prioritization and consistent payload construction, improving reliability and predictability.

Walkthrough

Refactors sort_loops in nettacker/core/module.py to partition steps into three buckets (no dependencies, temp-only dependencies, normal dependencies) and rebuilds the payload by concatenating those buckets, replacing the previous single-pass accumulation logic. No public API changes.

Changes

Cohort / File(s)	Summary
Module step sorting refactor nettacker/core/module.py	Rewrote sort_loops to classify steps into three lists based on dependent_on_temp_event and save_to_temp_events_only, then reconstruct the payload steps by concatenating those lists; removed previous inline accumulation and single-steps collection while preserving final ordering.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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
Title Check	✅ Passed	The title clearly summarizes the main change by stating that the sort_loops function in module.py was optimized and its readability improved, which aligns directly with the refactoring described in the diff. It is concise, specific to the core update, and avoids unrelated details.
Description Check	✅ Passed	The description directly relates to the changeset by explaining that sort_loops in module.py was refactored for readability and optimization, and it confirms context through checklist items showing tests passed and guidelines were followed.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between faf7a2e and baca7f5.

📒 Files selected for processing (1)

• nettacker/core/module.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use 4-space indents in Python code
Limit lines to 99 characters (ruff/ruff-format/isort profile=black)
Module and file names should use lower_snake_case
Function and variable names should use lower_snake_case
Class names should use PascalCase
Constants should use UPPER_SNAKE_CASE
Keep functions small and add type hints where practical

Files:

• nettacker/core/module.py

nettacker/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Add docstrings for public APIs in the nettacker package

Files:

• nettacker/core/module.py

nettacker/core/**

📄 CodeRabbit inference engine (AGENTS.md)

Place core libraries under nettacker/core/

Files:

• nettacker/core/module.py

⏰ 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). (4)

• GitHub Check: Test Docker 26.0.0-1ubuntu.24.04noble image build
• GitHub Check: Test Docker 26.1.4-1ubuntu.24.04noble image build
• GitHub Check: Test Docker image
• GitHub Check: Test Docker 27.5.0-1ubuntu.24.04noble image build

🔇 Additional comments (3)

nettacker/core/module.py (3)

122-124: LGTM! Clear variable names improve readability.

The three-bucket approach with descriptive variable names makes the sorting logic much more readable than the previous implementation. The names clearly indicate the purpose of each list and follow the snake_case convention.

---

126-133: Classification logic is correct and improves clarity.

The three-way partitioning correctly orders steps:

1. Steps without dependencies run first
2. Steps that save to temp events run second (making data available)
3. Steps that depend on temp events run last (consuming available data)

This refactor makes the dependency-based ordering explicit and easier to understand.

---

135-139: LGTM! Clean concatenation preserves intended ordering.

The concatenation clearly rebuilds the steps list in the correct order, and the multi-line formatting keeps the code within line length limits while remaining readable.

---

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.}

[Davda-James]

@pUrGe12 any updates on this ?

[pUrGe12]

LGTM, we'll let the maintainers take a look

[securestep9]

I am testing this PR and if all good will approve/merge soon

[arkid15r]

@Davda-James thanks for the PR!

It looks good for merging unless @securestep9's testing uncovers something unexpected. @pUrGe12 thanks for reviewing as well 👍

Just FYI:

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[securestep9]

array variable naming could be better

[Davda-James]

@securestep9 will change the variables names to be more self explanatory.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8787a2f and faf7a2e.

📒 Files selected for processing (1)

• nettacker/core/module.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use 4-space indents in Python code
Limit lines to 99 characters (ruff/ruff-format/isort profile=black)
Module and file names should use lower_snake_case
Function and variable names should use lower_snake_case
Class names should use PascalCase
Constants should use UPPER_SNAKE_CASE
Keep functions small and add type hints where practical

Files:

• nettacker/core/module.py

nettacker/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Add docstrings for public APIs in the nettacker package

Files:

• nettacker/core/module.py

nettacker/core/**

📄 CodeRabbit inference engine (AGENTS.md)

Place core libraries under nettacker/core/

Files:

• nettacker/core/module.py

⏰ 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: Test Docker 26.1.4-1ubuntu.24.04noble image build
• GitHub Check: Test Docker 26.0.0-1ubuntu.24.04noble image build
• GitHub Check: Test Docker 27.5.0-1ubuntu.24.04noble image build
• GitHub Check: Build package
• GitHub Check: Test Docker image

🔇 Additional comments (1)

nettacker/core/module.py (1)

122-139: Excellent refactoring—the three-bucket approach improves clarity.

The variable names (steps_without_dependencies, steps_with_temp_dependencies, steps_with_normal_dependencies) are self-explanatory and address the past review feedback about better naming. The logic correctly partitions steps based on dependency types and reconstructs them in the intended order.

[securestep9]

@Davda-James note that your commit is not verified and was rejected by the branch protection rule. Looks like you forgot to sign it?

[Davda-James]

@Davda-James note that your commit is not verified and was rejected by the branch protection rule. Looks like you forgot to sign it?

Signed it.

[securestep9]

GitHub branch protection still wont allow to merge. Either try to overwrite and sign the original commit or smiply submit a new PR (remember to sign the commit!) and close this one

[securestep9]

[Davda-James]

@securestep9 closed this one and raised a new one with single commit.